Bug 70120 - [Chromium] Pass MessagePortArray to SerializedScriptValue::serialize/deserialize
Summary: [Chromium] Pass MessagePortArray to SerializedScriptValue::serialize/deserialize
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dmitry Lomov
URL:
Keywords:
Depends on:
Blocks: 65209
  Show dependency treegraph
 
Reported: 2011-10-14 11:23 PDT by Dmitry Lomov
Modified: 2011-10-14 15:12 PDT (History)
4 users (show)

See Also:


Attachments
Fix (20.15 KB, patch)
2011-10-14 11:27 PDT, Dmitry Lomov
levin: review+
levin: commit-queue-
Details | Formatted Diff | Diff
CR feedback (20.10 KB, patch)
2011-10-14 12:02 PDT, Dmitry Lomov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Lomov 2011-10-14 11:23:16 PDT
SerializedScriptValue needs to know the message ports being sent with the message to implement trasfer
Comment 1 Dmitry Lomov 2011-10-14 11:27:54 PDT
Created attachment 111033 [details]
Fix
Comment 2 David Levin 2011-10-14 11:46:55 PDT
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.
Comment 3 David Levin 2011-10-14 11:47:35 PDT
(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.
Comment 4 David Levin 2011-10-14 11:50:22 PDT
OK ignore the enum comment since the uses are in file.
Comment 5 Dmitry Lomov 2011-10-14 12:02:25 PDT
Created attachment 111042 [details]
CR feedback
Comment 6 WebKit Review Bot 2011-10-14 15:12:09 PDT
Comment on attachment 111042 [details]
CR feedback

Clearing flags on attachment: 111042

Committed r97516: <http://trac.webkit.org/changeset/97516>
Comment 7 WebKit Review Bot 2011-10-14 15:12:14 PDT
All reviewed patches have been landed.  Closing bug.