Closed Bug 1029632 Opened 6 years ago Closed 6 years ago

CreateFixUpURI leaks Preference observers via AddBoolVarCache

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Type:
defect
Priority:
Not set
Severity:
normal
Points:
1

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla32
Iteration:
33.2
Tracking Flags:
Tracking Status
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- unaffected

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file)

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: 6 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.