Closed
Bug 1137933
Opened 9 years ago
Closed 9 years ago
Don't cache the docshell in WebNavigation in browser-child.js
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 40
People
(Reporter: mccr8, Assigned: billm)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file, 1 obsolete file)
3.37 KB,
patch
|
Felipe
:
review+
|
Details | Diff | Splinter Review |
When running toolkit/components/addoncompat/ we end up holding onto a docshell for too long. Bill and I looked at this, and it seems like what is happening is that the _webNavigation field of WebNavigation keeps the docshell alive, even past when we clear the webNavigation field off of the global. Bill wrote a patch to fix this that just makes us QI it every time we access it, avoiding the leak.
Reporter | ||
Updated•9 years ago
|
tracking-e10s:
--- → ?
Reporter | ||
Comment 1•9 years ago
|
||
Also don't cache the session history, because we only use it in init. (Bill wrote this patch.)
Reporter | ||
Updated•9 years ago
|
Attachment #8570736 -
Flags: review?(felipc)
Reporter | ||
Comment 2•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ea0a1eaf31a
Updated•9 years ago
|
Attachment #8570736 -
Flags: review?(felipc) → review+
Reporter | ||
Comment 3•9 years ago
|
||
Try run was green. It looks like this fixes up almost all of the remaining bc and dt DOCSHELL DOMWINDOW leaks, based on Bill's push to Holly. https://hg.mozilla.org/integration/mozilla-inbound/rev/a761ac6f8bcc
Reporter | ||
Comment 4•9 years ago
|
||
This seems to be making opt builds time out in e10s bc3, in various thumbnails tests. Very weird. I think the timeouts are due to this exception being thrown: 09:18:10 WARNING - TEST-UNEXPECTED-FAIL | unknown test url | uncaught exception - ReferenceError: executeSoon is not defined at chrome://mochitests/content/browser/toolkit/components/thumbnails/test/head.js:228
Reporter | ||
Comment 5•9 years ago
|
||
backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/d0e70d2cf479 I guess I'll look into these failures on Monday.
Updated•9 years ago
|
Updated•9 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee | ||
Comment 6•9 years ago
|
||
The original patch had a bug. There's a pre-existing comment explaining that we need to keep the sessionHistory alive. This patch regressed that. Anyway, this new patch fixes the issue and still avoids leaks.
Attachment #8570736 -
Attachment is obsolete: true
Attachment #8585216 -
Flags: review?(felipc)
Reporter | ||
Updated•9 years ago
|
Attachment #8585216 -
Attachment is patch: true
Updated•9 years ago
|
Attachment #8585216 -
Flags: review?(felipc) → review+
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/00e022f7e45e
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/00e022f7e45e
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
You need to log in
before you can comment on or make changes to this bug.
Description
•