No description provided.
Comment on attachment 346669 [details] [diff] [review] v1 >diff --git a/content/html/document/src/nsImageDocument.cpp b/content/html/document/src/nsImageDocument.cpp >+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsImageDocument, nsMediaDocument) >+ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mImageContent) Hmm, do we need to add Traverse/Unlink to nsImageLoadingContent too, in particular for its observers? >diff --git a/content/html/document/src/nsPluginDocument.cpp b/content/html/document/src/nsPluginDocument.cpp >+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsPluginDocument, nsMediaDocument) >+ NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mPluginContent) Only risk here looks like nsPluginStreamListener::OnStartRequest uses this unconditionally. No idea if we can unlink before that's called?
(In reply to comment #1) > Hmm, do we need to add Traverse/Unlink to nsImageLoadingContent too, in > particular for its observers? Maybe. That isn't needed to pass mochitest without leaks. "Typically we will have only one observer (our frame in the screen * prescontext)" The frame will be destroyed early enough. But if there are other kinds of observers, then not sure.
(In reply to comment #1) > Only risk here looks like nsPluginStreamListener::OnStartRequest uses this > unconditionally. Actually it doesn't. shell->GetPrimaryFrameFor(embed); returns nsnull if embed is nsnull.
Can I get a summary of risk and reward here?
This fixes possible leaks (I think I could even write a leak testcase). Can't see any real risks.
Comment on attachment 346669 [details] [diff] [review] v1 a191=beltzner
Attachment #346669 - Flags: approval1.9.1? → approval1.9.1+
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Fixed on trunk and 1.9.1
Target Milestone: --- → mozilla1.9.1b3
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.