Bug 742371 - Remove MutationReceiver from MutationObserver when the target is deleted, r=sicking

content/base/src/nsDOMMutationObserver.cpp

@@ -145,12 +145,35 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(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,
@@ -382,7 +405,7 @@ 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)
@@ -420,6 +443,12 @@ nsDOMMutationObserver::GetReceiverFor(nsINode* aNode, bool aMayCreate)
   return r;
 }
 
+void
+nsDOMMutationObserver::RemoveReceiver(nsMutationReceiver* aReceiver)
+{
+  mReceivers.RemoveObject(aReceiver);
+}
+
 void
 nsDOMMutationObserver::GetAllSubtreeObserversFor(nsINode* aNode,
                                                  nsTArray<nsMutationReceiver*>&
@@ -527,6 +556,14 @@ nsDOMMutationObserver::Observe(nsIDOMNode* aTarget,
   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;
 }
 
@@ -534,7 +571,7 @@ NS_IMETHODIMP
 nsDOMMutationObserver::Disconnect()
 {
   for (PRInt32 i = 0; i < mReceivers.Count(); ++i) {
-    mReceivers[i]->Disconnect();
+    mReceivers[i]->Disconnect(false);
   }
   mReceivers.Clear();
   mCurrentMutations.Clear();

content/base/src/nsDOMMutationObserver.h

@@ -151,7 +151,8 @@ protected:
 
   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);
@@ -228,7 +229,7 @@ public:
     aParent->AddClone(this);
   }
 
-  virtual ~nsMutationReceiver() { Disconnect(); }
+  virtual ~nsMutationReceiver() { Disconnect(false); }
 
   nsMutationReceiver* GetParent()
   {
@@ -240,12 +241,12 @@ public:
     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);
@@ -253,27 +254,11 @@ public:
     }
 
     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
@@ -304,7 +289,8 @@ public:
 
   virtual void NodeWillBeDestroyed(const nsINode *aNode)
   {
-    Disconnect();
+    NS_ASSERTION(!mParent, "Shouldn't have mParent here!");
+    Disconnect(true);
   }
   
 };
@@ -351,6 +337,8 @@ 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();