Closed
Bug 988863
Opened 10 years ago
Closed 10 years ago
Location expandos can disappear
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla31
Tracking | Status | |
---|---|---|
firefox28 | --- | affected |
firefox29 | + | fixed |
firefox30 | + | fixed |
firefox31 | + | fixed |
firefox-esr24 | --- | unaffected |
b2g18 | --- | unaffected |
People
(Reporter: bholley, Assigned: bholley)
References
Details
(Keywords: regression, site-compat)
Attachments
(1 file)
(deleted),
patch
|
bholley
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This is a regression from bug 961204. It affects FF29 and newer. Previously, we never preserved the Location wrapper, because it was a value prop on the Window. But in bug 961204 (FF29), we stopped doing that. In bug 809547, we added test coverage to make sure that Location expandos don't get collected. As Peter discovered, SpecialPowers was adding edges to the Location object, pinning it alive in automation when it would actually be collected in release builds. Bug 988334 exposed this, and started making that test coverage fail. The test coverage for this bug is effectively the test from bug 809547, which will start doing its job when we land bug 988334. I'll upload a patch.
Assignee | ||
Comment 1•10 years ago
|
||
This is a web-compat regression that we should track and fix before shipping the regression (currently on beta).
status-b2g18:
--- → unaffected
status-firefox28:
--- → unaffected
status-firefox29:
--- → affected
status-firefox30:
--- → affected
status-firefox31:
--- → affected
status-firefox-esr24:
--- → unaffected
tracking-firefox29:
--- → ?
tracking-firefox30:
--- → ?
tracking-firefox31:
--- → ?
Assignee | ||
Comment 3•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b1b90e5fd34
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8397845 [details] [diff] [review] Preserve Location in AddProperty. v1 r=bholley [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 961204 User impact if declined: nasty web-compat issues. User-defined properties on a Location objects can disappear non-deterministically during GC. Testing completed (on m-c, etc.): Just pushed to m-i. Risk to taking this patch (and alternatives if risky): Low risk. String or IDL/UUID changes made by this patch: None
Attachment #8397845 -
Flags: approval-mozilla-beta?
Attachment #8397845 -
Flags: approval-mozilla-aurora?
Comment 5•10 years ago
|
||
Backed out for: 09:29:23 INFO - ##### 09:29:23 INFO - ##### Running check-expectations step. 09:29:23 INFO - ##### 09:29:23 INFO - Running main action method: check_expectations 09:29:23 INFO - Reading from file /builds/slave/l64-br-haz_m-in_dep-0000000000/build/source/js/src/devtools/rootAnalysis/expect.browser.json 09:29:24 INFO - Contents: 09:29:24 INFO - { 09:29:24 INFO - "expect-hazards": 0 09:29:24 INFO - } 09:29:24 INFO - Reading from file /builds/slave/l64-br-haz_m-in_dep-0000000000/build/analysis/rootingHazards.txt 09:29:24 WARNING - TEST-UNEXPECTED-FAIL 1 more hazards than expected (expected 0, saw 1) https://tbpl.mozilla.org/php/getParsedLog.php?id=36812265&tree=Mozilla-Inbound#error0 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2d754cf4e122
![]() |
||
Comment 6•10 years ago
|
||
Presumably need to root obj?
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #6) > Presumably need to root obj? Sounds right. Running the analysis to double-check: https://tbpl.mozilla.org/?tree=Try&rev=d60acd46b188
Assignee | ||
Comment 8•10 years ago
|
||
Rooting analysis is green. Relanding: https://hg.mozilla.org/integration/mozilla-inbound/rev/7cad4bd4bfe5
Updated•10 years ago
|
https://hg.mozilla.org/mozilla-central/rev/7cad4bd4bfe5
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Updated•10 years ago
|
Attachment #8397845 -
Flags: approval-mozilla-beta?
Attachment #8397845 -
Flags: approval-mozilla-beta+
Attachment #8397845 -
Flags: approval-mozilla-aurora?
Attachment #8397845 -
Flags: approval-mozilla-aurora+
Comment 10•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/eee14ec21647 https://hg.mozilla.org/releases/mozilla-beta/rev/b21947c608e9
Comment 11•10 years ago
|
||
Is this fix fully covered by the test you've mentioned in comment 0?
Flags: needinfo?(bobbyholley)
Flags: in-testsuite?
Assignee | ||
Comment 12•10 years ago
|
||
(In reply to Florin Mezei, QA [:FlorinMezei] from comment #11) > Is this fix fully covered by the test you've mentioned in comment 0? Only with bug 988334 in the tree, unfortunately. This is a hard fix to QA, and I think we understand the issue well enough that we don't need to verify the bug.
Flags: needinfo?(bobbyholley)
Flags: in-testsuite?
Flags: in-testsuite+
![]() |
||
Comment 13•10 years ago
|
||
Firefox 28 had this problem too. See bug 994048.
Updated•10 years ago
|
QA Whiteboard: [qa-]
Updated•10 years ago
|
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•