CreateFixUpURI leaks Preference observers via AddBoolVarCache

RESOLVED FIXED in Firefox 31

Status

()

Product:
Core
Component:
Document Navigation
Type:
defect
Status:
RESOLVED FIXED
Opened:
5 years ago
Updated:
5 years ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

Version:
unspecified
Target:
mozilla32
Points:
1
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox31 fixed, firefox32 fixed, firefox33 unaffected)

Details

Attachments

(1 attachment)

prevent leaks through adding pref cache,
2.35 KB, patch
bzbarsky
: review+
sylvestre
: approval-mozilla-aurora+
sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review 
This was noticed in bug 982428, and I'm trying to ensure this never happens again in bug 1029509. In any case, because bug 982428 changes interfaces and isn't entirely risk-free uplift isn't straightforward. Whatever we end up doing there, we should definitely ensure that we fix the leakiness for 31/32.
Blocks: 935377
Patch against beta
Attachment #8445292 - Flags: review?(bzbarsky)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
(applies cleanly to aurora as well)
Comment on attachment 8445292 [details] [diff] [review]
prevent leaks through adding pref cache,

r=me
Attachment #8445292 - Flags: review?(bzbarsky) → review+
Comment on attachment 8445292 [details] [diff] [review]
prevent leaks through adding pref cache,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 935377
User impact if declined: (very) slow leakage as people use the browser
Testing completed (on m-c, etc.): local, has automated tests. Basically just a very small part of the patch for bug 982428, purely to counter the leakage that we uncovered.
Risk to taking this patch (and alternatives if risky): very low because of automated tests for this particular feature of the code
String or IDL/UUID changes made by this patch: none
Attachment #8445292 - Flags: approval-mozilla-beta?
Attachment #8445292 - Flags: approval-mozilla-aurora?
(In reply to :Gijs Kruitbosch from comment #4)
> Testing completed (on m-c, etc.): local, has automated tests. Basically just
> a very small part of the patch for bug 982428, purely to counter the leakage
> that we uncovered.

To be clear, this is essentially in lieu of uplifting bug 982428, which means I'm not intending to land this on fx-team / m-c.
Attachment #8445292 - Flags: approval-mozilla-beta?
Attachment #8445292 - Flags: approval-mozilla-beta+
Attachment #8445292 - Flags: approval-mozilla-aurora?
Attachment #8445292 - Flags: approval-mozilla-aurora+
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/0b67057373f0
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/53f73bdf5124
Status: ASSIGNED → RESOLVED
Iteration: --- → 33.2
Points: --- → 1
Closed: 5 years ago
QA Whiteboard: qa-]
status-firefox31: --- → fixed
status-firefox32: --- → fixed
Resolution: --- → FIXED
Marco, can you add this as well? Thanks!
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Added to Iteration 33.2
Flags: needinfo?(mmucci)
status-firefox33: --- → unaffected
Target Milestone: --- → mozilla32
QA Whiteboard: qa-] → [qa-]
You need to log in before you can comment on or make changes to this bug.