Closed Bug 1309861 Opened 5 years ago Closed 5 years ago

Enable SharedArrayBuffer and Atomics by default on Developer Edition

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: lth, Assigned: lth)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

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.
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+
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?
https://hg.mozilla.org/mozilla-central/rev/22293babd8a6
Status: ASSIGNED → RESOLVED
Closed: 5 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)
OK, will attend to this ASAP and offer a modified patch for uplift.
Flags: needinfo?(lhansen)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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+
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?
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
(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.
https://hg.mozilla.org/mozilla-central/rev/d41feccb93b2
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Attachment #8804170 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.