946641 - Drop delayed events associated with a document when it's closed

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Shu-yu Guo [:shu]]

No reason for delayed events to hold documents alive, since it's pretty hard get right in chrome JS to make sure we don't race the call to unsuppress with tab/window closures.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Clear delayed events on closed documents.

Comment 2

[Shu-yu Guo [:shu]]

Attachment: Clear delayed events on closed documents.

Left in a superfluous change in the last version.

Comment 3

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

Comment on attachment 8343009 [details] [diff] [review]
Clear delayed events on closed documents.

This may overlap some stuff mrbkap has been doing lately.

Update uuid of nsIFocusManager

Comment 4

[Shu-yu Guo [:shu]]

Attachment: Part 2: Revert in Debugger frontend to balance event suppression/unsuppression.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8343315 [details] [diff] [review]
Part 2: Revert in Debugger frontend to balance event suppression/unsuppression.

Review of attachment 8343315 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't tested the patch as it needs some rebasing, but does this fix the leak? If so, this is great!

::: browser/devtools/debugger/test/browser.ini
@@ +85,5 @@
>  [browser_dbg_breakpoints-pane.js]
>  [browser_dbg_chrome-debugging.js]
>  [browser_dbg_clean-exit-window.js]
>  # Disabled because of leaks - See Bug 933950.
> +#skip-if = true

Just remove both comment lines.

::: toolkit/devtools/server/actors/webbrowser.js
@@ -797,5 @@
>    postNest: function BTA_postNest(aNestData) {
>      if (!this.window) {
>        // The tab is already closed.
> -      dbg_assert(this._nestedEventLoopDepth === 0,
> -                 "window shouldn't be closed before all nested event loops have been popped");

You will also have to revert my changes from bug 942931 here.

Comment 6

[Shu-yu Guo [:shu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7186d67c952
https://hg.mozilla.org/integration/mozilla-inbound/rev/80db40e98167

Comment 7

[Shu-yu Guo [:shu]]

http://hg.mozilla.org/mozilla-central/rev/80db40e98167
http://hg.mozilla.org/mozilla-central/rev/b7186d67c952

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Shu, are you going to uplift this fix to beta (Firefox 27)? The hang-on-close in bug 942931 is still present there.

Comment 9

[Shu-yu Guo [:shu]]

(In reply to Panos Astithas [:past] from comment #8)
> Shu, are you going to uplift this fix to beta (Firefox 27)? The
> hang-on-close in bug 942931 is still present there.

To uplift this to beta without re-causing the leaks, we will also need to uplift:

 - bug 951413 (me)
 - bug 933483 (mrbkap)

NI mrbkap for status of uplift.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Shu-yu Guo [:shu] from comment #9)
>  - bug 933483 (mrbkap)

Note that bug 933483 caused bug 959585 (and bug 956704). It was already uplifted to Aurora, but we shouldn't uplift it to beta (for sure) until we shake out the rest of the regressions.