Last Comment Bug 1312446 - Enable SharedArrayBuffer and Atomics in Firefox 52 Beta and Release
: Enable SharedArrayBuffer and Atomics in Firefox 52 Beta and Release
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 52 Branch
: All All
P1 normal (vote)
: mozilla53
Assigned To: Lars T Hansen [:lth]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 1302036
Blocks: 1225406
  Show dependency treegraph
 
Reported: 2016-10-24 08:39 PDT by Lars T Hansen [:lth]
Modified: 2017-02-17 06:43 PST (History)
9 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
bug1312446-sab-atomics-all-channels.patch (1.35 KB, patch)
2016-10-24 08:45 PDT, Lars T Hansen [:lth]
no flags Details | Diff | Splinter Review
bug1312446-sab-atomics-all-channels-v2.patch (2.06 KB, patch)
2016-10-25 07:15 PDT, Lars T Hansen [:lth]
no flags Details | Diff | Splinter Review
bug1312446-sab-atomics-ff52.patch (4.26 KB, patch)
2016-11-07 07:00 PST, Lars T Hansen [:lth]
no flags Details | Diff | Splinter Review
bug1312446-enable-sab-and-atomics.patch (7.69 KB, patch)
2017-01-19 08:29 PST, Lars T Hansen [:lth]
shu: review+
Details | Diff | Splinter Review
bug1312446-enable-sab-and-atomics-AURORA.patch (7.66 KB, patch)
2017-01-19 08:34 PST, Lars T Hansen [:lth]
jcristau: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Lars T Hansen [:lth] 2016-10-24 08:39:56 PDT
Tentatively this feature is scheduled to ship with FF51.  We're awaiting final security review.

Shared memory and atomics are currently on by default on Nightly and (since bug 1309861) FF51 Developer Edition.  The feature is available, but disabled by default, in Beta and Release.
Comment 1 User image Lars T Hansen [:lth] 2016-10-24 08:45:03 PDT
Created attachment 8803906 [details] [diff] [review]
bug1312446-sab-atomics-all-channels.patch
Comment 2 User image Johnny Stenback (:jst, jst@mozilla.com) 2016-10-24 09:06:08 PDT
We're still working through some final details on how and when to ship this, please hold.
Comment 3 User image Lars T Hansen [:lth] 2016-10-25 07:15:43 PDT
Created attachment 8804253 [details] [diff] [review]
bug1312446-sab-atomics-all-channels-v2.patch

Updated patch.  (Still holding: Not reviewed, and additional requested functionality still not ready.)
Comment 4 User image Lars T Hansen [:lth] 2016-10-27 22:33:39 PDT
Note, some Xrays support is about to land on trunk, see bug 1130988.  This containts tests for specific channels.  When landing this patch on central, those tests must additionally be adapted.
Comment 5 User image Lars T Hansen [:lth] 2016-11-07 07:00:34 PST
Created attachment 8808169 [details] [diff] [review]
bug1312446-sab-atomics-ff52.patch

Patch appropriate for FF52 and later (current mozilla-central).
Comment 6 User image Lars T Hansen [:lth] 2017-01-19 06:46:45 PST
When landing this also remember to update js/src/shell/js.cpp, which has a RELEASE_OR_BETA guard on enabling SAB+Atomics by default in the shell.
Comment 7 User image Lars T Hansen [:lth] 2017-01-19 08:29:01 PST
Created attachment 8828381 [details] [diff] [review]
bug1312446-enable-sab-and-atomics.patch

We want to enable SAB+Atomics on beta for a while to ensure greater exposure to the web -- the feature has been enabled only on Nightly and Aurora so far.  If the ship criteria are not met by February 20 I will back the patch out again.

(This patch for m-c, uplift request and maybe aurora patch to follow.)
Comment 8 User image Lars T Hansen [:lth] 2017-01-19 08:34:59 PST
Created attachment 8828383 [details] [diff] [review]
bug1312446-enable-sab-and-atomics-AURORA.patch

Ditto patch for aurora.
Comment 9 User image Shu-yu Guo [:shu] 2017-01-19 18:35:02 PST
Comment on attachment 8828381 [details] [diff] [review]
bug1312446-enable-sab-and-atomics.patch

Review of attachment 8828381 [details] [diff] [review]:
-----------------------------------------------------------------

Changes themselves look good to me. The commit message says "on beta", so just confirming, this is done by not backporting this patch to release?
Comment 10 User image Lars T Hansen [:lth] 2017-01-19 23:49:03 PST
(In reply to Shu-yu Guo [:shu] from comment #9)
> Comment on attachment 8828381 [details] [diff] [review]
> bug1312446-enable-sab-and-atomics.patch
> 
> Review of attachment 8828381 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Changes themselves look good to me. The commit message says "on beta", so
> just confirming, this is done by not backporting this patch to release?

The commit message does not mention "beta" but the comment submitted with the patch above does.  The thing is, there's no difference between release and beta in the code - features are enabled for both or neither.  So what's happening here (cf my comment above) is that I'm enabling for beta by landing this patch on central and aurora now (aurora becomes beta next week) but not enabling for release by promising (again cf comment) to back it out before beta becomes release, if the release criteria are not met.
Comment 12 User image Lars T Hansen [:lth] 2017-01-20 02:52:52 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d870873f2b76157d57bcff8447140f7c0f7f877
Bug 1312446 - Enable SharedArrayBuffer and Atomics by default. r=shu
Comment 13 User image Lars T Hansen [:lth] 2017-01-20 02:55:58 PST
Comment on attachment 8828383 [details] [diff] [review]
bug1312446-enable-sab-and-atomics-AURORA.patch

Approval Request Comment

[Feature/Bug causing the regression]:
SharedArrayBuffer and Atomics

[User impact if declined]:
Feature will have to be manually enabled on beta, which is the current situation

[Is this code covered by automated tests?]:
Yes

[Has the fix been verified in Nightly?]:
Yes - enabled by default on Nightly (and indeed in Aurora) for months.  Available behind flag on Beta and Release for ditto time.

[Needs manual test from QE? If yes, steps to reproduce]: 
No

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
Not really, and in any case, we want to test web compat, so some risk is required.

[Why is the change risky/not risky?]:
Extremely remote chance that the names of new global JS types clash with names that are used in programs on the web.

[String changes made/needed]:
None
Comment 14 User image Carsten Book [:Tomcat] 2017-01-20 06:38:29 PST
https://hg.mozilla.org/mozilla-central/rev/4d870873f2b7
Comment 15 User image Julien Cristau [:jcristau] 2017-01-20 09:28:39 PST
Comment on attachment 8828383 [details] [diff] [review]
bug1312446-enable-sab-and-atomics-AURORA.patch

keep SharedArrayBuffer and Atomics enabled in beta, aurora52+
Comment 16 User image Ryan VanderMeulen [:RyanVM] 2017-01-20 15:01:52 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/3a090ab6da4a
Comment 18 User image Ryan VanderMeulen [:RyanVM] 2017-01-20 19:59:59 PST
Went a little better with bug 1332576 included.
https://hg.mozilla.org/releases/mozilla-aurora/rev/3209927fdbf9

Note You need to log in before you can comment on or make changes to this bug.