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

Issue 700788 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Show other hotlists

Hotlists containing this issue:
Wasm-M59


Sign in to add a comment

postMessage: v8::Context used at deserialization differs from the one used in script

Project Member Reported by mtrofin@chromium.org, Mar 13 2017

Issue description

Tracking for issue discovered when implementing limitations on structured cloning. When posting to an iframe, the context used at deserialization differs from the context the object ends up being used in.
 
Owner: jbroman@chromium.org
Status: Available (was: Untriaged)
Jeremy, do you have any idea why this may happen? Happy to offload, too.
Owner: mtrofin@chromium.org
Script can certainly choose to do this. At present I believe deserialization of MessageEvent data happens in the current context (i.e. the context in which the data getter function is defined). In ordinary use that should be the context in which the MessageEvent was created and dispatched.

You can, however, pass either the MessageEvent or the deserialized data to some other same-origin context, because you have direct access to such script contexts.
Cc: rossberg@chromium.org
If I understand this correctly, the  current behavior is equivalent to a handle hand-over - it's the same object, in the same context.

Do we want the current behavior, or do we rather want to rehydrate in the destination context?

Added Andreas for a JS language perspective.
Cc: jochen@chromium.org
and +Jochen, maybe he has an insight into this, too (specifically, my last question)

Comment 5 by jochen@chromium.org, Mar 24 2017

Can you provide an example that demonstrates this issue?
See this CL: https://codereview.chromium.org/2749503002/

specifically, third_party/WebKit/LayoutTests/http/tests/wasm/wasm_local_iframe_test.html

Comment 7 by jochen@chromium.org, Mar 24 2017

Ok, and what is the context you expect to be used?
The context of the recipient of the postMessage.

Here's more data: currently, we install a symbol in the native context. We use this symbol for tagging ("branding") wasm module objects. It appears there's a different native context object in the destination frame, compared to the sending one. (see v8/src/wasm/wasm-js.cc, look for wasm_module_sym and also HasBrand).

When we make a new wasm module object as result of deserialization, we do so (as it appears) in the sender's context, meaning that the value of wasm_module_sym is used from that context. When we call instantiate, we check if the wasm_module_sym is present - by comparing with the one we have in the current context (which, now, is the receiver's context). So instantiation fails, because the wasm_module_sym values are different.

I don't know what the right thing is here: should deserialization happen in the destination context, or should wasm_module_sym be rather associated with the isolate - or something else?

Comment 9 by jochen@chromium.org, Mar 24 2017

Are you sure this is what causes the failure? Note that in your test, the message handler you install on the iframe is a function defined in the main frame.

Maybe the module is correctly deserialized in the iframe's context, however, you fail to instantiate it in the context of the main frame.
I'm quite sure this (the difference in contexts) is what causes the failure, and also on the identity of the contexts: I set a breakpoint in the serializer and deserializer and compared the value of the context pointer (and got lucky that there was no gc). Did same at instantiation time. The first 2 were the same values, the latter wasn't (they also have a hash, come to think of it, which made me trust what I saw even more so - i.e. gc or not, I assume the hash is constant for the lifetime of an object).

Now, maybe what you pointed out, that the message handler is defined in the main frame, may be the problem. What's the right way to set this up - any pointers would be great!

Thanks!
In a test like this, of expect the message handler to be defined in a separate file, or via the srcdoc attribute of the iframe.

Another thing that could be happening here is that the data attribute is defined as a lazy accessor in which case there's currently a bug in v8 where we don't change the context correctly before invoking the getter
Interesting - do you know the bug by any chance, if it turns out it's the same issue (and also to see its symptoms and see if they match here).

I'll also do what you suggested with the message handler.

Taking a step back, it sounds like we do believe that the deserialized object should be contextualized in the recipient's context (seemed intuitively desirable to me, otherwise why not just pass the reference when in same isolate - but worth doublechecking); and, in that case, we're doing the right thing in wasm by having per-context definitions of the wasm symbol we use to determine if an object is a wasm module or not.
Cc: yukishiino@chromium.org
I'm not sure I entirely understand was this symbol is used for and what you're trying to achieve here?

Yuki, is there a bug on file for the lazy accessors not entering the context?

It seems I didn't file a bug.  I'll file it next week (sorry, I'm running to the last train now).
To Jochen's question at comment#13: Andreas may provide more authoritative answer. My understanding is that we want to type-check an object ("is it a WebAssembly.Module object"), and do so by tagging it with a value.
You should use an separate instance type for this
Just FYI, I filed a bug at https://crbug.com/v8/6156 .

re: #11, if the property is a *data* property, then it shouldn't be a lazy accessor because it's not an accessor property, I think?

If the property is a data property, then the current context is expected to *not* change because there is no JS function running, i.e. the current context (in C++ callback function) is the caller's context.

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 19 2017

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

commit 16aa23bd2b41c31151e42d612f10d683defaa5f1
Author: mtrofin <mtrofin@chromium.org>
Date: Wed Apr 19 15:17:21 2017

[wasm] Enable local iframe test.

It appears the original reason this bug was disabled does not reproduce
anymore; also fixed unrelated test bug (it's unrelated to the original
reason the test was disabled, because, originally, instantiation wasn't
working full stop because of differences in contexts. Test bug just
wasn't receiving data correctly.)

BUG= chromium:700788 

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

[modify] https://crrev.com/16aa23bd2b41c31151e42d612f10d683defaa5f1/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/16aa23bd2b41c31151e42d612f10d683defaa5f1/third_party/WebKit/LayoutTests/http/tests/wasm/resources/frame.html
[modify] https://crrev.com/16aa23bd2b41c31151e42d612f10d683defaa5f1/third_party/WebKit/LayoutTests/http/tests/wasm/wasm_local_iframe_test.html

Status: WontFix (was: Available)
Issue doesn't repro anymore, and the blocked test could be re-enabled.

Sign in to add a comment