Closed Bug 1321773 Opened 3 years ago Closed 3 years ago

Perma failures when aurora will move to beta in | test_serviceworker_interfaces.html | false: If this is failing: DANGER, are you sure you want to expose the new interface SharedArrayBuffer to all webpages as a property on the service worker?

Categories

(Core :: JavaScript Engine, defect, blocker)

defect
Not set
blocker

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox52 blocking fixed
firefox53 --- fixed

People

(Reporter: cbook, Assigned: lth)

Details

Attachments

(1 file)

from uplift simulations (aurora as beta) https://treeherder.mozilla.org/logviewer.html#?job_id=31875953&repo=try

 10:08:11 INFO - TEST-UNEXPECTED-FAIL | dom/workers/test/serviceworkers/test_serviceworker_interfaces.html | false: If this is failing: DANGER, are you sure you want to expose the new interface SharedArrayBuffer to all webpages as a property on the service worker? Do not make a change to this file without a review from a DOM peer for that specific change!!! (or a JS peer for changes to ecmaGlobals) 

lars do you know whats going on here (or do we just need to uplift a fix ?
Flags: needinfo?(lhansen)
Oh, this is weird.  I distinctly thought "release" meant "release or beta", there was an announcement about that recently, and I don't know why that shouldn't be the case here.

What we're looking at is test_serviceworker_interfaces.js, where we see:

  {name: "SharedArrayBuffer", release: false},

the intent of which is to say that this is enabled in nightly and aurora but not (yet) in beta or release.

After Kyle left I don't know who is mr Worker any longer, let's ask Andrea for guidance.
Flags: needinfo?(lhansen) → needinfo?(amarchesini)
This is not about workers. We have the same error in dom/tests/mochitest/general/test_interfaces.html:

https://treeherder.mozilla.org/logviewer.html#?job_id=31875952&repo=try#L116-L38728

10:31:14 INFO - TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_interfaces.html | If this is failing: DANGER, are you sure you want to expose the new interface SharedArrayBuffer to all webpages as a property on the window (XBL: false)? Do not make a change to this file without a review from a DOM peer for that specific change!!! (or a JS peer for changes to ecmaGlobals) 

It seems that we are exposing SharedArrayBuffer to beta somehow.
Flags: needinfo?(amarchesini)
[Tracking Requested - why for this release]:

request tracking since this might be a potential tree closing bug when aurora moves to beta 

Lars, Baku: any idea who could take a look ?
Severity: normal → blocker
Flags: needinfo?(lhansen)
Flags: needinfo?(amarchesini)
I'll investigate more.
Flags: needinfo?(lhansen)
tracking as a blocking issue since this might cause tree closure on merge day
Flags: needinfo?(amarchesini)
This is a missing guard in moz.build, almost certainly my fault.  Experimenting with a fix now.
Make that js/src/moz.build, duh.
It's actually a simpler fix: RELEASE_BUILD used to have the meaning "release or beta" but now means only "release".  For "release or beta" we must use "RELEASE_OR_BETA" in modules/libpref/init/all.js when deciding on the value for the javascript.options.shared_memory.

See mail from Sebastian Hengst to dev-platform on Oct 8 2016 for more information.

My patch landed on Oct 13 and got the guard wrong, or alternatively, I should have updated the test cases.  Let's fix the guard for now.
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Patch for both central and aurora.  See comment 8 for further information.
Attachment #8825403 - Flags: review?(shu)
Attachment #8825403 - Flags: review?(shu) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8aeabea13c128c5741632adf562d3445e4412c9c
Bug 1321773 - SAB+Atomics are off by default in both release and beta. r=shu
Comment on attachment 8825403 [details] [diff] [review]
bug1321773-correct-ifdef-for-sab-switch.patch

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1309861 was intended to flip the flag to true on DevEd but ended up flipping it on DevEd and Beta because it overlapped the change that altered the meaning of the RELEASE_BUILD define.

[User impact if declined]:
The API will be exposed by default on Beta.

[Is this code covered by automated tests?]:
Yes, see comment 0.

[Has the fix been verified in Nightly?]:
Has landed now, but Nightly will experience no change, so really it can only be verified when aurora is built as beta.

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

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

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Replaces an ifdef that is now clearly wrong with one that is clearly right, given what the intent has been all along.

[String changes made/needed]:
None.
Attachment #8825403 - Flags: approval-mozilla-aurora?
Comment on attachment 8825403 [details] [diff] [review]
bug1321773-correct-ifdef-for-sab-switch.patch

properly disable a pref on beta to avoid perma-fail, take in aurora52

Thanks Lars.
Attachment #8825403 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/8aeabea13c12
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.