Bug 70120 – [Chromium] Pass MessagePortArray to SerializedScriptValue::serialize/deserialize

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Dmitry Lomov]

SerializedScriptValue needs to know the message ports being sent with the message to implement trasfer

[Dmitry Lomov]

Created attachment 111033 [details]
Fix

[David Levin]

Comment on attachment 111033 [details]
Fix

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

Please fix these small issues and commit away!

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:1923
> +SerializedScriptValue::SerializedScriptValue(v8::Handle<v8::Value> value, MessagePortArray* messagePorts, bool& didThrow)

Omit the var name (since it isn't used) or else you'll get compile errors on some platforms.

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:1968
> +v8::Handle<v8::Value> SerializedScriptValue::deserialize(MessagePortArray* messagePorts)

Ditto.

> Source/WebCore/bindings/v8/SerializedScriptValue.h:81
> +    SerializedScriptValue(v8::Handle<v8::Value>, MessagePortArray* messagePorts, bool& didThrow);

Omit the variable name since it adds no information.

> Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:356
> +    return handlePostMessageCallback(args, false);

We prefer enums in WebKit when the value passed in is just true/false to make the call site more readable.

> Source/WebCore/bindings/v8/custom/V8DedicatedWorkerContextCustom.cpp:57
> +     ExceptionCode ec = 0;

Indentation is off.

[David Levin]

(In reply to comment #2)
> (From update of attachment 111033 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111033&action=review
> 
> > Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:356
> > +    return handlePostMessageCallback(args, false);
> 
> We prefer enums in WebKit when the value passed in is just true/false to make the call site more readable.

Note this one is true in several places.

[David Levin]

OK ignore the enum comment since the uses are in file.

[Dmitry Lomov]

Created attachment 111042 [details]
CR feedback

[WebKit Review Bot]

Comment on attachment 111042 [details]
CR feedback

Clearing flags on attachment: 111042

Committed r97516: <http://trac.webkit.org/changeset/97516>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.