951413 - Intermittent browser_dbg_conditional-breakpoints-03.js | leaked until shutdown [nsGlobalWindow #xxxx http://example.com/browser/browser/devtools/debugger/test/doc_conditional-breakpoints.html]

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

+++ This bug was initially created as a clone of Bug #949162 +++

https://tbpl.mozilla.org/php/getParsedLog.php?id=32097607&tree=Mozilla-Inbound

Rev5 MacOSX Mountain Lion 10.8 mozilla-inbound opt test mochitest-browser-chrome on 2013-12-17 09:04:55 PST for push bee6174bea6b
slave: talos-mtnlion-r5-032

09:38:40  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-03.js | leaked until shutdown [nsGlobalWindow #9587 http://example.com/browser/browser/devtools/debugger/test/doc_conditional-breakpoints.html]
09:38:40  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-03.js | leaked until shutdown [nsGlobalWindow #9585 http://example.com/browser/browser/devtools/debugger/test/doc_conditional-breakpoints.html]

Comment 1

[Legacy TBPL/Treeherder Robot]

Tomcat
https://tbpl.mozilla.org/php/getParsedLog.php?id=32206128&tree=Mozilla-Inbound
Rev5 MacOSX Mountain Lion 10.8 mozilla-inbound opt test mochitest-browser-chrome on 2013-12-19 00:54:31
revision: c245cc9c1b43
slave: talos-mtnlion-r5-071

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-03.js | leaked until shutdown [nsGlobalWindow #9631 http://example.com/browser/browser/devtools/debugger/test/doc_conditional-breakpoints.html]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-03.js | leaked until shutdown [nsGlobalWindow #9629 http://example.com/browser/browser/devtools/debugger/test/doc_conditional-breakpoints.html]

Comment 2

[Shu-yu Guo [:shu]]

I believe this is a regression from bug 933483

Comment 3

[Shu-yu Guo [:shu]]

As discussed on IRC, unsuppressing events in |nsGlobalWindow::DropOutWindowDocs| fixes the leak for me locally.

Comment 4

[Shu-yu Guo [:shu]]

Attachment: Unsuppress events on dropping doc from outer window.

ughhhhhhhhh

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8350248 [details] [diff] [review]
Unsuppress events on dropping doc from outer window.

rs=mrbkap but please file a followup on doing this right (as discussed on IRC).

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

The patch looks wrong. We shouldn't unsuppress random documents. Only documents which the nsGlobalWindow itself has suppressed.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Though, no one should be able to bring such documents back to a visible window, so not too bad
bug.

But in any case, caller who suppress, should also unsuppress.

Comment 8

[Shu-yu Guo [:shu]]

(In reply to Olli Pettay [:smaug] from comment #7)
> Though, no one should be able to bring such documents back to a visible
> window, so not too bad
> bug.
> 
> But in any case, caller who suppress, should also unsuppress.

Why should documents that get closed/navigated away from keep its suppression state?

Comment 9

[Shu-yu Guo [:shu]]

Attachment: Really unsuppress inner window docs on detach from docshell.

Attempt #2. What's going on in the devtools test is that it has 3 sets of
{pre,post}Nest which suppresses/unsuppresses event handling on the same window.
The postNests *all* race with window closure so that when we are inside
FreeInnerObjects the suppression depth might be > 1. This patch loops until we
have actually unsuppressed the docs.

Comment 10

[Shu-yu Guo [:shu]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/1c549a5d7700

Comment 11

[Shu-yu Guo [:shu]]

(In reply to Blake Kaplan (:mrbkap) from comment #5)
> Comment on attachment 8350248 [details] [diff] [review]
> Unsuppress events on dropping doc from outer window.
> 
> rs=mrbkap but please file a followup on doing this right (as discussed on
> IRC).

Filed bug 952318.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1c549a5d7700

Comment 13

[Andrew McCreight [:mccr8]]

Thanks for looking into this!

Comment 15

[Shu-yu Guo [:shu]]

Intermittents seem to have stopped.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Looks like this applies to Aurora as well. Is there any reason we shouldn't uplift this to it?

Comment 17

[Shu-yu Guo [:shu]]

Comment on attachment 8350357 [details] [diff] [review]
Really unsuppress inner window docs on detach from docshell.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 933483
User impact if declined: Leaks
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/773035d73263

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/releases/mozilla-aurora/rev/773035d73263 along with the other three commits from Ryan's uplift because one of the four caused windows m5 test failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=32842361&tree=Mozilla-Aurora

Comment 20

[Ryan VanderMeulen [:RyanVM]]

I still hit the failures with this backed out. Re-landed.
https://hg.mozilla.org/releases/mozilla-aurora/rev/89c68fcb1448