Closed Bug 664444 Opened 14 years ago Closed 14 years ago

Unlink nsGlobalChromeWindow

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
The patch does also unlink mDoc and calls ELM->Disconnect() properly.
Attachment #539541 - Flags: review?(peterv)
Comment on attachment 539541 [details] [diff] [review] patch Review of attachment 539541 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsGlobalWindow.cpp @@ +1412,5 @@ > NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mOpenerScriptPrincipal) > + if (tmp->mListenerManager) { > + tmp->mListenerManager->Disconnect(); > + NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mListenerManager) > + } I have to say I don't really like this. Either we let the CC unlink things or we do it manually, if we need to do both it means our CC traversal is still incomplete and we're just hiding that with manual unlinking. @@ +10119,5 @@ > + nsGlobalWindow) > + NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mBrowserDOMWindow) > + if (tmp->mMessageManager) { > + static_cast<nsFrameMessageManager*>( > + tmp->mMessageManager.get())->Disconnect(); Shouldn't this be done from the nsFrameMessageManager unlink code? What do the cycles that this is unlinking look like? And what are we missing to let the CC do the unlinking.
> @@ +1412,5 @@ > > NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mOpenerScriptPrincipal) > > + if (tmp->mListenerManager) { > > + tmp->mListenerManager->Disconnect(); > > + NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mListenerManager) > > + } > > I have to say I don't really like this. Either we let the CC unlink things > or we do it manually, if we need to do both it means our CC traversal is > still incomplete and we're just hiding that with manual unlinking. Well, listenermanager asserts if it is deleted without disconnect. And message manager should be disconnected when the owner doesn't have pointer to it anymore.
(In reply to comment #2) > Well, listenermanager asserts if it is deleted without disconnect. And right before the assert is a comment: // XXX azakai: Is there any reason to not just call Disconnect // from right here, if not previously called? So, any reason?
Attachment #539541 - Flags: review?(peterv) → review+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: