Bug 742371 - Remove MutationReceiver from MutationObserver when the target is deleted, r=sicking
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Wed, 04 Apr 2012 20:36:40 +0300
changeset 94283 320065be1731c72dc5e57237cde3d7f5fd1b630f
parent 94282 d9678c14dea97ff2a5aa2aa558219848dd02732e
child 94291 c598b7b202e7c94f1135f4437898294548f53005
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs742371
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 742371 - Remove MutationReceiver from MutationObserver when the target is deleted, r=sicking
content/base/src/nsDOMMutationObserver.cpp
content/base/src/nsDOMMutationObserver.h
--- a/content/base/src/nsDOMMutationObserver.cpp
+++ b/content/base/src/nsDOMMutationObserver.cpp
@@ -140,22 +140,45 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(nsMutat
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsMutationReceiver)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
   NS_INTERFACE_MAP_ENTRY(nsIMutationObserver)
   NS_INTERFACE_MAP_ENTRY(nsMutationReceiver)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsMutationReceiver)
-  tmp->Disconnect();
+  tmp->Disconnect(false);
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsMutationReceiver)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
+void
+nsMutationReceiver::Disconnect(bool aRemoveFromObserver)
+{
+  if (mRegisterTarget) {
+    mRegisterTarget->RemoveMutationObserver(this);
+    mRegisterTarget = nsnull;
+  }
+
+  mParent = nsnull;
+  nsINode* target = mTarget;
+  mTarget = nsnull;
+  nsIDOMMozMutationObserver* observer = mObserver;
+  mObserver = nsnull;
+  RemoveClones();
+
+  if (target && observer) {
+    if (aRemoveFromObserver) {
+      static_cast<nsDOMMutationObserver*>(observer)->RemoveReceiver(this);
+    }
+    // UnbindObject may delete 'this'!
+    target->UnbindObject(observer);
+  }
+}
 
 void
 nsMutationReceiver::AttributeWillChange(nsIDocument* aDocument,
                                         mozilla::dom::Element* aElement,
                                         PRInt32 aNameSpaceID,
                                         nsIAtom* aAttribute,
                                         PRInt32 aModType)
 {
@@ -377,17 +400,17 @@ NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsDOMMutationObserver)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsDOMMutationObserver)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsDOMMutationObserver)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mScriptContext)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mOwner)
   for (PRInt32 i = 0; i < tmp->mReceivers.Count(); ++i) {
-    tmp->mReceivers[i]->Disconnect();
+    tmp->mReceivers[i]->Disconnect(false);
   }
   tmp->mReceivers.Clear();
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMARRAY(mPendingMutations)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mCallback)
   // No need to handle mTransientReceivers
   NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsDOMMutationObserver)
@@ -416,16 +439,22 @@ nsDOMMutationObserver::GetReceiverFor(ns
   }
 
   nsMutationReceiver* r = new nsMutationReceiver(aNode, this);
   mReceivers.AppendObject(r);
   return r;
 }
 
 void
