87466 - Global object and focus stuff leaking via cycle, per window

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

A bunch of focus related stuff leaks per-window via a cycle that includes the
global object, and, if a key is pressed in that window, drags in an XBL document
for each window where a key is pressed (which is 30K of logged (via
nsTraceRefcnt) objects per window, probably a good bit more (50% more? 500%
more?) of unlogged objects).  The following diagram explains what I'm seeing
(where |A| mFoo--> |B| represents A having a member mFoo that has an owning
pointer to B):

          Another GlobalWindowImpl, in one case but not the others
            ^
            |
         mOpener    mChromeEventHandler--
  ------------------                     \   --------------
  |GlobalWindowImpl|                      -> |nsWindowRoot|
  ------------------                         --------------
               ^             -----mFocusController     mListenerManager
              /             /                               /
             /             /                               /
            /             /   ------------------------    /
           /             /    |nsEventListenerManager| <--
     mFocusedWindow     v     ------------------------
      -------------------         m???Listeners
      |nsFocusController|           /    \
      ------------------- <---------      v
                                         -----------------------
                                         |nsXBLWindowKeyHandler|
                                         -----------------------
                                             |
                                             |
                                   (if a key is pressed)
                                             |
                                             v
                                     -------------------
                                     |nsXBLDocumentInfo|
                                     -------------------
                                             |
                                             v
                                       ------------
                                       |nsDocument|----> Content nodes
                                       ------------

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This seems related to hyatt's fix for bug 54023.  He changed:

  nsIChromeEventHandler*        mChromeEventHandler; // Weak Reference         

to

  nsCOMPtr<nsIChromeEventHandler> mChromeEventHandler; // [Strong] We break it 
when we get torn down.

but I don't see any code to break the cycle when the global is torn down.  Such
code would be easy to write, though....

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch fixing leak

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Well, the above patch works and it doesn't seem to do anything harmful after
about a minute of using the browser.  Are there any problems that it could cause?

Comment 4

[David Hyatt]

sr=hyatt

Comment 5

[saari (gone)]

looks right to me, r=saari

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in 2001-06-26 14:29 PDT.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Marking [PDT+] per conversation with chofmann.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked into branch 2001-06-30 12:01 PDT.

Comment 9

[Madhur Bhatia]

marking verifed as per developers comments.