Enable SharedArrayBuffer and Atomics by default on Developer Edition

RESOLVED FIXED in Firefox 51

Status

()

defect
P1
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: lth, Assigned: lth)

Tracking

({dev-doc-complete})

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed, firefox52 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

3 years ago
Enabling on Dev Edition is a reasonable first step to test webcompat concerns created by introducing two new top-level names ('Atomics' and 'SharedArrayBuffer').  No problems are expected, and none have been reported in the many months it's been enabled by default on Nightly.
Assignee

Comment 1

3 years ago
Switch the default setting for SAB+Atomics enablement from false to true for DevEd / Aurora (as well as for Nightly, where that was already the case).
Attachment #8800616 - Flags: review?(jorendorff)
Attachment #8800616 - Flags: review?(jorendorff) → review+
Assignee

Comment 3

3 years ago
Comment on attachment 8800616 [details] [diff] [review]
bug1309861-sab-enable-on-deved.patch

Approval Request Comment

[Feature/regressing bug #]:
Shared memory feature will be enabled by default in FF51 (pending final security review) and we would like to enable it by default for Aurora right now, this should have happened months ago but was overlooked.

[User impact if declined]:
Feature must be enabled manually in about config; content will not work automatically.

[Describe test coverage new/current, TreeHerder]:
Has been enabled on Nightly for a long time and many tests exist already in the repo.

[Risks and why]: 
Low risk resulting from introducing two new top-level names in the JS globals, "Atomics" and "SharedArrayBuffer".  No problems have been discovered after having these names in nightly for over a year.

[String/UUID change made/needed]:
None, I think; this just flips a pref default from false to true on Aurora.
Attachment #8800616 - Flags: approval-mozilla-aurora?

Comment 4

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/22293babd8a6
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8800616 [details] [diff] [review]
bug1309861-sab-enable-on-deved.patch

Enable shared memory feature in 51 aurora by default. Aurora 51+.
Attachment #8800616 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I had to back this out from aurora in https://hg.mozilla.org/releases/mozilla-aurora/rev/f4283f32adb12d2bc35d055fccabd737b90feba6 because tests are failing due to the apparent lack of dom peer review on enabling a new interface:
https://treeherder.mozilla.org/logviewer.html#?job_id=3951991&repo=mozilla-aurora
Flags: needinfo?(lhansen)
Assignee

Comment 8

3 years ago
OK, will attend to this ASAP and offer a modified patch for uplift.
Flags: needinfo?(lhansen)
Assignee

Updated

3 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 9

3 years ago
Whitelist SharedArrayBuffer and Atomics APIs for aurora in Mochitests.

According to the file, this requires JS peer review, not DOM peer review.
Attachment #8804170 - Flags: review?(hv1989)
Attachment #8804170 - Flags: review?(hv1989) → review+
Assignee

Comment 11

3 years ago
Comment on attachment 8804170 [details] [diff] [review]
bug1309861-whitelist-sab-atomics.patch

Approval Request Comment

See Comment #3 for justification.  This patch contains additional functionality that is needed to pass the test suite, which guards against new APIs being introduced accidentally.

Note that the other patch also has to be relanded since it was backed out.
Attachment #8804170 - Flags: approval-mozilla-aurora?
Assignee

Updated

3 years ago
Priority: -- → P1
(In reply to Lars T Hansen [:lth] from comment #11)
> Comment on attachment 8804170 [details] [diff] [review]
> bug1309861-whitelist-sab-atomics.patch
> 
> Approval Request Comment
> 
> See Comment #3 for justification.  This patch contains additional
> functionality that is needed to pass the test suite, which guards against
> new APIs being introduced accidentally.
> 
> Note that the other patch also has to be relanded since it was backed out.

note: we need this also since this will cause otherwise issues on merge day like bug 1312757
Assignee

Comment 13

3 years ago
(In reply to Carsten Book [:Tomcat] from comment #12)
> (In reply to Lars T Hansen [:lth] from comment #11)
> > Comment on attachment 8804170 [details] [diff] [review]
> > bug1309861-whitelist-sab-atomics.patch
> > 
> > Approval Request Comment
> > 
> > See Comment #3 for justification.  This patch contains additional
> > functionality that is needed to pass the test suite, which guards against
> > new APIs being introduced accidentally.
> > 
> > Note that the other patch also has to be relanded since it was backed out.
> 
> note: we need this also since this will cause otherwise issues on merge day
> like bug 1312757

Right.  Note this has landed on m-i already I figured it would just get merged with the pref change, on branch day.

Comment 14

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d41feccb93b2
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
Attachment #8804170 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 17

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76d7f4eb6100
Update the dom/workers interfaces test expectations as well. r=bz
You need to log in before you can comment on or make changes to this bug.