Closed Bug 1208687 Opened 4 years ago Closed 4 years ago

Make sure we're clearing the right queue

Categories

(Core :: DOM: Workers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox42 --- fixed
firefox43 --- fixed
firefox44 --- fixed
firefox-esr38 --- affected

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
See the patch's commit messages
Attachment #8666288 - Flags: review?(ehsan)
Comment on attachment 8666288 [details] [diff] [review]
Patch

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

Thanks for the fix!
Attachment #8666288 - Flags: review?(ehsan) → review+
Unfortunately, mochitest-8 on the b2g emulator is still orange with this patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0938d2a56aba
Blocks: 1204596
This has caused W(4) bustages like this /workers/interfaces/WorkerGlobalScope/close/incoming-message.html | close() and incoming message - expected FAIL

Can you fix or shall I backout?
Flags: needinfo?(khuey)
That's an unexpected pass ... easy to fix.
Flags: needinfo?(khuey)
https://hg.mozilla.org/mozilla-central/rev/d16e42e9f1ba
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Comment on attachment 8666288 [details] [diff] [review]
Patch

We should take this on branches because it fixes a violation of an important invariant in the worker code.  This bug could lead to random instability and crashes.

There are no string or UUID changes in this patch.  The flaw was introduced in bug 914762 in Firefox 29.
Attachment #8666288 - Flags: approval-mozilla-esr38?
Attachment #8666288 - Flags: approval-mozilla-beta?
Attachment #8666288 - Flags: approval-mozilla-aurora?
Kyle, could you fill the uplift template? In particular, we need the risk evaluation (especially for esr)
Thanks
Flags: needinfo?(khuey)
Comment on attachment 8666288 [details] [diff] [review]
Patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Prevents a violation of a key invariant in workers code, which may cause random instability and crashes.
User impact if declined: Random instability
Fix Landed on Version: 44
Risk to taking this patch (and alternatives if risky): Low risk. The alternative is to do nothing and accept a potentially higher crash rate.
String or UUID changes made by this patch: N/A

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(khuey)
Comment on attachment 8666288 [details] [diff] [review]
Patch

OK, looks like it will improve the overall quality of Firefox.
Taking it for 42 & 43. Should be in 42 beta 4.

About esr38, it doesn't match the criteria for uplift. We are only taking sec-high & sec-critical patches. However, I am leaving the flag for the release owner (Ritu)
Attachment #8666288 - Flags: approval-mozilla-beta?
Attachment #8666288 - Flags: approval-mozilla-beta+
Attachment #8666288 - Flags: approval-mozilla-aurora?
Attachment #8666288 - Flags: approval-mozilla-aurora+
Comment on attachment 8666288 [details] [diff] [review]
Patch

As Sylvestre mentioned in comment 13, ESR triage bar allows for sec-crits and sec-highs. As such this bug fix does not meet that bar.
Attachment #8666288 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38-
You need to log in before you can comment on or make changes to this bug.