Bug 70580 – [Chromium] Implement MessagePort transfer in chromium port of webkit

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

[Dmitry Lomov]

SerializedScriptValue should support transferring message ports

[Dmitry Lomov]

Created attachment 111901 [details]
Fix

[David Levin]

Comment on attachment 111901 [details]
Fix

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

Looks good (except I just want to understand the wrap issue before I r+).

> LayoutTests/fast/events/resources/message-port-multi.js:58
> +            if(event.ports && event.ports.length > 0 && event.ports[0] === event.data.port)

add space after if

> LayoutTests/fast/events/resources/message-port-multi.js:79
> +            testFailed("Unexpexcted message " + event.data);

typo: Unexpexcted

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:1065
> +            uint32_t messagePortIndex;

indent is incorrect here and below

> Source/WebCore/bindings/v8/SerializedScriptValue.cpp:1801
> +        *object = V8MessagePort::wrap(m_transferredMessagePorts->at(index).get());

It is unintuitive to me that both sides of this use "wrap". (line 598 and 1801). It feels like one of them should be unwrap.

[Dmitry Lomov]

(In reply to comment #2)

> It is unintuitive to me that both sides of this use "wrap". (line 598 and 1801). It feels like one of them should be unwrap.

On both sides we go from MessagePort (webkit object) to corresponding V8 object. Hence the wrap on both ends.

[David Levin]

Comment on attachment 111901 [details]
Fix

ok. Please fix up the nits mentioned before landing. Thanks!

[Dmitry Lomov]

Created attachment 112039 [details]
Fixes in tests + testdata for JSC

[WebKit Review Bot]

Comment on attachment 112039 [details]
Fixes in tests + testdata for JSC

Clearing flags on attachment: 112039

Committed r98175: <http://trac.webkit.org/changeset/98175>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.