Bug 1423097 - part 2: Add overloads of nsRange::SetStart(), nsRange::SetEnd(), nsRange::IsPointInRange() and nsRange::ComparePoint() to use them with RawRangeBoundary r=smaug

nsRange::SetStart(), nsRange::SetEnd(), nsRange::IsPointInRange() and
nsRange::ComparePoint() take a set of container node and offset in it to
specifying a DOM point.  However, the caller may not have computed the offset
but may know the child node at the point.  In such case, they can avoid
computing the offset with nsINode::IndexOf() if they have overloads which
take RawRangeBoundary.

Therefore, this patch implements the overloads and changes the callers in
editor.

MozReview-Commit-ID: E4DLbAgTTCI

--HG--
extra : rebase_source : 8d1632a030f1e0a0dd2b81c3996c19d427e8b0bd

dom/base/nsRange.cpp

@@ -285,7 +285,7 @@ nsRange::~nsRange()
   NS_ASSERTION(!IsInSelection(), "deleting nsRange that is in use");
 
   // we want the side effects (releases and list removals)
-  DoSetRange(nullptr, 0, nullptr, 0, nullptr);
+  DoSetRange(RawRangeBoundary(), RawRangeBoundary(), nullptr);
 }
 
 nsRange::nsRange(nsINode* aNode)
@@ -859,14 +859,14 @@ nsRange::IsPointInRange(nsIDOMNode* aContainer, uint32_t aOffset, bool* aResult)
   }
 
   ErrorResult rv;
-  *aResult = IsPointInRange(*container, aOffset, rv);
+  *aResult = IsPointInRange(RawRangeBoundary(container, aOffset), rv);
   return rv.StealNSResult();
 }
 
 bool
