Comment on attachment 591025 [details] [diff] [review] this might even compile Review of attachment 591025 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/public/nsContentUtils.h @@ +993,5 @@ > */ > static nsEventListenerManager* GetListenerManager(nsINode* aNode, > bool aCreateIfNotFound); > > + static void UnmarkGrayJSListenersInCCGenerationDocuments(PRUint32 aGeneration); That's quite a long name. Too bad InCCGeneration wasn't something a little shorter. Oh well. :) ::: content/base/src/nsContentUtils.cpp @@ +3398,5 @@ > + PRUint32* gen = static_cast<PRUint32*>(aArg); > + EventListenerManagerMapEntry* entry = > + static_cast<EventListenerManagerMapEntry*>(aEntry); > + if (entry) { > + nsINode* n = static_cast<nsINode*>(entry->mListenerManager->GetTarget()); The target keeps the listener manager alive, right? @@ +3400,5 @@ > + static_cast<EventListenerManagerMapEntry*>(aEntry); > + if (entry) { > + nsINode* n = static_cast<nsINode*>(entry->mListenerManager->GetTarget()); > + if (n && n->IsInDoc() && > + n->OwnerDoc()->GetMarkedCCGeneration() == nsCCUncollectableMarker::sGeneration) { Should this be nsCCUncollectableMarker::InGeneration(n->OwnerDoc()->GetMarkedCCGeneration()) so you check that the generation is non-zero?

(In reply to Andrew McCreight [:mccr8] from comment #2) > > The target keeps the listener manager alive, right? yes > > @@ +3400,5 @@ > > + static_cast<EventListenerManagerMapEntry*>(aEntry); > > + if (entry) { > > + nsINode* n = static_cast<nsINode*>(entry->mListenerManager->GetTarget()); > > + if (n && n->IsInDoc() && > > + n->OwnerDoc()->GetMarkedCCGeneration() == nsCCUncollectableMarker::sGeneration) { > > Should this be > nsCCUncollectableMarker::InGeneration(n->OwnerDoc()- > >GetMarkedCCGeneration()) so you check that the generation is non-zero? Ah, I guess I added this before adding the non cb version of generation check.

It looks like this landed: http://hg.mozilla.org/mozilla-central/rev/2bfef4ad7ad3 Should this bug be closed? Also, this introduced this build warning: > nsContentUtils.cpp:3397:13: warning: unused variable ‘gen’ [-Wunused-variable] from this chunk: > 3393 PLDHashOperator > 3394 ListenerEnumerator(PLDHashTable* aTable, PLDHashEntryHdr* aEntry, > 3395 PRUint32 aNumber, void* aArg) > 3396 { > 3397 PRUint32* gen = static_cast<PRUint32*>(aArg); |gen| is indeed unused in that method. Can we just drop it?

https://hg.mozilla.org/mozilla-central/rev/2bfef4ad7ad3 I'm closing stuff once I know the tree is green. And yes, that gen looks like a leftover. I'll fix it.