+nsDOMMutationObserver::RemoveReceiver(nsMutationReceiver* aReceiver)
+{
+  mReceivers.RemoveObject(aReceiver);
+}
+
+void
 nsDOMMutationObserver::GetAllSubtreeObserversFor(nsINode* aNode,
                                                  nsTArray<nsMutationReceiver*>&
                                                    aReceivers)
 {
   nsINode* n = aNode;
   while (n) {
     if (n->MayHaveDOMMutationObserver()) {
       nsMutationReceiver* r = GetReceiverFor(n, false);
@@ -522,24 +551,32 @@ nsDOMMutationObserver::Observe(nsIDOMNod
   r->SetAttributes(d.attributes);
   r->SetCharacterData(d.characterData);
   r->SetSubtree(d.subtree);
   r->SetAttributeOldValue(d.attributeOldValue);
   r->SetCharacterDataOldValue(d.characterDataOldValue);
   r->SetAttributeFilter(filters);
   r->SetAllAttributes(allAttrs);
   r->RemoveClones();
+
+#ifdef DEBUG
+  for (PRInt32 i = 0; i < mReceivers.Count(); ++i) {
+    NS_WARN_IF_FALSE(mReceivers[i]->Target(),
+                     "All the receivers should have a target!");
+  }
+#endif
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMMutationObserver::Disconnect()
 {
   for (PRInt32 i = 0; i < mReceivers.Count(); ++i) {
-    mReceivers[i]->Disconnect();
+    mReceivers[i]->Disconnect(false);
   }
   mReceivers.Clear();
   mCurrentMutations.Clear();
   mPendingMutations.Clear();
   return NS_OK;
 }
 
 
--- a/content/base/src/nsDOMMutationObserver.h
+++ b/content/base/src/nsDOMMutationObserver.h
@@ -146,17 +146,18 @@ protected:
   {
     mRegisterTarget->AddMutationObserver(this);
     mRegisterTarget->SetMayHaveDOMMutationObserver();
     mRegisterTarget->OwnerDoc()->SetMayHaveDOMMutationObservers();
   }
 
   nsMutationReceiverBase(nsINode* aRegisterTarget,
                          nsMutationReceiverBase* aParent)
-  : mObserver(nsnull), mParent(aParent), mRegisterTarget(aRegisterTarget)
+  : mTarget(nsnull), mObserver(nsnull), mParent(aParent),
+    mRegisterTarget(aRegisterTarget)
   {
     NS_ASSERTION(mParent->Subtree(), "Should clone a non-subtree observer!");
     mRegisterTarget->AddMutationObserver(this);
     mRegisterTarget->SetMayHaveDOMMutationObserver();
     mRegisterTarget->OwnerDoc()->SetMayHaveDOMMutationObservers();
   }
 
   bool ObservesAttr(mozilla::dom::Element* aElement,
@@ -223,62 +224,46 @@ public:
   nsMutationReceiver(nsINode* aRegisterTarget, nsMutationReceiverBase* aParent)
   : nsMutationReceiverBase(aRegisterTarget, aParent)
   {
     NS_ASSERTION(!static_cast<nsMutationReceiver*>(aParent)->GetParent(),
                  "Shouldn't create deep observer hierarchies!");
     aParent->AddClone(this);
   }
 
-  virtual ~nsMutationReceiver() { Disconnect(); }
+  virtual ~nsMutationReceiver() { Disconnect(false); }
 
   nsMutationReceiver* GetParent()
   {
     return static_cast<nsMutationReceiver*>(mParent.get());
   }
 
   void RemoveClones()
   {
     for (PRInt32 i = 0; i < mTransientReceivers.Count(); ++i) {
       nsMutationReceiver* r =
         static_cast<nsMutationReceiver*>(mTransientReceivers[i]);
-      r->Disconnect();
+      r->DisconnectTransientReceiver();
     }
     mTransientReceivers.Clear();
   }
 
-  void DisconnectTransientReceivers()
+  void DisconnectTransientReceiver()
   {
     if (mRegisterTarget) {
       mRegisterTarget->RemoveMutationObserver(this);
       mRegisterTarget = nsnull;
     }
 
     mParent = nsnull;
+    NS_ASSERTION(!mTarget, "Should not have mTarget");
+    NS_ASSERTION(!mObserver, "Should not have mObserver");
   }
 
-  void Disconnect()
-  {
-    if (mRegisterTarget) {
-      mRegisterTarget->RemoveMutationObserver(this);
-      mRegisterTarget = nsnull;
-    }
-
-    mParent = nsnull;
-    nsINode* target = mTarget;
-    mTarget = nsnull;
-    nsIDOMMozMutationObserver* observer = mObserver;
-    mObserver = nsnull;
-    RemoveClones();
-
-    if (target && observer) {
-      // Unbind may delete 'this'!
-      target->UnbindObject(observer);
-    }
-  }
+  void Disconnect(bool aRemoveFromObserver);
 
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IMUTATION_OBSERVER_IID)
   NS_DECL_AND_IMPL_ZEROING_OPERATOR_NEW
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(nsMutationReceiver)
 
   virtual void AttributeWillChange(nsIDocument* aDocument,
                                    mozilla::dom::Element* aElement,
@@ -299,17 +284,18 @@ public:
   virtual void ContentRemoved(nsIDocument *aDocument,
                               nsIContent* aContainer,
                               nsIContent* aChild,
                               PRInt32 aIndexInContainer,
                               nsIContent* aPreviousSibling);
 
   virtual void NodeWillBeDestroyed(const nsINode *aNode)
   {
-    Disconnect();
+    NS_ASSERTION(!mParent, "Shouldn't have mParent here!");
+    Disconnect(true);
   }
   
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsMutationReceiver, NS_MUTATION_OBSERVER_IID)
 
 class nsDOMMutationObserver : public nsIDOMMozMutationObserver,
                               public nsIJSNativeInitializer
@@ -346,16 +332,18 @@ public:
     return sCurrentObserver;
   }
 
   static void Shutdown();
 protected:
   friend class nsMutationReceiver;
   friend class nsAutoMutationBatch;
   nsMutationReceiver* GetReceiverFor(nsINode* aNode, bool aMayCreate);
+  void RemoveReceiver(nsMutationReceiver* aReceiver);
+
   void GetAllSubtreeObserversFor(nsINode* aNode,
                                  nsTArray<nsMutationReceiver*>& aObservers);
   void ScheduleForRun();
   void RescheduleForRun();
 
   nsDOMMutationRecord* CurrentRecord(const nsAString& aType);
   bool HasCurrentRecord(const nsAString& aType);