1605176 - Clarify the remote type for cross origin/site iframe if both COOP and COEP headers are set

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, task, P1)

Description

[Tom Tung [:tt, :ttung]]

The original question:
Foo.com

• Boo.com

If both COOP and COEP are set, which remote type of the process for Boo.com should be?

I guess maybe it's a question about:
Foo.com

• [iframe] Boo.com
  • [Nested iframe] Boo.com

Can we allow iframe Boo.com to postMessage SAB to its nested iframe Boo.com?

Comment 1

[Tom Tung [:tt, :ttung]]

Attachment: Disable a wpt test and figure out the solution on Bug 1605176;

Comment 2

[Phabricator Automation]

Comment on attachment 9117190 [details]
Disable a wpt test and figure out the solution on Bug 1605176;

Revision D57945 was moved to bug 1594748. Setting attachment 9117190 [details] to obsolete.

Comment 3

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1605176 - Allow SAB to be serialized in BrowsingContext::PostMessageMoz();

Serialize SAB on BrowsingContext::PostMessageMoz was not allowed. And, this
patch fixes that. Now, SAB is able to be serialized if the callerWindow passes
the check for CrossOriginIsolated. Note that the deserialization for this case
is still not allowed because the caller window and target windows are assumed
to be in the two different agent cluster and thus the message event should
throw on the target window.

Comment 4

[Anne (:annevk)]

If Foo.com nests Boo.com they are cross-site so cannot have shared memory. In the future we might even put them in different processes, but that's not required today.

If Foo.com nests Boo.com and that nests Boo.com, then Foo.com is one agent that the two instances of Boo.com are another. The two instances of Boo.com will be able to share memory. The two agents in this setup (Foo.com and Boo.com) might end up in different processes, but that's not required today, nor the plan.

Does that help?

Comment 5

[Tom Tung [:tt, :ttung]]

(In reply to Anne (:annevk) from comment #4)

If Foo.com nests Boo.com and that nests Boo.com, then Foo.com is one agent that the two instances of Boo.com are another. The two instances of Boo.com will be able to share memory. The two agents in this setup (Foo.com and Boo.com) might end up in different processes, but that's not required today, nor the plan.

Does that help?

Yeah, that helps. Thanks for clarification!

So we have already ensured that Foo.com and Boo.com be not able to share memory, but the first nested Boo.com cannot share memory with its nested Boo.com.

I found this while attaching the patch in this bug (I hit the assertion for ensure the process remote type of the inner-most Boo.com iframe be the webCOOP+COEP). I was thinking the assertion is correct and thus the inner-most iframe should live in the webCOOP+COEP process so that it could share its memory with its parent iframe (another Boo.com iframe).

However, if I read your reply correctly, it's not required (through two instances of Boo.com should be able to share memory) so I would just probably change the the assertion to ignore the cases like this.

Comment 6

[Anne (:annevk)]

Are you saying that with Fission, when there's multiple content processes for a single browsing context group, we use a COOP+COEP process for the parent document, but a different process type for the child? I think that would be a bug. With out-of-process iframes we should still use COOP+COEP processes for them.

Comment 7

[Tom Tung [:tt, :ttung]]

(In reply to Anne (:annevk) from comment #6)

Are you saying that with Fission, when there's multiple content processes for a single browsing context group, we use a COOP+COEP process for the parent document, but a different process type for the child? I think that would be a bug. With out-of-process iframes we should still use COOP+COEP processes for them.

Yes, to avoid confusion, I list what I found below:

// Assuming all headers are set
 
Window A (foo.com) // top-level window
- Window B (foo.com)

Window A and window B are in the same COOP+COEP process and that is expected.

However, for the case like this:

// Assuming all headers are set
 
Window A (foo.com) // top-level window
- Window B (boo.com)
  - Window C (boo.com)

Either window B or window C are not in the COOP+COEP process. (To be more clear, Window A is in the COOP+COEP process) The assertion I mentioned caught the issue. If we only bypass that assertion, Window B and Window C cannot share memory.

Comment 8

[Anne (:annevk)]

Thanks for the clarification. For the A/B/C case 1) B and C have to be in the same process as they can access each other synchronously through script. 2) They have to be in a COOP+COEP process. That could be a distinct process instance from A though.

