1562667 - Enable SharedArrayBuffer by default on Nightly

Status: RESOLVED FIXED

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

Description

[Anne (:annevk)]

Once we implement the logic needed for bug 1562663, we could ship SharedArrayBuffer by default, even if we don't yet ship COOP/COEP which would allow using it in a threaded/high-resolution timer fashion.

This might be useful for determining the level of breakage with this overall approach.

To restate, the plan is that we have SharedArrayBuffer available at all times, but you cannot postMessage() it without imposing restrictions on yourself (COOP and COEP). Without postMessage() support SharedArrayBuffer is as powerful as ArrayBuffer.

This plan has been coordinated with various standards bodies (e.g., TC39 at https://github.com/tc39/ecma262/issues/1435) and no concerns have been raised thus far over quite a significant period of time.

Comment 1

[Anne (:annevk)]

So we need a better error message for postMessage() failures due to usage of SharedArrayBuffer and Webassembly.Module.

Suggestion:

The {X} object cannot be serialized The Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy HTTP headers will enable this in the future.

(This will also help those that raised and commented in bug 1586217.)

Comment 2

[Tom Tung [:tt, :ttung]]

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

So we need a better error message for postMessage() failures due to usage of SharedArrayBuffer and Webassembly.Module.

Suggestion:

The {X} object cannot be serialized The Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy HTTP headers will enable this in the future.

(This will also help those that raised and commented in bug 1586217.)

Just to confirm, I assume that we want to return this error message while returning DOM_DATA_CLONE_ERROR [1]. Is that correct?

I ask that because I found Bug 1587007 and I assume Lars will improve error message for [2].

[1] https://searchfox.org/mozilla-central/rev/7cc0f0e89cb40e43bf5c96906f13d44705401042/dom/base/StructuredCloneHolder.cpp#264
[2] https://searchfox.org/mozilla-central/rev/7cc0f0e89cb40e43bf5c96906f13d44705401042/js/src/vm/StructuredClone.cpp#1251-1252

Comment 3

[Anne (:annevk)]

The exception that's thrown from postMessage() due to the COOP/COEP bit not being set should have the above proposal as its message. https://groups.google.com/d/msg/mozilla.dev.platform/9cRww2hlScY/XsgsjtbfBAAJ has instructions for which API to use.

Bug 1587007 seems like a duplicate of this bug, but perhaps we should fix it there to keep this bug purely for landing the preference flip.

Comment 4

[Lars T Hansen [:lth]]

Yeah, this bug is pretty meta, let's fix error messages on bug 1587007.

Comment 5

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1562667 - P4a - Enable SAB by default on nightly;

Comment 6

[Tom Tung [:tt, :ttung]]

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

This patch does:

• Rename CanShareMemory to IsCrossOriginIsolated
• Only check COOP and COEP on the serializing side
• Check if the caller AgentClusterId is same as target on the deserializng side

Note that this patch assumes that it's safe to not throw for the case that the
target window is navigated to another origin but in the same agent cluster while
the MessageEvent is in-flight.

Comment 7

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1562667 - P2 - Expect ok for one more wpt test;

Depends on D50198

Comment 8

[Tom Tung [:tt, :ttung]]

try for P1 + P2 https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6e3c1d1fb59ac8bd85fa3781197a56097b7a338

Comment 9

[Tom Tung [:tt, :ttung]]

try for P1+P2+P3 https://treeherder.mozilla.org/#/jobs?repo=try&revision=011ae94860c6943ee4665acc0d6528fb8c33ca5b

Comment 10

[Tom Tung [:tt, :ttung]]

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

try for P1 + P2 https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6e3c1d1fb59ac8bd85fa3781197a56097b7a338

After addressing comment for P1
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1142ed55fa11692093c3becfb03577399ab4143e

Comment 11

[Tom Tung [:tt, :ttung]]

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

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

try for P1 + P2 https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6e3c1d1fb59ac8bd85fa3781197a56097b7a338

After addressing comment for P1
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1142ed55fa11692093c3becfb03577399ab4143e

It seems the change for checking remoteType makes wpt test on Android fails. I'm checking more info on try.
Try without the remoteType check: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c3aed6ef80556e14c00ac8c79ac653067351b01
Try with the check and printing some log: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f25ca34f9ca59c69cdd15bd03c3fb6b3b74433ba

If it's related to, then I will probably extract the code for checking remote type out to another patch and land the part without it.

Comment 12

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1562667 - P3 - Check remoteType in IsCrossOriginIsolated;

Depends on D50444

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5bf8a9b97c92
P1 - Refine isCrossOriginIsolated check and agentcluster check for postmessage; r=nika
https://hg.mozilla.org/integration/autoland/rev/f67361bfb377
P2 - Expect ok for one more wpt test; r=nika

Comment 14

[Tom Tung [:tt, :ttung]]

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

It seems the change for checking remoteType makes wpt test on Android fails. I'm checking more info on try.
Try without the remoteType check: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c3aed6ef80556e14c00ac8c79ac653067351b01
Try with the check and printing some log: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f25ca34f9ca59c69cdd15bd03c3fb6b3b74433ba

Nika, the try result shows that it hit the assertion for checking remoteType on try in IsCrossOriginIsloated(). I'm not really familiar with the behavior remoteType and contentChild on Android (Gecko view). So, I'm printing more information to understanding what's going on, but maybe you have some idea of that?

Comment 15

[Tom Tung [:tt, :ttung]]

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

Nika, the try result shows that it hit the assertion for checking remoteType on try in IsCrossOriginIsloated(). I'm not really familiar with the behavior remoteType and contentChild on Android (Gecko view). So, I'm printing more information to understanding what's going on, but maybe you have some idea of that?

It seems that Fenix is not mulit e10s (one content process) so this is expected to fail on Andriod. I will disable to test on Andriod.

Comment 16

[Tom Tung [:tt, :ttung]]

try for P3: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0762973920d09f1eb7e426419f82e278b442714c

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

Comment 17

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5bf8a9b97c92
https://hg.mozilla.org/mozilla-central/rev/f67361bfb377

Comment 18

[Tom Tung [:tt, :ttung]]

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

try for P1+P2+P3 https://treeherder.mozilla.org/#/jobs?repo=try&revision=011ae94860c6943ee4665acc0d6528fb8c33ca5b

The remaining work is to expect some tests pass in this try

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c6b97228f0a
P3 - Check remoteType in IsCrossOriginIsolated; r=nika

Comment 20

[Tom Tung [:tt, :ttung]]

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

I still need to update some tests in SM.

Comment 21

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9c6b97228f0a

Comment 22

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5bf8a9b97c92
https://hg.mozilla.org/integration/mozilla-inbound/rev/f67361bfb377

Comment 23

[Tom Tung [:tt, :ttung]]

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

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

I still need to update some tests in SM.

Remaining wpt tests is webaudio/the-audio-api/the-audiobuffer-interface/audiobuffer-copy-channel.html and encoding/streams/decode-utf8.any.sharedworker.html

Comment 24

[Tom Tung [:tt, :ttung]]

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

Note: there should still have jit tests failures, but I assume fixing them are easier (because I can just disable a whole test rather than like wpt test I need to disable sub-test one by one)

Comment 25

[Tom Tung [:tt, :ttung]]

lastest try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3b6e046eda6661a404a55c7cdeb75412dbbb4d0&selectedJob=277006660

Somehow failure jit tests can pass without any modification on my Mac. So this try is to verify if I don't need to make any further change on these jit tests. If they are still failing, then I will update a patch with disabling all of them.

Comment 26

[Tom Tung [:tt, :ttung]]

One more try for wpt tests https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b8ee1cdbd0dc0858d9bff4924d5dc14fbbb94f3

Comment 27

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1562667 - P4b - Disable jsref test for Atomic on Nightly;

After enabling the perf for SAB by default on Nightly, some jsreftests fail.
And they all have log like:
tests/test262/shell.js:492: Error: Agents not available item 1

It seems that there are some issues on test262 shell, so the plan of this patch
is to disable these tests and open another follow-up ticket to re-enable them.
So that we could check the impact of enable the perf for SAB without enabling
the functionality of being postMessage'ed on Nightly users.

Depends on D48838

Comment 28

[Tom Tung [:tt, :ttung]]

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

Comment 29

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1562667 - P4c - Revert the check for SAB for a js chrome test on Nightly;

Depends on D53966

Comment 30

[Tom Tung [:tt, :ttung]]

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

Comment 31

[Tom Tung [:tt, :ttung]]

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

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/73f35057a2b2
P4a - Enable SAB by default on nightly; r=bzbarsky,luke
https://hg.mozilla.org/integration/autoland/rev/5337448b7a79
P4b - Disable jsref test for Atomic on Nightly; r=lth,jorendorff
https://hg.mozilla.org/integration/autoland/rev/2e5253665a3a
P4c - Revert the check for SAB for a js chrome test on Nightly; r=lth

Comment 33

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/73f35057a2b2
https://hg.mozilla.org/mozilla-central/rev/5337448b7a79
https://hg.mozilla.org/mozilla-central/rev/2e5253665a3a

Comment 34

[Florian Scholz (Open Web Docs)]

See https://github.com/mdn/sprints/issues/2219 for docs