Ben Kelly [:bkelly, not reviewing] Assignee
	Description • 8 years ago

Today in 50.0a1 (2016-07-31) I discovered I had a couple leaked browser.xul windows.  Both were being retained via this path:

00000238DA4EBCE0 [BackstagePass 238e000a680]
    --[PdfjsChromeUtils]--> 00000238DDD04EC0 [Object <no private>]
    --[_browsers]--> 00000238DFC7F580 [Set 238ddb9b400]
    --[key]--> 00000238EBA433E0 [Proxy <no private>]
    --[private]--> 00000238EC67E190 [XULElement <no private>]
    --[shape]--> 00000238ECFCCAC0 [shape]
    --[base]--> 00000238ECFD4A60 [base_shape]
    --[global]--> 00000238D9A9E060 [Window <no private>]

Seems like this is a pretty easy case where we could use a HashSet() instead of a Set().

Brendan, what do you think?  Is this maintained upstream?

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 1 • 8 years ago

I meant, we could use a WeakSet(). :-)

	Brendan Dahl [:bdahl]
	Comment 2 • 8 years ago

It's maintained upstream, but we also will pull down changes from mc. Looks like bug 1072350 added this.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 3 • 8 years ago

Thanks, that helped me find steps to reproduce:

1) Launch fresh browser
2) open about:memory in a tab
3) Open a new window
4) Navigate to a pdf in the new window (I used an IRS form)
5) Use ctrl-f to find a word in the pdf
6) Close the new window
7) Minimize and measure memory in the about:memory tab

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 4 • 8 years ago

This fixes the problem for me locally.

	Brendan Dahl [:bdahl]
	Comment 5 • 8 years ago

I was hoping to look at this a bit more today, but didn't get a chance. FYI, we've been having some discussion upstream in https://github.com/mozilla/pdf.js/pull/7521 on why pdf.js's current code doesn't work.

	Pulsebot
	Comment 6 • 8 years ago

Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aeb1e15857b3
Make PdfjsChromeUtils.jsm use a WeakSet for tracking browsers. r=bdahl

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 8 years ago

Please push changes to browser/ to fx-team in the future to avoid merge conflicts.

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/aeb1e15857b3