-nsRange::IsPointInRange(nsINode& aContainer, uint32_t aOffset, ErrorResult& aRv)
+nsRange::IsPointInRange(const RawRangeBoundary& aPoint, ErrorResult& aRv)
 {
-  uint16_t compareResult = ComparePoint(aContainer, aOffset, aRv);
+  uint16_t compareResult = ComparePoint(aPoint, aRv);
   // If the node isn't in the range's document, it clearly isn't in the range.
   if (aRv.ErrorCodeIs(NS_ERROR_DOM_WRONG_DOCUMENT_ERR)) {
     aRv.SuppressException();
@@ -886,42 +886,46 @@ nsRange::ComparePoint(nsIDOMNode* aContainer, uint32_t aOffset,
   NS_ENSURE_TRUE(container, NS_ERROR_DOM_HIERARCHY_REQUEST_ERR);
 
   ErrorResult rv;
-  *aResult = ComparePoint(*container, aOffset, rv);
+  *aResult = ComparePoint(RawRangeBoundary(container, aOffset), rv);
   return rv.StealNSResult();
 }
 
 int16_t
-nsRange::ComparePoint(nsINode& aContainer, uint32_t aOffset, ErrorResult& aRv)
+nsRange::ComparePoint(const RawRangeBoundary& aPoint, ErrorResult& aRv)
 {
+  if (NS_WARN_IF(!aPoint.IsSet())) {
+    // FYI: Shouldn't reach this case if it's called by JS.  Therefore, it's
+    //      okay to warn.
+    aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
+    return 0;
+  }
+
   // our range is in a good state?
   if (!mIsPositioned) {
     aRv.Throw(NS_ERROR_NOT_INITIALIZED);
     return 0;
   }
 
-  if (!nsContentUtils::ContentIsDescendantOf(&aContainer, mRoot)) {
+  if (!nsContentUtils::ContentIsDescendantOf(aPoint.Container(), mRoot)) {
     aRv.Throw(NS_ERROR_DOM_WRONG_DOCUMENT_ERR);
     return 0;
   }
 
-  if (aContainer.NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
+  if (aPoint.Container()->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) {
     aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
     return 0;
   }
 
-  if (aOffset > aContainer.Length()) {
+  if (aPoint.Offset() > aPoint.Container()->Length()) {
     aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
     return 0;
   }
 
-  int32_t cmp = nsContentUtils::ComparePoints(&aContainer, aOffset,
-                                              mStart.Container(),
-                                              mStart.Offset());
+  int32_t cmp = nsContentUtils::ComparePoints(aPoint, mStart.AsRaw());
   if (cmp <= 0) {
     return cmp;
   }
-  if (nsContentUtils::ComparePoints(mEnd.Container(), mEnd.Offset(),
-                                    &aContainer, aOffset) == -1) {
+  if (nsContentUtils::ComparePoints(mEnd.AsRaw(), aPoint) == -1) {
     return 1;
   }
 
@@ -1378,7 +1382,7 @@ nsRange::SetStart(nsINode& aNode, uint32_t aOffset, ErrorResult& aRv)
   }
 
   AutoInvalidateSelection atEndOfBlock(this);
-  aRv = SetStart(&aNode, aOffset);
+  SetStart(RawRangeBoundary(&aNode, aOffset), aRv);
 }
 
 NS_IMETHODIMP
@@ -1394,33 +1398,29 @@ nsRange::SetStart(nsIDOMNode* aContainer, uint32_t aOffset)
   return rv.StealNSResult();
 }
 
-/* virtual */ nsresult
-nsRange::SetStart(nsINode* aContainer, uint32_t aOffset)
+void
+nsRange::SetStart(const RawRangeBoundary& aPoint, ErrorResult& aRv)
 {
-  nsINode* newRoot = IsValidBoundary(aContainer);
+  nsINode* newRoot = IsValidBoundary(aPoint.Container());
   if (!newRoot) {
-    return NS_ERROR_DOM_INVALID_NODE_TYPE_ERR;
+    aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
+    return;
   }
 
-  if (!IsValidOffset(aContainer, aOffset)) {
-    return NS_ERROR_DOM_INDEX_SIZE_ERR;
+  if (!aPoint.IsSetAndValid()) {
+    aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
+    return;
   }
 
   // Collapse if not positioned yet, if positioned in another doc or
   // if the new start is after end.
   if (!mIsPositioned || newRoot != mRoot ||
-      nsContentUtils::ComparePoints(aContainer,
-                                    static_cast<int32_t>(aOffset),
-                                    mEnd.Container(), mEnd.Offset()) == 1) {
-    DoSetRange(aContainer, aOffset, aContainer, aOffset, newRoot);
-
-    return NS_OK;
+      nsContentUtils::ComparePoints(aPoint, mEnd.AsRaw()) == 1) {
+    DoSetRange(aPoint, aPoint, newRoot);
+    return;
   }
 
-  RawRangeBoundary newStart(aContainer, aOffset);
-  DoSetRange(newStart, mEnd.AsRaw(), mRoot);
-
-  return NS_OK;
+  DoSetRange(aPoint, mEnd.AsRaw(), mRoot);
 }
 
 void
@@ -1518,7 +1518,7 @@ nsRange::SetEnd(nsINode& aNode, uint32_t aOffset, ErrorResult& aRv)
     return;
   }
   AutoInvalidateSelection atEndOfBlock(this);
-  aRv = SetEnd(&aNode, aOffset);
+  SetEnd(RawRangeBoundary(&aNode, aOffset), aRv);
 }
 
 NS_IMETHODIMP
@@ -1534,34 +1534,29 @@ nsRange::SetEnd(nsIDOMNode* aContainer, uint32_t aOffset)
   return rv.StealNSResult();
 }
 
-/* virtual */ nsresult
-nsRange::SetEnd(nsINode* aContainer, uint32_t aOffset)
+void
+nsRange::SetEnd(const RawRangeBoundary& aPoint, ErrorResult& aRv)
 {
-  nsINode* newRoot = IsValidBoundary(aContainer);
+  nsINode* newRoot = IsValidBoundary(aPoint.Container());
   if (!newRoot) {
-    return NS_ERROR_DOM_INVALID_NODE_TYPE_ERR;
+    aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
+    return;
   }
 
-  if (!IsValidOffset(aContainer, aOffset)) {
-    return NS_ERROR_DOM_INDEX_SIZE_ERR;
+  if (!aPoint.IsSetAndValid()) {
+    aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
+    return;
   }
 
   // Collapse if not positioned yet, if positioned in another doc or
   // if the new end is before start.
   if (!mIsPositioned || newRoot != mRoot ||
-      nsContentUtils::ComparePoints(mStart.Container(),
-                                    static_cast<int32_t>(mStart.Offset()),
-                                    aContainer,
-                                    static_cast<int32_t>(aOffset)) == 1) {
-    DoSetRange(aContainer, aOffset, aContainer, aOffset, newRoot);
-
-    return NS_OK;
+      nsContentUtils::ComparePoints(mStart.AsRaw(), aPoint) == 1) {
+    DoSetRange(aPoint, aPoint, newRoot);
+    return;
   }
 
-  RawRangeBoundary newEnd(aContainer, aOffset);
-  DoSetRange(mStart.AsRaw(), newEnd, mRoot);
-
-  return NS_OK;
+  DoSetRange(mStart.AsRaw(), aPoint, mRoot);
 }
 
 void
@@ -1642,18 +1637,6 @@ nsRange::SetStartAndEnd(const RawRangeBoundary& aStart,
   return NS_OK;
 }
 
-nsresult
-nsRange::SetStartAndEnd(nsINode* aStartContainer, uint32_t aStartOffset,
-                        nsINode* aEndContainer, uint32_t aEndOffset)
-{
-  if (NS_WARN_IF(!aStartContainer) || NS_WARN_IF(!aEndContainer)) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
-  return SetStartAndEnd(RawRangeBoundary(aStartContainer, aStartOffset),
-                        RawRangeBoundary(aEndContainer, aEndOffset));
-}
-
 void
 nsRange::SetEndBeforeJS(nsINode& aNode, ErrorResult& aErr)
 {
@@ -1804,7 +1787,8 @@ nsRange::SelectNode(nsINode& aNode, ErrorResult& aRv)
   }
 
   AutoInvalidateSelection atEndOfBlock(this);
-  DoSetRange(container, index, container, index + 1, newRoot);
+  DoSetRange(RawRangeBoundary(container, index),
+             RawRangeBoundary(container, index + 1), newRoot);
 }
 
 NS_IMETHODIMP
@@ -1842,7 +1826,8 @@ nsRange::SelectNodeContents(nsINode& aNode, ErrorResult& aRv)
   }
 
   AutoInvalidateSelection atEndOfBlock(this);
-  DoSetRange(&aNode, 0, &aNode, aNode.Length(), newRoot);
+  DoSetRange(RawRangeBoundary(&aNode, 0),
+             RawRangeBoundary(&aNode, aNode.Length()), newRoot);
 }
 
 // The Subtree Content Iterator only returns subtrees that are

