AutoCompleteE10S.jsm leaks windows

RESOLVED FIXED in Firefox 50

Status

()

Toolkit
Autocomplete
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

({regression})

Trunk
mozilla50
regression
Points:
---

Firefox Tracking Flags

(e10s?, firefox50 fixed)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
I got a leaked browser.xul in my nightly 50.0a1 (2016-06-29) today:

│   ├───4.87 MB (02.38%) -- top(none)/detached
│   │   ├──4.87 MB (02.38%) -- window(chrome://browser/content/browser.xul)
│   │   │  ├──2.62 MB (01.28%) ++ js-compartment([System Principal], about:blank)
│   │   │  └──2.26 MB (01.10%) ++ (3 tiny)
│   │   └──0.00 MB (00.00%) ── window([system])/dom/other

Looking at the CC logs its being leaked through AutoCompleteE10S.jsm:

Parsing cc-edges.8716.1467228425.log. Done loading graph.

0000023C93066060 [JS Object (Window)]
    --[UnwrapDOMObject(obj)]--> 0000023C9B310000 [nsGlobalWindow # 145 inner chrome://browser/conten
t/browser.xul]

    Root 0000023C93066060 is a marked GC object.


bkelly@kosh /c/devel/tmp/cclogs/20160629-browser-xul
$ /c/devel/heapgraph/find_roots.py gc-edges.8716.1467228425.log -bro 0000023C93066060
Parsing gc-edges.8716.1467228425.log. Done loading graph.

via persistent-Object :
0000023C87EB6CE0 [BackstagePass 23c894db280]
    --[AutoCompleteE10S]--> 0000023C930AB080 [Object <no private>]
    --[_popupCache]--> 0000023C97B8BD80 [Object <no private>]
    --[browserWindow]--> 0000023C9817B3A0 [Proxy <no private>]
    --[private]--> 0000023C9307D040 [Proxy <no private>]
    --[private]--> 0000023C93066060 [Window <no private>]

Unfortunately I don't know what I did to trigger this.

Blake, you touched this file last.  What do you think?
Flags: needinfo?(mrbkap)
(Assignee)

Comment 1

2 years ago
I guess I'm not 100% sure if this is a regression or not.
(Assignee)

Comment 2

2 years ago
This could also be related to the cycle collector strangeness in bug 1283264.
See Also: → bug 1283264
Looking at the code, nothing seems to clear this._popupCache. Maybe there's also a similar problem with this.browser.
Flags: needinfo?(felipc)
tracking-e10s: --- → ?
(Assignee)

Updated

2 years ago
See Also: → bug 1286008
Whiteboard: [MemShrink] → [MemShrink:P2]
(Assignee)

Comment 4

2 years ago
I have steps to reproduce this:

1) Open a new nightly browser instance
2) Open a example.com in a tab (to hold the child process alive)
3) Open about:memory in a tab
4) Open a new second window
5) Open a site with an address form (like usps.gov/umove wizard) in second window
6) Type some stuff into the address fields
7) Close the second window
8) Minimize and measure memory in about:memory tab
9) Observe that the second window has leaked
(Assignee)

Comment 5

2 years ago
I hit this enough that I'm going to try to fix it.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
(Assignee)

Comment 6

2 years ago
Created attachment 8772167 [details] [diff] [review]
P1 Cleanup the AutoCompleteE10S popup cache when the input is disconnected.  r=mrbkap

I think this is a 95% solution.  It cleans up the cache when:

1) a user tabs out of an autocompleting form field \
2) a user closes the tab while focused on a form field if there are other tabs open in the window

It does not, however, fix the problem if the user closes the entire window while focused on a form field.  I've had a lot of trouble trying to get the message manager to send its message in that case.
Flags: needinfo?(felipc)
(Assignee)

Comment 7

2 years ago
Try build (although treeherder is broken at the moment):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4bfec9520a9c
(Assignee)

Comment 8

2 years ago
Created attachment 8772399 [details] [diff] [review]
P1 Make AutomCompleteE10S clear its popup cache when the associated browser window is closed. r=mrbkap

The 100% solution.  Just listen for the browser unload event and clear the cache.
Attachment #8772167 - Attachment is obsolete: true
(Assignee)

Comment 10

2 years ago
Comment on attachment 8772399 [details] [diff] [review]
P1 Make AutomCompleteE10S clear its popup cache when the associated browser window is closed. r=mrbkap

This patch simply clears the _popupCache when the associated browser is unloaded.

I tried to write a test for this but I can't seem to trigger the AutoCompleteE10S code in a new window in a browser chrome test.  Any suggestions on how to do this are welcome.  I may just land this without a new test, though.
Flags: needinfo?(mrbkap)
Attachment #8772399 - Flags: review?(mrbkap)

Updated

2 years ago
Attachment #8772399 - Flags: review?(mrbkap) → review+

Comment 11

2 years ago
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4de50147d99c
Make AutoCompleteE10S clear its popup cache when the associated browser window is closed. r=mrbkap

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4de50147d99c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.