Bug 1213589 part.3 ContentEventHandler::GetFlatTextLengthInRange() should handle specially when it's called by nsIMutationObserver::ContentRemoved() r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 02 Dec 2015 13:20:00 +0900
changeset 309284 95efe6bad896355799aaaacea515f003c0c14f7e
parent 309283 d045e3b436223420edaf2933fb281d4f6cc464a3
child 309285 b7191f6c21ae13a810b89041da3adc4d5c153fc3
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1213589
milestone45.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 1213589 part.3 ContentEventHandler::GetFlatTextLengthInRange() should handle specially when it's called by nsIMutationObserver::ContentRemoved() r=smaug
dom/events/ContentEventHandler.cpp
dom/events/ContentEventHandler.h
dom/events/IMEContentObserver.cpp
--- a/dom/events/ContentEventHandler.cpp
+++ b/dom/events/ContentEventHandler.cpp
@@ -1437,56 +1437,86 @@ ContentEventHandler::OnQueryDOMWidgetHit
 }
 
 /* static */ nsresult
 ContentEventHandler::GetFlatTextLengthInRange(
                        const NodePosition& aStartPosition,
                        const NodePosition& aEndPosition,
                        nsIContent* aRootContent,
                        uint32_t* aLength,
-                       LineBreakType aLineBreakType)
+                       LineBreakType aLineBreakType,
+                       bool aIsRemovingNode /* = false */)
 {
   if (NS_WARN_IF(!aRootContent) || NS_WARN_IF(!aStartPosition.IsValid()) ||
       NS_WARN_IF(!aEndPosition.IsValid()) || NS_WARN_IF(!aLength)) {
     return NS_ERROR_INVALID_ARG;
   }
 
-  RefPtr<nsRange> prev = new nsRange(aRootContent);
-  nsresult rv = aStartPosition.SetToRangeStart(prev);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
+  // Don't create nsContentIterator instance until it's really necessary since
+  // destroying without initializing causes unexpected NS_ASSERTION() call.
+  nsCOMPtr<nsIContentIterator> iter;
 
-  nsCOMPtr<nsIContentIterator> iter = NS_NewContentIterator();
-  if (aEndPosition.OffsetIsValid()) {
-    // Offset is within node's length; set end of range to that offset
-    rv = aEndPosition.SetToRangeEnd(prev);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    rv = iter->Init(prev);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-  } else if (aEndPosition.mNode != aRootContent) {
-    // Offset is past node's length; set end of range to end of node
-    rv = aEndPosition.SetToRangeEndAfter(prev);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    rv = iter->Init(prev);
+  // This may be called for retrieving the text of removed nodes.  Even in this
+  // case, the node thinks it's still in the tree because UnbindFromTree() will
+  // be called after here.  However, the node was already removed from the
+  // array of children of its parent.  So, be careful to handle this case.
+  if (aIsRemovingNode) {
+    DebugOnly<nsIContent*> parent = aStartPosition.mNode->GetParent();
+    MOZ_ASSERT(parent && parent->IndexOf(aStartPosition.mNode) == -1,
+      "At removing the node, the node shouldn't be in the array of children "
+      "of its parent");
+    MOZ_ASSERT(aStartPosition.mNode == aEndPosition.mNode,
+      "At removing the node, start and end node should be same");
+    MOZ_ASSERT(aStartPosition.mOffset == 0,
+      "When the node is being removed, the start offset should be 0");
+    MOZ_ASSERT(static_cast<uint32_t>(aEndPosition.mOffset) ==
+                 aEndPosition.mNode->GetChildCount(),
+      "When the node is being removed, the end offset should be child count");
+    iter = NS_NewContentIterator();
+    nsresult rv = iter->Init(aStartPosition.mNode);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   } else {
-    // Offset is past the root node; set end of range to end of root node
-    rv = iter->Init(aRootContent);
+    RefPtr<nsRange> prev = new nsRange(aRootContent);
+    nsresult rv = aStartPosition.SetToRangeStart(prev);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
+
+    if (aEndPosition.OffsetIsValid()) {
+      // Offset is within node's length; set end of range to that offset
+      rv = aEndPosition.SetToRangeEnd(prev);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
+      iter = NS_NewContentIterator();
+      rv = iter->Init(prev);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
+    } else if (aEndPosition.mNode != aRootContent) {
+      // Offset is past node's length; set end of range to end of node
+      rv = aEndPosition.SetToRangeEndAfter(prev);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
+      iter = NS_NewPreContentIterator();
+      rv = iter->Init(prev);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
+    } else {
+      // Offset is past the root node; set end of range to end of root node
+      iter = NS_NewPreContentIterator();
+      rv = iter->Init(aRootContent);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
+    }
   }
 
   *aLength = 0;
   for (; !iter->IsDone(); iter->Next()) {
     nsINode* node = iter->GetCurrentNode();
     if (NS_WARN_IF(!node)) {
       break;
     }
--- a/dom/events/ContentEventHandler.h
+++ b/dom/events/ContentEventHandler.h
@@ -128,21 +128,24 @@ public:
     nsresult SetToRangeEndAfter(nsRange* aRange) const
     {
       nsCOMPtr<nsIDOMNode> domNode(do_QueryInterface(mNode));
       return aRange->SetEndAfter(domNode);
     }
   };
 
   // Get the flatten text length in the range.
+  // @param aIsRemovingNode     Should be true only when this is called from
+  //                            nsIMutationObserver::ContentRemoved().
   static nsresult GetFlatTextLengthInRange(const NodePosition& aStartPosition,
                                            const NodePosition& aEndPosition,
                                            nsIContent* aRootContent,
                                            uint32_t* aLength,
-                                           LineBreakType aLineBreakType);
+                                           LineBreakType aLineBreakType,
+                                           bool aIsRemovingNode = false);
   // Computes the native text length between aStartOffset and aEndOffset of
   // aContent.  aContent must be a text node.
   static uint32_t GetNativeTextLength(nsIContent* aContent,
                                       uint32_t aStartOffset,
                                       uint32_t aEndOffset);
   // Get the native text length of aContent.  aContent must be a text node.
   static uint32_t GetNativeTextLength(nsIContent* aContent,
                                       uint32_t aMaxLength = UINT32_MAX);
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -1011,23 +1011,22 @@ IMEContentObserver::ContentRemoved(nsIDo
     offset = mStartOfRemovingTextRangeCache.mFlatTextLength;
   }
 
   // get offset at the end of the deleted node
   uint32_t textLength = 0;
   if (aChild->IsNodeOfType(nsINode::eTEXT)) {
     textLength = ContentEventHandler::GetNativeTextLength(aChild);
   } else {
-    uint32_t nodeLength =
-      std::max(static_cast<int32_t>(aChild->GetChildCount()), 1);
+    uint32_t nodeLength = static_cast<int32_t>(aChild->GetChildCount());
     rv = ContentEventHandler::GetFlatTextLengthInRange(
                                 NodePosition(aChild, 0),
                                 NodePosition(aChild, nodeLength),
                                 mRootContent, &textLength,
-                                LINE_BREAK_TYPE_NATIVE);
+                                LINE_BREAK_TYPE_NATIVE, true);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       mStartOfRemovingTextRangeCache.Clear();
       return;
     }
   }
 
   if (!textLength) {
     return;