Bug 578696 part 3. Stop holding strong refs when calling ParentChainChanged. r=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 21 Jul 2010 11:33:31 -0400
changeset 48008 b357183c1add6f17713dd91d8414a1942a2a4135
parent 48007 a0556636778c3658bcfdc2814a1478a5497df779
child 48009 f6599baced65454bd4a95aac83533e89d9b5084a
push id14533
push userbzbarsky@mozilla.com
push dateWed, 21 Jul 2010 15:42:13 +0000
treeherdermozilla-central@b20c759afb7c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs578696
milestone2.0b3pre
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 578696 part 3. Stop holding strong refs when calling ParentChainChanged. r=sicking
content/base/public/nsIMutationObserver.h
content/base/src/nsNodeUtils.cpp
content/base/src/nsRange.cpp
--- a/content/base/public/nsIMutationObserver.h
+++ b/content/base/public/nsIMutationObserver.h
@@ -249,16 +249,20 @@ public:
    * happens when either the node or one of its ancestors is inserted
    * or removed as a child of another node.
    *
    * Note that when a node is inserted this notification is sent to
    * all descendants of that node, since all such nodes have their
    * parent chain changed.
    *
    * @param aContent  The piece of content that had its parent changed.
+   *
+   * @note Callers of this method might not hold a strong reference to
+   *       the observer.  The observer is responsible for making sure it
+   *       stays alive for the duration of the call as needed.
    */
 
   virtual void ParentChainChanged(nsIContent *aContent) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIMutationObserver, NS_IMUTATION_OBSERVER_IID)
 
 #define NS_DECL_NSIMUTATIONOBSERVER_CHARACTERDATAWILLCHANGE                  \
--- a/content/base/src/nsNodeUtils.cpp
+++ b/content/base/src/nsNodeUtils.cpp
@@ -215,17 +215,17 @@ void
 nsNodeUtils::ParentChainChanged(nsIContent *aContent)
 {
   // No need to notify observers on the parents since their parent
   // chain must have been changed too and so their observers were
   // notified at that time.
 
   nsINode::nsSlots* slots = aContent->GetExistingSlots();
   if (slots && !slots->mMutationObservers.IsEmpty()) {
-    NS_OBSERVER_ARRAY_NOTIFY_XPCOM_OBSERVERS(
+    NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(
         slots->mMutationObservers,
         nsIMutationObserver,
         ParentChainChanged,
         (aContent));
   }
 }
 
 void
--- a/content/base/src/nsRange.cpp
+++ b/content/base/src/nsRange.cpp
@@ -348,16 +348,18 @@ nsRange::ContentRemoved(nsIDocument* aDo
 void
 nsRange::ParentChainChanged(nsIContent *aContent)
 {
   NS_ASSERTION(mRoot == aContent, "Wrong ParentChainChanged notification?");
   nsINode* newRoot = IsValidBoundary(mStartParent);
   NS_ASSERTION(newRoot, "No valid boundary or root found!");
   NS_ASSERTION(newRoot == IsValidBoundary(mEndParent),
                "Start parent and end parent give different root!");
+  // This is safe without holding a strong ref to self as long as the change
+  // of mRoot is the last thing in DoSetRange.
   DoSetRange(mStartParent, mStartOffset, mEndParent, mEndOffset, newRoot);
 }
 
 /********************************************************
  * Utilities for comparing points: API from nsIDOMNSRange
  ********************************************************/
 NS_IMETHODIMP
 nsRange::IsPointInRange(nsIDOMNode* aParent, PRInt32 aOffset, PRBool* aResult)
@@ -468,16 +470,18 @@ nsRange::DoSetRange(nsINode* aStartN, PR
     }
   }
  
   mStartParent = aStartN;
   mStartOffset = aStartOffset;
   mEndParent = aEndN;
   mEndOffset = aEndOffset;
   mIsPositioned = !!mStartParent;
+  // This needs to be the last thing this function does.  See comment
+  // in ParentChainChanged.
   mRoot = aRoot;
 }
 
 static PRInt32
 IndexOf(nsIDOMNode* aChildNode)
 {
   // convert node to nsIContent, so that we can find the child index