New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 667910 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , All
Pri: 2
Type: Compat



Sign in to add a comment

ImageBitmap is not transferable over messagePort (event.data === null)

Reported by acmesqua...@gmail.com, Nov 22 2016

Issue description

UserAgent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0

Steps to reproduce the problem:
const channel = new MessageChannel;

channel.port1.onmessage = function(e) {
 console.log( e.data, e.data !== null );
};
channel.port1.start();

createImageBitmap( new ImageData(16,16) ).then( bitmap => {

  channel.port2.postMessage(bitmap, [bitmap]);
  console.log( bitmap, bitmap.width === 0 );
});

What is the expected behavior?
ImageBitmap implements transferable object. Should transfer

What went wrong?
e.data is null

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 56.0.2922.1  Channel: dev
OS Version: 
Flash Version: 

https://bugs.chromium.org/p/chromium/issues/detail?id=334408
 
Labels: M-56 Needs-Feedback
Could you please provide a sample html testcase for ease of bisecting?
Cc: junov@chromium.org nhiroki@chromium.org
Components: Blink>Messaging
Labels: -Type-Bug -M-56 OS-All Type-Compat
Status: Available (was: Unconfirmed)

Comment 3 by junov@chromium.org, Nov 28 2016

Owner: junov@chromium.org
Status: Assigned (was: Available)
Bug Confirmed.

Repro URL: https://jsfiddle.net/vjxn1byq/
Look at dev tools console.
junov@, thank you for taking a look at this. I'm not sure the internals of ImageBitmap, but some parts of my fix for ArrayBuffer could be reusable if it's similar to ArrayBuffer:
https://codereview.chromium.org/2414333003

Comment 5 by falken@chromium.org, Nov 29 2016

Labels: -Needs-Feedback

Comment 6 by junov@chromium.org, Nov 29 2016

While debugging this, I am observing that in the messagePort use case, postMessageImpl takes the copy-and-neuter path rather than the move path is there any good reason for this? In this example, the two ports of the channel are in the same realm. Therefore, I would expect move semantics to be just fine.  Am I wrong about that?

If there is a valid reason not to use move semantics for the transfer, then this bug may be sort of a duplicate of 669529.

Comment 7 by mek@chromium.org, Nov 29 2016

The problem with using move semantics for the transfer is that at the time you postMessage something into a MessagePort it is unknown where the message will actually end up. Even if at the time of the postMessage the port on the other side is still in the same realm, the port itself could be posted to a different realm before the message event gets dispatched. So any kind of move-semantic-for-same-realm-messages optimization is going to be fairly complex to get right.

Comment 8 by junov@chromium.org, Nov 29 2016

Thanks for the guidance.  A fix is pending review here: https://codereview.chromium.org/2541683002/

This fix however does not resolve issue for window.postMessage because that one is implemented differently using a custom binding.  I will follow-up on that case in  issue 669529 .
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633

commit 6df1a49e882a63a088bbb2f62fe5a3b30c6d0633
Author: junov <junov@chromium.org>
Date: Wed Dec 07 00:26:39 2016

Make ImageBitmap transfers work in cases that require serialization

Transferring an ImageBitmap object, until now, relied on sharing a
RefPtr with the destination realm in order to pass the pixel data.
This mechanism does not work for all post message use cases. Some
cases require full serialization.  This change fixes the all the
postMessage interfaces that use the PostMessage extended attribute
in the IDL by employing the same strategy that was used for
ArrayBuffer: to fallback to the structured clone code path, and neuter
the source object.  This change does not fix window.postMessage

BUG= 667910 

Review-Url: https://codereview.chromium.org/2541683002
Cr-Commit-Position: refs/heads/master@{#436774}

[add] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/LayoutTests/fast/canvas/canvas-messagechannel-imagebitmap-transfer.html
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/bindings/scripts/v8_methods.py
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/core/dom/MessagePort.h
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/core/workers/DedicatedWorkerGlobalScope.h
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/core/workers/InProcessWorkerBase.h
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerGlobalScope.h
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/modules/serviceworkers/ServiceWorker.h
[modify] https://crrev.com/6df1a49e882a63a088bbb2f62fe5a3b30c6d0633/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerClient.h

Status: Fixed (was: Assigned)
Labels: M-57
 Issue 669529  has been merged into this issue.

Sign in to add a comment