dom/base/nsRange.h

@@ -182,8 +182,18 @@ public:
    * When you set both start and end of a range, you should use
    * SetStartAndEnd() instead.
    */
-  nsresult SetStart(nsINode* aContainer, uint32_t aOffset);
-  nsresult SetEnd(nsINode* aContainer, uint32_t aOffset);
+  nsresult SetStart(nsINode* aContainer, uint32_t aOffset)
+  {
+    ErrorResult error;
+    SetStart(RawRangeBoundary(aContainer, aOffset), error);
+    return error.StealNSResult();
+  }
+  nsresult SetEnd(nsINode* aContainer, uint32_t aOffset)
+  {
+    ErrorResult error;
+    SetEnd(RawRangeBoundary(aContainer, aOffset), error);
+    return error.StealNSResult();
+  }
 
   already_AddRefed<nsRange> CloneRange() const;
 
@@ -196,7 +206,11 @@ public:
    * the range will be collapsed at the end point.
    */
   nsresult SetStartAndEnd(nsINode* aStartContainer, uint32_t aStartOffset,
-                          nsINode* aEndContainer, uint32_t aEndOffset);
+                          nsINode* aEndContainer, uint32_t aEndOffset)
+  {
+    return SetStartAndEnd(RawRangeBoundary(aStartContainer, aStartOffset),
+                          RawRangeBoundary(aEndContainer, aEndOffset));
+  }
   nsresult SetStartAndEnd(const RawRangeBoundary& aStart,
                           const RawRangeBoundary& aEnd);
 
