720630 - Add a way to unmark all the listeners in black documents

Status: RESOLVED FIXED

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

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: this might even compile

Comment 2

[Andrew McCreight [:mccr8]]

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?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(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.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 5

[Daniel Holbert [:dholbert]]

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?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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.