New issue
Advanced search Search tips

Issue 798572 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 746486



Sign in to add a comment

SerializedScriptValue should know about agent clusters

Project Member Reported by mek@chromium.org, Jan 2 2018

Issue description

Several aspects of serializing/deserializing data should behave differently depending on if the source and target agent clusters are the same. To do this SSV should keep track of the source agent cluster and use that knowledge during deserialization.

For now I'm adding workarounds in MessagePort to make sure to strip out some data when source/target are for sure in different agent clusters, but those checks aren't perfect (and can be removed when SSV is fixed to take agent cluster into account).
 

Comment 1 by mek@chromium.org, Jan 3 2018

(note that I ended up not actually adding those workarounds in MessagePort, since it felt like doing more harm than good. So for now nothing is blocking things from being transferred across agent clusters as long as everything stays in the same process).
Cc: bradnelson@chromium.org binji@chromium.org
Owner: malcolmwhite@chromium.org
Status: Started (was: Available)
I'm hoping it will be possible to think of an Agent Cluster as a tree of (Window/Worker) -> (same-origin Window/DedicatedWorker/iframes they created), with a primogenitor Window at the root. If this model is valid, then we can take advantage of the DOMWindow's opener() and parent() methods to walk to the same-origin root of an Agent Cluster on the sending side, pass an identifier for the root in the SSV, and check that the receiving side shares the root during deserialization (with some additional logic e.g. "is this a shared worker?"). 

Can you think of any scenarios that might break the above tree model? or anything simpler? 

Comment 3 by mek@chromium.org, Mar 12 2018

Generally sounds sensible, but a few possible concerns: 

 - I think the tree you need to walk should ignore origins (i.e. two foo.com iframes in a bar.com window should be in the same AgentCluster afaik).

 - You might run into threading issues if you try to walk this tree when the sender is a dedicated worker (as the postMessage/serialization happens on the worker thread, but the tree "lives" on the main thread)?

 - It is possible to set a windows opener to null after the window is opened, to "disown" it. I assume the spec still says the two windows are supposed to be in the same agent cluster after that, but there might be a problem here where it is not clear to me if you'll still be able to access that opener from native code even though it is cleared out on the script side.
@mek regarding origins, I'm having a little trouble parsing the spec examples at https://html.spec.whatwg.org/multipage/webappapis.html#integration-with-the-javascript-agent-cluster-formalism

The iframe examples say that an iframe will/won't be in the same AC as their parent window if the iframe "could/cannot be same origin-domain with" the parent window. I don't see why the spec uses could/cannot instead of is/isn't. This feels funny to ask, but do you know what could and cannot mean in this sense?

Comment 5 by mek@chromium.org, Mar 12 2018

"could be same origin-domain with" means that through manipulation of window.domain on either window they could become same origin-domain (so two different suborigins could set window.domain to the top-level origin and become same origin-domain that way).

And the case I tried to bring up doesn't seem to be in that list of examples since all the examples seem to be between a window and its parent/opener; none are about siblings or other more indirect related windows.
Makes sense, thanks. To handle your sibling example (which does smell like an AC), it seems like we would need to also pass the sender's origin in the SSV?
Another wrinkle -- Windows and DedicatedWorkers are not in the same AC as any SharedWorkers they create, but a SharedWorker is in the same AC as any DedicatedWorker it creates (afaik). That breaks the assumption that the root of the AC tree is a Window, but it doesn't seem like it would be too much trouble to work around.
I put together a quick proposal on how to track ACs (and how to fence in SABs): https://docs.google.com/document/d/1VCaWIjBNOIYqgfvQoikQX2D5Slxm72c08q1RkqWUxQQ/edit?usp=sharing

Comment 9 by binji@chromium.org, Jun 13 2018

Cc: -binji@chromium.org -malcolmwhite@google.com -bradnelson@chromium.org
Owner: binji@chromium.org
Status: Available (was: Started)
Taking ownership, removing people who left the project
Blocking: 746486
Labels: -Pri-3 Pri-1
Upping priority because this is blocking 746486
Status: Assigned (was: Available)
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 6

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

commit 969f886bd1766c2b9a8e5fba26d9c6f1574791af
Author: Ben Smith <binji@chromium.org>
Date: Mon Aug 06 21:01:05 2018

Introduce AgentClusterID to serialized messages

There are restrictions to sharing SharedArrayBuffer (and WebAssembly
Modules) between execution contexts; for example, you cannot share a
SharedArrayBuffer between a Window and the Service Worker it created.
The rules are described here:
https://html.spec.whatwg.org/multipage/webappapis.html#integration-with-the-javascript-agent-cluster-formalism

This defines an "agent" which is similar to an execution context, and
an "agent cluster" which is a collection of agents that can share
between each other.

This CL begins to model some of this behavior by using a
`base::UnguessableToken` as an agent cluster ID that can be queried
from an `ExecutionContext`.

This CL also has the correct behavior when a message is sent to an
agent that is not part of its agent cluster; in that case a
"messageerror" event is sent instead of a "message" event.