@@ -290,7 +304,11 @@ public:
   int16_t CompareBoundaryPoints(uint16_t aHow, nsRange& aOther,
                                 ErrorResult& aErr);
   int16_t ComparePoint(nsINode& aContainer, uint32_t aOffset,
-                       ErrorResult& aErr);
+                       ErrorResult& aErr)
+  {
+    return ComparePoint(RawRangeBoundary(&aContainer, aOffset), aErr);
+  }
+  int16_t ComparePoint(const RawRangeBoundary& aPoint, ErrorResult& aErr);
   void DeleteContents(ErrorResult& aRv);
   already_AddRefed<mozilla::dom::DocumentFragment>
     ExtractContents(ErrorResult& aErr);
@@ -301,7 +319,11 @@ public:
   uint32_t GetEndOffset(ErrorResult& aRv) const;
   void InsertNode(nsINode& aNode, ErrorResult& aErr);
   bool IntersectsNode(nsINode& aNode, ErrorResult& aRv);
-  bool IsPointInRange(nsINode& aContainer, uint32_t aOffset, ErrorResult& aErr);
+  bool IsPointInRange(nsINode& aContainer, uint32_t aOffset, ErrorResult& aErr)
+  {
+    return IsPointInRange(RawRangeBoundary(&aContainer, aOffset), aErr);
+  }
+  bool IsPointInRange(const RawRangeBoundary& aPoint, ErrorResult& aErr);
 
   // *JS() methods are mapped to Range.*() of DOM.
   // They may move focus only when the range represents normal selection.
@@ -329,9 +351,11 @@ public:
   void SelectNode(nsINode& aNode, ErrorResult& aErr);
   void SelectNodeContents(nsINode& aNode, ErrorResult& aErr);
   void SetEnd(nsINode& aNode, uint32_t aOffset, ErrorResult& aErr);
+  void SetEnd(const RawRangeBoundary& aPoint, ErrorResult& aErr);
   void SetEndAfter(nsINode& aNode, ErrorResult& aErr);
   void SetEndBefore(nsINode& aNode, ErrorResult& aErr);
   void SetStart(nsINode& aNode, uint32_t aOffset, ErrorResult& aErr);
+  void SetStart(const RawRangeBoundary& aPoint, ErrorResult& aErr);
   void SetStartAfter(nsINode& aNode, ErrorResult& aErr);
   void SetStartBefore(nsINode& aNode, ErrorResult& aErr);
 
@@ -469,16 +493,6 @@ protected:
                   const RawRangeBoundary& upperBound,
                   nsINode* aRoot, bool aNotInsertedYet = false);
 
