475717 - Disconnected document with title stays alive past cycle collection

Status: RESOLVED DUPLICATE of bug 488786

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

Description

[Peter Van der Beken [:peterv]]

1 nsDocument::cycleCollection::Unlink(void*) (nsDocument.cpp:1857)
 1 nsGenericHTMLElement::UnbindFromTree(int, int) (nsGenericHTMLElement.cpp:929)
  1 nsGenericElement::UnbindFromTree(int, int) (nsGenericElement.cpp:2689)
   1 nsGenericHTMLElement::UnbindFromTree(int, int) (nsGenericHTMLElement.cpp:929)
    1 nsGenericElement::UnbindFromTree(int, int) (nsGenericElement.cpp:2689)
     1 nsHTMLTitleElement::UnbindFromTree(int, int) (nsHTMLTitleElement.cpp:195)
      1 nsHTMLTitleElement::SendTitleChangeEvent(int) (nsHTMLTitleElement.cpp:214)
       1 nsDocument::NotifyPossibleTitleChange(int) (nsDocument.cpp:5036)
        1 nsRunnableMethod<nsDocument>::nsRunnableMethod(nsDocument*, void (nsDocument::*)()) (libgklayout.dylib)
         1 nsHTMLDocument::AddRef() (nsHTMLDocument.cpp:250)
          1 nsDocument::AddRef() (nsDocument.cpp:1642)

Unlinking a document with a title element will actually add an additional reference to the document, because we post a runnable to notify about a title change. The document will be released when the runnable has run, but it seems wrong to post the runnable for a document that's not reachable by anything. We could bail if it doesn't have a presshell, but that doesn't seem like the right thing to do. Roc, thoughts?

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

How can we tell if it's not reachable by anything? Can we set a flag in Unlink to tell us that?

Comment 2

[Peter Van der Beken [:peterv]]

Usually we check for a member being null. I don't think there's anything in nsDocument that is null only when we unlinked. We could maybe check mExternalResourceMap.HaveShutDown()? Or add an unlinked flag.

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 362611 [details] [diff] [review]
v1

Wow, if only all patches were this easy :)

Comment 5

[Peter Van der Beken [:peterv]]

:-/.