Last Comment Bug 742371 - Remove MutationReceiver from MutationObserver when the target is deleted
: Remove MutationReceiver from MutationObserver when the target is deleted
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 12 Branch
: x86_64 Linux
-- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug] (pto-ish for couple of days)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-04 07:31 PDT by Olli Pettay [:smaug] (pto-ish for couple of days)
Modified: 2012-04-04 13:53 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (7.99 KB, patch)
2012-04-04 07:31 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
jonas: review+
Details | Diff | Splinter Review
+assertion (8.13 KB, patch)
2012-04-04 10:34 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
no flags Details | Diff | Splinter Review

Description User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-04-04 07:31:37 PDT
Created attachment 612183 [details] [diff] [review]
patch

Currently if target is deleted, MutationObserver ends up having receivers
with null target. Nothing scary there, but no need to keep such things
alive.

This patch fixes also transient receiver disconnecting.
s/Disconnect/DisconnectTransientReceiver/

The patch adds also some warnings and assertions.
Comment 1 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-04-04 09:59:55 PDT
Comment on attachment 612183 [details] [diff] [review]
patch

If I understand correctly NodeWillBeDestroyed can only be called on non-transient receivers, since transient receivers are always attached to nodes which we hold a strong reference to in a MutationRecord.

Is this correct?

If so, could you add an assertion to NodeWillBeDestroyed that checks that the receiver doesn't have a parent? Mostly as documentation, but also to make sure that we don't have any bad bugs.
Comment 2 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-04-04 10:05:02 PDT
(In reply to Jonas Sicking (:sicking) from comment #1)
> Comment on attachment 612183 [details] [diff] [review]
> patch
> 
> If I understand correctly NodeWillBeDestroyed can only be called on
> non-transient receivers, since transient receivers are always attached to
> nodes which we hold a strong reference to in a MutationRecord.
> 
> Is this correct?
No, but nsMutationObserver has node->transientobserverlist hashtable.
That keeps node and transientobservers alive.


> 
> If so, could you add an assertion to NodeWillBeDestroyed that checks that
> the receiver doesn't have a parent? Mostly as documentation, but also to
> make sure that we don't have any bad bugs.
Sure.
Comment 3 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-04-04 10:34:18 PDT
Created attachment 612252 [details] [diff] [review]
+assertion
Comment 4 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-04-04 10:57:39 PDT
https://hg.mozilla.org/mozilla-central/rev/320065be1731
Comment 5 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2012-04-04 13:53:55 PDT
Makes sense.

Note You need to log in before you can comment on or make changes to this bug.