1589673 - /html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-serviceworker-failure.https.html - Test timed out

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect, P2)

Comment 1

[Tom Tung [:tt, :ttung]]

We should check agent cluster id while deserializing.

Comment 2

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1589673 - Refine isCrossOriginIsolated check and agentcluster check for postmessage;

Comment 3

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1589673 - Expect to pass the wpt test;

Depends on D50198

Comment 4

[Tom Tung [:tt, :ttung]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e179f2d5735a46c5624530d14b640b6467e60125

Comment 5

[Tom Tung [:tt, :ttung]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=44eca9a19e6f9421434c02ce78059b5aae8f0461

Comment 6

[Tom Tung [:tt, :ttung]]

So, the reason for not passing this wpt test is that we haven't supported ServiceWorker::PostMessage(). So it would just throw an DATA_CLONE_ERROR on the caller side because the default cloneDataPolicy is "deny".

The main reason for not supporting is that it always tunnel the serialized data to the parent process and then dispatch to all the targeting children process. That just like implementation for MessagePort and BrocastChannel, so I guess we probably won't deal with these in the short term.

The list which we haven't support the IsCrossOriginIsolated and same agent cluster check for postMessage:

• BroadcastChannel::PostMessage() : https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/dom/webidl/BroadcastChannel.webidl#18
• MessagePort::PostMessage(): https://searchfox.org/mozilla-central/source/dom/webidl/MessagePort.webidl#13
• ServiceWorker::PostMessage() : https://searchfox.org/mozilla-central/source/dom/webidl/ServiceWorker.webidl#23
• SharedWorker::PostMessage() : (I somehow couldn't find the webidl for this)
• Client::PostMessage() : https://searchfox.org/mozilla-central/source/dom/webidl/Client.webidl#26
• (CrossOrigin Window.postMessage) BrowsingContext::PostMessageMoz(): https://searchfox.org/mozilla-central/source/dom/webidl/Window.webidl#90

Note: for "not support" here, I refer to always throw on the caller side when there is a SAB rather than allow SAB to be serialized.

Comment 7

[Tom Tung [:tt, :ttung]]

Nika and Anne:

Is there any of item in the list that you think we need to support (making them be able to postMessage the SAB) before shipping the reSAB? If not, I will probably just file a follow-up bug for them.

(In reply to Tom Tung [:tt, :ttung] from comment #6)

The list which we haven't support the IsCrossOriginIsolated and same agent cluster check for postMessage:

• BroadcastChannel::PostMessage() : https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/dom/webidl/BroadcastChannel.webidl#18
• MessagePort::PostMessage(): https://searchfox.org/mozilla-central/source/dom/webidl/MessagePort.webidl#13
• ServiceWorker::PostMessage() : https://searchfox.org/mozilla-central/source/dom/webidl/ServiceWorker.webidl#23
• SharedWorker::PostMessage() : (I somehow couldn't find the webidl for this)
• Client::PostMessage() : https://searchfox.org/mozilla-central/source/dom/webidl/Client.webidl#26
• (CrossOrigin Window.postMessage) BrowsingContext::PostMessageMoz(): https://searchfox.org/mozilla-central/source/dom/webidl/Window.webidl#90

The main reason for not supporting them is that they all:

• Serialize data
• Send to the parent process
• Send to all the targeting child processes.
• Deserialize
  I'm not so sure if it's okay to serialize a shared memory and then deserialized it after dispatching it to other process and back. (If it's safe to do that, maybe I can quickly make a patch for them). If it's not safe, then I guess bigger refactoring stuff is needed and maybe it's not okay to do that right now?

Comment 8

[Phabricator Automation]

Comment on attachment 9103515 [details]
Bug 1589673 - Refine isCrossOriginIsolated check and agentcluster check for postmessage;

Revision D50198 was moved to bug 1562667. Setting attachment 9103515 [details] to obsolete.

Comment 9

[Anne (:annevk)]

Tom, that's tracked by bug 1360434 as follow-up work for resab already. If we can make it work that'd be great, but it's not a priority.

I'll also try to answer your question. When you serialize SAB to a record, you basically store a reference to the agent cluster and the SAB within that agent cluster in the record. It's fine for that information to go into other processes. However, only if that information returns to the process that serialized it can it be successfully deserialized as the agent cluster needs to match first and then within that agent cluster the SAB needs to be returned.

Lars did point out at one point that this creates challenges for GC as the moment you serialize a SAB you don't know when you can collect the SAB's underlying data. Given how we limit SABs to a single tab a permanent leak that's the responsibility of the site might be acceptable here however.

Appreciate your attention to detail!

Comment 10

[Tom Tung [:tt, :ttung]]

I don't think I will actively work on this recently (in a month) since it needs bug 1360434 to be fixed. Drop the assignee for now.

Comment 11

[Anne (:annevk)]

See also bug 1612694. The remaining issue here is no COEP support for shared/service workers, but the test is no longer intermittent.