Bug 84202 – [V8] Add an optional Isolate argument to wrap()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kentaro Hara]

The final objective is to pass Isolate around in V8 bindings. In this bug, we add an optional Isolate argument to wrap(). After rewriting all wrap() callers so that they pass Isolate to wrap(), I'll make the Isolate argument non-optional.

[Kentaro Hara]

Created attachment 137620 [details]
Patch

[WebKit Review Bot]

Attachment 137620 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/bindings/scripts/test/V8/V8TestEventTarget.h:43:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestEventConstructor.h:44:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestInterface.h:45:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestNode.h:43:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8Float64Array.h:44:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestObj.h:43:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestNamedConstructor.h:49:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestMediaQueryListListener.h:43:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestActiveDOMObject.h:43:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestCustomNamedGetter.h:43:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.h:45:  The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 11 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kentaro Hara]

All bots got green. The style error seems false positive (We want to write the default value in the header). Review?

[Nate Chapin]

Comment on attachment 137620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137620&action=review

>> Source/WebCore/bindings/scripts/test/V8/V8Float64Array.h:44
>> +    inline static v8::Handle<v8::Object> wrap(Float64Array*, v8::Isolate* isolate = 0);
> 
> The parameter name "isolate" adds no information, so it should be removed.  [readability/parameter_name] [5]

It's objecting to the variable name, not the assignment. I believe you can change this to "v8::Isolate* = 0"?

[Kentaro Hara]

Committed r114519: <http://trac.webkit.org/changeset/114519>

[Kentaro Hara]

(In reply to comment #4)
> It's objecting to the variable name, not the assignment. I believe you can change this to "v8::Isolate* = 0"?

Fixed. Thanks for reviews!

[Kentaro Hara]

Reverted r114519 for reason:

Chromium crash

Committed r114730: <http://trac.webkit.org/changeset/114730>

[Kentaro Hara]

Created attachment 138367 [details]
Patch for relanding. It is revealed that the patch was innocent.

[Kentaro Hara]

Committed r114911: <http://trac.webkit.org/changeset/114911>