electron / libchromiumcontent

Shared library build of Chromium’s Content module
MIT License
486 stars 183 forks source link

Make v8 api forwards compatible with v8 6.7 #570

Closed nornagon closed 6 years ago

nornagon commented 6 years ago

Cribbed from https://github.com/nodejs/node/commit/2a3f8c3a83e3820d3c601bdf02eda8db462027aa

alexeykuzmin commented 6 years ago

Why do we need this?

nornagon commented 6 years ago

Building 3rd-party modules against the version of node we depend on (v10.2.0) results in modules that are incompatible with the version of v8 in chrome 66 (v6.6.346.32), because node v10.2.0 includes this patch in its vendored v8.

In particular, the thing i ran into is that node v10.2.0 has this signature for v8::FunctionTemplate::New:

Local<FunctionTemplate> FunctionTemplate::New(
    Isolate* isolate, FunctionCallback callback, v8::Local<Value> data,
    v8::Local<Signature> signature, int length, ConstructorBehavior behavior,
    SideEffectType side_effect_type);

but the function signature for v8::FunctionTemplate::New in chromium's version of v8 (v6.6.346.32) is:

Local<FunctionTemplate> FunctionTemplate::New(
    Isolate* isolate, FunctionCallback callback, v8::Local<Value> data,
    v8::Local<Signature> signature, int length, ConstructorBehavior behavior);

So a module built against node v10.2.0's v8 headers won't work with our v8.

alexeykuzmin commented 6 years ago

So a module built against node v10.2.0's v8 headers won't work with our v8.

Is it expected to do so? @zcbenz ,can you help us to understand how the thing should work here?

nornagon commented 6 years ago

ah, i see that the create-node-headers.py script in e/e overrides nodejs's v8 headers with the headers from chrome. I should probably figure out a way to do that in the gn build.

MarshallOfSound commented 6 years ago

In the past we've tried to make this work (building native modules for electron by building them on the right node version) but afaik it has sometimes worked on *nix but it has never worked on windows.

We have electron-rebuild and friends to make getting around this problem easy

nornagon commented 6 years ago

electron-rebuild doesn't work for development tho (i.e. developing on an unreleased electron)

MarshallOfSound commented 6 years ago

This is correct but there are helper scripts in the electron repo to rebuild native modules with electron generated headers.

nornagon commented 6 years ago

Closed, going to instead figure out a nice way to build our test modules against the proper v8 headers in the GN build.