-  void DoSetRange(nsINode* aStartContainer, uint32_t aStartOffset,
-                  nsINode* aEndContainer, uint32_t aEndOffset,
-                  nsINode* aRoot, bool aNotInsertedYet = false)
-  {
-    RawRangeBoundary start(aStartContainer, aStartOffset);
-    RawRangeBoundary end(aEndContainer, aEndOffset);
-
-    DoSetRange(start, end, aRoot, aNotInsertedYet);
-  }
-
   /**
    * For a range for which IsInSelection() is true, return the common ancestor
    * for the range, which we had to compute when the common ancestor changed or

editor/libeditor/HTMLEditRules.cpp

@@ -6116,8 +6116,10 @@ HTMLEditRules::GetNodesForOperation(
         // The new end parent becomes the parent node of the text.
         // XXX We want nsRange::SetEnd(const RawRangeBoundary&)
         EditorRawDOMPoint atContainerOfSplitNode(atEnd.Container());
-        range->SetEnd(atContainerOfSplitNode.Container(),
-                      atContainerOfSplitNode.Offset());
+        range->SetEnd(atContainerOfSplitNode, error);
+        if (NS_WARN_IF(error.Failed())) {
+          error.SuppressException();
+        }
       }
     }
   }
@@ -6614,8 +6616,11 @@ HTMLEditRules::GetNodesFromPoint(
     return NS_ERROR_INVALID_ARG;
   }
   RefPtr<nsRange> range = new nsRange(aPoint.Container());
-  DebugOnly<nsresult> rv = range->SetStart(aPoint.Container(), aPoint.Offset());
-  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  IgnoredErrorResult error;
+  range->SetStart(aPoint, error);
+  if (NS_WARN_IF(error.Failed())) {
+    MOZ_ASSERT(!error.Failed());
+  }
 
   // Expand the range to include adjacent inlines
   PromoteRange(*range, aOperation);
@@ -8640,16 +8645,19 @@ HTMLEditRules::ConfirmSelectionInBody()
 nsresult
 HTMLEditRules::UpdateDocChangeRange(nsRange* aRange)
 {
+  if (NS_WARN_IF(!mHTMLEditor)) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   // first make sure aRange is in the document.  It might not be if
   // portions of our editting action involved manipulating nodes
   // prior to placing them in the document (e.g., populating a list item
   // before placing it in its list)
-  nsCOMPtr<nsINode> startNode = aRange->GetStartContainer();
-  if (NS_WARN_IF(!startNode)) {
+  const RangeBoundary& atStart = aRange->StartRef();
+  if (NS_WARN_IF(!atStart.IsSet())) {
     return NS_ERROR_FAILURE;
   }
-  NS_ENSURE_STATE(mHTMLEditor);
-  if (!mHTMLEditor->IsDescendantOfRoot(startNode)) {
+  if (!mHTMLEditor->IsDescendantOfRoot(atStart.Container())) {
     // just return - we don't need to adjust mDocChangeRange in this case
     return NS_OK;
   }
@@ -8675,8 +8683,11 @@ HTMLEditRules::UpdateDocChangeRange(nsRange* aRange)
     NS_ENSURE_SUCCESS(rv, rv);
     // Positive result means mDocChangeRange start is after aRange start.
     if (result > 0) {
-      rv = mDocChangeRange->SetStart(startNode, aRange->StartOffset());
-      NS_ENSURE_SUCCESS(rv, rv);
+      ErrorResult error;
+      mDocChangeRange->SetStart(atStart.AsRaw(), error);
+      if (NS_WARN_IF(error.Failed())) {
+        return error.StealNSResult();
+      }
     }
 
     // compare ends of ranges
@@ -8685,12 +8696,15 @@ HTMLEditRules::UpdateDocChangeRange(nsRange* aRange)
     NS_ENSURE_SUCCESS(rv, rv);
     // Negative result means mDocChangeRange end is before aRange end.
     if (result < 0) {
-      nsINode* endNode = aRange->GetEndContainer();
-      if (NS_WARN_IF(!endNode)) {
+      const RangeBoundary& atEnd = aRange->EndRef();
+      if (NS_WARN_IF(!atEnd.IsSet())) {
         return NS_ERROR_FAILURE;
       }
-      rv = mDocChangeRange->SetEnd(endNode, aRange->EndOffset());
-      NS_ENSURE_SUCCESS(rv, rv);
+      ErrorResult error;
+      mDocChangeRange->SetEnd(atEnd.AsRaw(), error);
+      if (NS_WARN_IF(error.Failed())) {
+        return error.StealNSResult();
+      }
     }
   }
   return NS_OK;