Pre-Fission and on systems were it is not feasible it's acceptable for B and C to share a process with A and that's our shipping plan, but it's not ideal and long term they should have distinct processes.

Comment 9

[Tom Tung [:tt, :ttung]]

(I will get back to this once I finished bug 1601594)

Comment 10

[Chris Peterson [:cpeterson]]

Tracking for Fission Nightly (M6)

Comment 11

[Tom Tung [:tt, :ttung]]

So, now for the testing/web-platform/tests/html/infrastructure/safe-passing-of-structured-data/shared-array-buffers/window-failure.https.html

• When the fission is not enabled this test would timeout (I changed expectation in bug 1594748)
  What I got while running the test was:

 0:45.29 pid:23753 [Child 23989, Main Thread] WARNING: Failed to clone data.: file /home/shes050117/Work/mozilla-central/dom/base/StructuredCloneHolder.cpp, line 154
 0:45.29 pid:23753 [Child 23989, Main Thread] WARNING: 'rv.Failed()', file /home/shes050117/Work/mozilla-central/dom/base/PostMessageEvent.cpp, line 196

After talking with Anne, I wonder if we want to remove only passing isSharedMemoryAllowed when the agentclusters are matching while serailizing. (Because it would cause serailizing fail when the agent cluster mismatch). And removing that is fine because :baku added clonePolicy.allowIntraClusterClonableSharedObjects(); to prevent reading the content while deserailizing.

:baku does this change sound reasonable to you?

• When the fission is enabled the test would fail because in the iframe the window.crossOriginIsolated is false.

I will figure out why, but I guess it's either because it's running in a wrong process or just getting a wrong value for COOP and COEP

Comment 12

[Tom Tung [:tt, :ttung]]

Sorry for the noise. I found the expection is for fission. Please ignore my comment for the case that fission is not enabled

Comment 13

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1605176 - Run a cross site origin iframe in webCOOP+COEP process if the COOP and COEP are set;

We get the remote type DocumentLoadListener::MaybeTriggerProcessSwitch().
In e10sUtils->GetRemoteTypeForPrincipal, a remote type is prepended with
the webCOOP+COEP prefix if a frame has the same value for site origin as the
current (parent window's) remote type.

However, this wpt tests that a cross site iframe that set valid COEP and
postMessage a SharedArrayBuffer back to its parnet that running in COOP+COEP
environment. It fails because the iframe is not running in the process with the
web_COOP+COEP prefix.

To fix this, this patch assumes that the iframe doesn't have an wrong COOP and
COEP values. (Otherwise, it should fail while checking these values in the http
channel.) And thus, if the prefer remote type has been prepended with the
prefix, we set the same prefix for the frame as well.

Comment 14

[Tom Tung [:tt, :ttung]]

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

• When the fission is enabled the test would fail because in the iframe the window.crossOriginIsolated is false.

I will figure out why, but I guess it's either because it's running in a wrong process or just getting a wrong value for COOP and COEP

It's because of here
For the non-same-origin iframe: the remote type will be changed
For the same-origin iframe, the remote type will be the same

Comment 15

[Tom Tung [:tt, :ttung]]

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

Comment 16

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1605176 - Send a error message data and cause a message error on the receiver side when the message data contains a shared memory object in BrowsingContext::PostMessageMoz;

Comment 17

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1605176 - Update the expectation for a wpt test;

Depends on D75035

Comment 18

[Tom Tung [:tt, :ttung]]

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

Comment 19

[Tom Tung [:tt, :ttung]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=017bf3b3bfa5a88260a5af3a9c8090bb4d16e503&selectedTaskRun=TjmIg3u9SMOpX2koooLEyQ-0

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf1aa383a8fe
Run a cross site origin iframe in webCOOP+COEP process if the COOP and COEP are set; r=nika
https://hg.mozilla.org/integration/autoland/rev/9aa5ee2e3931
Send a error message data and cause a message error on the receiver side when the message data contains a shared memory object in BrowsingContext::PostMessageMoz; r=baku,kmag
https://hg.mozilla.org/integration/autoland/rev/61141a5f9c11
Update the expectation for a wpt test; r=nika

Comment 21

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/bf1aa383a8fe
https://hg.mozilla.org/mozilla-central/rev/9aa5ee2e3931
https://hg.mozilla.org/mozilla-central/rev/61141a5f9c11