Closed Bug 1354160 Opened 3 years ago Closed 3 years ago

Disable SharedArrayBuffer and Atomics on FF53

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox53 + fixed

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(1 file)

This patch may or may not land but I need a review on it in case it needs to land; decision to be made at the executive level.

The patch is for mozilla-beta (only).  It is effectively the same patch as the one that disabled this feature on FF52, bug 1342095.

Try build coming up.
Attachment #8855326 - Flags: review?(shu)
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Comment on attachment 8855326 [details] [diff] [review]
disable-sab-and-atomics-on-53beta.patch

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

I haven't been paying attention to this -- is Chrome going to miss their next release?
Attachment #8855326 - Flags: review?(shu) → review+
The word is we're shipping this.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Per the email thread.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
We are going to hold off and plan for release in 54 in mid-June.
n-i on myself to make sure we land this before Monday (I'm holding off on approvals because of the issues right now with beta 10)
Flags: needinfo?(lhenry)
See Also: → 1352681
Comment on attachment 8855326 [details] [diff] [review]
disable-sab-and-atomics-on-53beta.patch

From discussion on release-drivers we need to disable this feature for 53.
Flags: needinfo?(lhenry)
Attachment #8855326 - Flags: approval-mozilla-beta+
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Lars, is this enabled by default in 54? 
We probably don't want it to be, judging by the open bugs for SharedArrayBuffer (https://bugzilla.mozilla.org/showdependencytree.cgi?id=1054841&hide_resolved=1).
Flags: needinfo?(lhansen)
Flags: needinfo?(gchang)
Flags: needinfo?(sdeckelmann)
Flags: needinfo?(nihsanullah)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #9)
> Lars, is this enabled by default in 54? 

Yes: https://dxr.mozilla.org/mozilla-beta/source/modules/libpref/init/all.js#1332

> We probably don't want it to be, judging by the open bugs for
> SharedArrayBuffer
> (https://bugzilla.mozilla.org/showdependencytree.
> cgi?id=1054841&hide_resolved=1).

No, that's not true.  Most of those bugs are all-but-resolved and/or corner cases.  For the remaining, the situation is that the DOM side of things is currently evolving rapidly and nobody else is implementing anything quite like the current spec, but they're shipping anyway, as should we.  Everyone is in agreement on some basic functionality (being able to share to/from deidcated workers with postMessage, esp from regular HTML documents) and their APIs, and have implemented that.

I'm working on an intent-to-ship but I'm blocked waiting on some information that goes into it, I'll try to unblock that today...
Flags: needinfo?(lhansen)
OK, my concerns are simply that we disabled this during 52 beta, then again for 53 beta. . I don't know why Martin decided not to ship in 52. For 53, there was a last minute security issue, which is now fixed, but in discussion on release-drivers list, several people recommended we wait to ship in 54. 

If we're ready to ship now, that's great.
Handling remaining details via "Intent to Ship" document (https://docs.google.com/a/mozilla.com/document/d/1JPjNo70Xy_-jC18TOFXbPWujq51nOkrZeEzJKh0e6yE/edit?usp=sharing)
Flags: needinfo?(nihsanullah)
For everyone listening in:

- the intent-to-ship has gone out
- we're shipping in Fx55, not Fx54
- the bug to disable in Fx54 is bug 1361520 and a patch will appear there shortly

(ni :lizzard for maximum visibility only)
Flags: needinfo?(lhenry)
Flags: needinfo?(gchang)
Flags: needinfo?(sdeckelmann)
You need to log in before you can comment on or make changes to this bug.