Bug: chromium:798572, chromium:714842
Change-Id: Ie70cce4cbd0ebd04d8d270d66f59690caf7f616a
Reviewed-on: https://chromium-review.googlesource.com/1130505
Commit-Queue: Ben Smith <binji@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580984}
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/WebKit/LayoutTests/external/wpt/wasm/resources/service-worker.js
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/WebKit/LayoutTests/external/wpt/wasm/wasm_service_worker_test.https.html
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/common/message_port/cloneable_message_struct_traits.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/common/message_port/cloneable_message_struct_traits.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/public/common/message_port/cloneable_message.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/public/mojom/message_port/message_port.mojom
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/bindings/core/v8/custom/v8_message_event_custom.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/bindings/core/v8/serialization/serialized_script_value.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/dom/document.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/events/message_event.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/events/message_event.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/execution_context/execution_context.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/execution_context/execution_context.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/messaging/blink_cloneable_message.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/messaging/blink_cloneable_message_struct_traits.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/messaging/blink_cloneable_message_struct_traits.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/messaging/blink_transferable_message.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/messaging/message_port.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/testing/null_execution_context.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/workers/dedicated_worker.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/workers/global_scope_creation_params.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/workers/global_scope_creation_params.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/workers/worker_global_scope.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/workers/worker_global_scope.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/core/workers/worklet_global_scope.h
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/modules/broadcastchannel/broadcast_channel.cc
[modify] https://crrev.com/969f886bd1766c2b9a8e5fba26d9c6f1574791af/third_party/blink/renderer/modules/service_worker/service_worker_container.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 27

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

commit fbb650b347fa3a53bfb8202d0ecf722a553da812
Author: Ben Smith <binji@chromium.org>
Date: Mon Aug 27 23:20:54 2018

Add WPT serialization tests for wasm modules

These WPT tests are based on similar tests in
wpt/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/

This also fixes some test failures for the SharedArrayBuffer tests.

Bug: chromium:798572
Change-Id: I50b0edbebe4e97f5358513d248657d2831f48201
Reviewed-on: https://chromium-review.googlesource.com/1175490
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586480}
[modify] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/MSANExpectations
[modify] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/resources/sharedworker-failure.js
[modify] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/OWNERS
[delete] https://crrev.com/ff520a66e86c8e2681a387837a7c4d526bf13ad2/third_party/WebKit/LayoutTests/external/wpt/wasm/resources/frame.html
[delete] https://crrev.com/ff520a66e86c8e2681a387837a7c4d526bf13ad2/third_party/WebKit/LayoutTests/external/wpt/wasm/resources/service-worker.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/broadcastchannel-success-and-failure.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/broadcastchannel-success.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/identity-not-preserved.html
[copy] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/incrementer.wasm
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/nested-worker-success-sharedworker-expected.txt
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/nested-worker-success.any.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/nested-worker-success.any.sharedworker-expected.txt
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/no-transferring.html
[rename] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/blank.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/broadcastchannel-iframe.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/broadcastchannel-sharedworker.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/broadcastchannel-worker.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/create-empty-wasm-module.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/echo-iframe.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/echo-worker.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/incrementer-iframe-domain.sub.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/incrementer-iframe.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/incrementer-popup.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/incrementer-worker-with-channel.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/incrementer-worker.js
[rename] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/incrementer.wasm
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/nested-iframe-1.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/nested-iframe-2.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/nested-iframe-3.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/nested-iframe-4-incrementer.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/serviceworker-failure.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/sharedworker-failure.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/resources/test-incrementer.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/serialization-via-history.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/serialization-via-idb.any.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/serialization-via-notifications-api.any.js
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/window-domain-success.sub.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/window-messagechannel-success.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/window-serviceworker-failure.https.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/window-sharedworker-failure.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/window-similar-but-cross-origin-success.sub.html
[add] https://crrev.com/fbb650b347fa3a53bfb8202d0ecf722a553da812/third_party/WebKit/LayoutTests/external/wpt/wasm/serialization/window-simple-success.html
[delete] https://crrev.com/ff520a66e86c8e2681a387837a7c4d526bf13ad2/third_party/WebKit/LayoutTests/external/wpt/wasm/wasm_local_iframe_test.html
[delete] https://crrev.com/ff520a66e86c8e2681a387837a7c4d526bf13ad2/third_party/WebKit/LayoutTests/external/wpt/wasm/wasm_serialization_tests.html
[delete] https://crrev.com/ff520a66e86c8e2681a387837a7c4d526bf13ad2/third_party/WebKit/LayoutTests/external/wpt/wasm/wasm_serialization_tests.js
[delete] https://crrev.com/ff520a66e86c8e2681a387837a7c4d526bf13ad2/third_party/WebKit/LayoutTests/external/wpt/wasm/wasm_serialization_worker.js
[delete] https://crrev.com/ff520a66e86c8e2681a387837a7c4d526bf13ad2/third_party/WebKit/LayoutTests/external/wpt/wasm/wasm_service_worker_test.https.html

Sign in to add a comment