Bug 1415414 - Make DeleteRangeTransaction::CreateTxnsToDeleteBetween() and DeleteRangeTransaction::CreateTxnsToDeleteContent() use RawRangeBoundary as their arguments r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 08 Nov 2017 15:01:23 +0900
changeset 444219 2d6c76ecff8dcb7cfce961885e7e0fa2f1b608c3
parent 444218 14e4883dc59825c0e30a482d1903ad87aff4afdd
child 444220 818860c42bcaa778c3c88bec897bc7bb1f417f3b
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1415414, 1407352
milestone58.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 1415414 - Make DeleteRangeTransaction::CreateTxnsToDeleteBetween() and DeleteRangeTransaction::CreateTxnsToDeleteContent() use RawRangeBoundary as their arguments r=m_kato By the fix of bug 1407352, DeleteRangeTransaction::CreateTxnsToDeleteBetween() starts to use child node instead of a set of container node and offset in it. However, this is ugly and using RawRangeBoundary may reduce computing offset more. So, they should use RawRangeBoundary to refer delete points. MozReview-Commit-ID: pwDHOxpz0E
editor/libeditor/DeleteRangeTransaction.cpp
editor/libeditor/DeleteRangeTransaction.h
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -6,16 +6,17 @@
 #include "DeleteRangeTransaction.h"
 
 #include "DeleteNodeTransaction.h"
 #include "DeleteTextTransaction.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/EditorBase.h"
 #include "mozilla/dom/Selection.h"
 #include "mozilla/mozalloc.h"
+#include "mozilla/RangeBoundary.h"
 #include "nsCOMPtr.h"
 #include "nsDebug.h"
 #include "nsError.h"
 #include "nsIContent.h"
 #include "nsIContentIterator.h"
 #include "nsINode.h"
 #include "nsAString.h"
 
@@ -51,56 +52,64 @@ DeleteRangeTransaction::DoTransaction()
   // Swap mRangeToDelete out into a stack variable, so we make sure to null it
   // out on return from this function.  Once this function returns, we no longer
   // need mRangeToDelete, and keeping it alive in the long term slows down all
   // DOM mutations because it's observing them.
   RefPtr<nsRange> rangeToDelete;
   rangeToDelete.swap(mRangeToDelete);
 
   // build the child transactions
-  nsCOMPtr<nsINode> startContainer = rangeToDelete->GetStartContainer();
-  int32_t startOffset = rangeToDelete->StartOffset();
-  nsCOMPtr<nsINode> endContainer = rangeToDelete->GetEndContainer();
-  int32_t endOffset = rangeToDelete->EndOffset();
-  MOZ_ASSERT(startContainer && endContainer);
+  const RangeBoundary& startRef = rangeToDelete->StartRef();
+  const RangeBoundary& endRef = rangeToDelete->EndRef();
+  MOZ_ASSERT(startRef.IsSetAndValid());
+  MOZ_ASSERT(endRef.IsSetAndValid());
 
-  if (startContainer == endContainer) {
+  if (startRef.Container() == endRef.Container()) {
     // the selection begins and ends in the same node
-    nsIContent* startChild = rangeToDelete->GetChildAtStartOffset();
-    nsresult rv =
-      CreateTxnsToDeleteBetween(startContainer, startOffset,
-                                startChild, endOffset);
-    NS_ENSURE_SUCCESS(rv, rv);
+    nsresult rv = CreateTxnsToDeleteBetween(startRef.AsRaw(), endRef.AsRaw());
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   } else {
     // the selection ends in a different node from where it started.  delete
     // the relevant content in the start node
-    nsresult rv =
-      CreateTxnsToDeleteContent(startContainer, startOffset, nsIEditor::eNext);
-    NS_ENSURE_SUCCESS(rv, rv);
+    nsresult rv = CreateTxnsToDeleteContent(startRef.AsRaw(), nsIEditor::eNext);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     // delete the intervening nodes
     rv = CreateTxnsToDeleteNodesBetween(rangeToDelete);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     // delete the relevant content in the end node
-    rv = CreateTxnsToDeleteContent(endContainer, endOffset,
-                                   nsIEditor::ePrevious);
-    NS_ENSURE_SUCCESS(rv, rv);
+    rv = CreateTxnsToDeleteContent(endRef.AsRaw(), nsIEditor::ePrevious);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
 
   // if we've successfully built this aggregate transaction, then do it.
   nsresult rv = EditAggregateTransaction::DoTransaction();
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   // only set selection to deletion point if editor gives permission
   bool bAdjustSelection;
   mEditorBase->ShouldTxnSetSelection(&bAdjustSelection);
   if (bAdjustSelection) {
     RefPtr<Selection> selection = mEditorBase->GetSelection();
-    NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
-    rv = selection->Collapse(startContainer, startOffset);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(!selection)) {
+      return NS_ERROR_NULL_POINTER;
+    }
+    rv = selection->Collapse(startRef.AsRaw());
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
   // else do nothing - dom range gravity will adjust selection
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DeleteRangeTransaction::UndoTransaction()
@@ -117,108 +126,118 @@ DeleteRangeTransaction::RedoTransaction(
 NS_IMETHODIMP
 DeleteRangeTransaction::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("DeleteRangeTransaction");
   return NS_OK;
 }
 
 nsresult
-DeleteRangeTransaction::CreateTxnsToDeleteBetween(nsINode* aNode,
-                                                  int32_t aStartOffset,
-                                                  nsIContent* aChildAtStartOffset,
-                                                  int32_t aEndOffset)
+DeleteRangeTransaction::CreateTxnsToDeleteBetween(
+                          const RawRangeBoundary& aStart,
+                          const RawRangeBoundary& aEnd)
 {
+  if (NS_WARN_IF(!aStart.IsSetAndValid()) ||
+      NS_WARN_IF(!aEnd.IsSetAndValid()) ||
+      NS_WARN_IF(aStart.Container() != aEnd.Container())) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   if (NS_WARN_IF(!mEditorBase)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // see what kind of node we have
-  if (aNode->IsNodeOfType(nsINode::eDATA_NODE)) {
+  if (aStart.Container()->IsNodeOfType(nsINode::eDATA_NODE)) {
     // if the node is a chardata node, then delete chardata content
     int32_t numToDel;
-    if (aStartOffset == aEndOffset) {
+    if (aStart == aEnd) {
       numToDel = 1;
     } else {
-      numToDel = aEndOffset - aStartOffset;
+      numToDel = aEnd.Offset() - aStart.Offset();
+      MOZ_DIAGNOSTIC_ASSERT(numToDel > 0);
     }
 
     RefPtr<nsGenericDOMDataNode> charDataNode =
-      static_cast<nsGenericDOMDataNode*>(aNode);
+      static_cast<nsGenericDOMDataNode*>(aStart.Container());
 
     RefPtr<DeleteTextTransaction> deleteTextTransaction =
-      new DeleteTextTransaction(*mEditorBase, *charDataNode, aStartOffset,
+      new DeleteTextTransaction(*mEditorBase, *charDataNode, aStart.Offset(),
                                 numToDel, mRangeUpdater);
     // If the text node isn't editable, it should be never undone/redone.
     // So, the transaction shouldn't be recorded.
     if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
       return NS_ERROR_FAILURE;
     }
     AppendChild(deleteTextTransaction);
     return NS_OK;
   }
 
-  nsIContent* child = aChildAtStartOffset;
-  for (int32_t i = aStartOffset; i < aEndOffset; ++i) {
-    // Even if we detect invalid range, we should ignore it for removing
-    // specified range's nodes as far as possible.
-    if (NS_WARN_IF(!child)) {
-      break;
-    }
+  // Even if we detect invalid range, we should ignore it for removing
+  // specified range's nodes as far as possible.
+  for (nsIContent* child = aStart.GetChildAtOffset();
+       child && child != aEnd.GetChildAtOffset();
+       child = child->GetNextSibling()) {
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
       new DeleteNodeTransaction(*mEditorBase, *child, mRangeUpdater);
     // XXX This is odd handling.  Even if some children are not editable,
     //     editor should append transactions because they could be editable
     //     at undoing/redoing.  Additionally, if the transaction needs to
     //     delete/restore all nodes, it should at undoing/redoing.
     if (deleteNodeTransaction->CanDoIt()) {
       AppendChild(deleteNodeTransaction);
     }
-    child = child->GetNextSibling();
   }
 
   return NS_OK;
 }
 
 nsresult
-DeleteRangeTransaction::CreateTxnsToDeleteContent(nsINode* aNode,
-                                                  int32_t aOffset,
-                                                  nsIEditor::EDirection aAction)
+DeleteRangeTransaction::CreateTxnsToDeleteContent(
+                          const RawRangeBoundary& aPoint,
+                          nsIEditor::EDirection aAction)
 {
+  if (NS_WARN_IF(!aPoint.IsSetAndValid())) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   if (NS_WARN_IF(!mEditorBase)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  // see what kind of node we have
-  if (aNode->IsNodeOfType(nsINode::eDATA_NODE)) {
-    // if the node is a chardata node, then delete chardata content
-    uint32_t start, numToDelete;
-    if (nsIEditor::eNext == aAction) {
-      start = aOffset;
-      numToDelete = aNode->Length() - aOffset;
-    } else {
-      start = 0;
-      numToDelete = aOffset;
-    }
+  if (!aPoint.Container()->IsNodeOfType(nsINode::eDATA_NODE)) {
+    return NS_OK;
+  }
+
+  // If the node is a chardata node, then delete chardata content
+  uint32_t startOffset, numToDelete;
+  if (nsIEditor::eNext == aAction) {
+    startOffset = aPoint.Offset();
+    numToDelete = aPoint.Container()->Length() - aPoint.Offset();
+  } else {
+    startOffset = 0;
+    numToDelete = aPoint.Offset();
+  }
 
-    if (numToDelete) {
-      RefPtr<nsGenericDOMDataNode> dataNode =
-        static_cast<nsGenericDOMDataNode*>(aNode);
-      RefPtr<DeleteTextTransaction> deleteTextTransaction =
-        new DeleteTextTransaction(*mEditorBase, *dataNode, start, numToDelete,
-                                  mRangeUpdater);
-      // If the text node isn't editable, it should be never undone/redone.
-      // So, the transaction shouldn't be recorded.
-      if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
-        return NS_ERROR_FAILURE;
-      }
-      AppendChild(deleteTextTransaction);
-    }
+  if (!numToDelete) {
+    return NS_OK;
   }
 
+  RefPtr<nsGenericDOMDataNode> dataNode =
+    static_cast<nsGenericDOMDataNode*>(aPoint.Container());
+  RefPtr<DeleteTextTransaction> deleteTextTransaction =
+    new DeleteTextTransaction(*mEditorBase, *dataNode, startOffset, numToDelete,
+                              mRangeUpdater);
+  // If the text node isn't editable, it should be never undone/redone.
+  // So, the transaction shouldn't be recorded.
+  if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
+    return NS_ERROR_FAILURE;
+  }
+  AppendChild(deleteTextTransaction);
+
   return NS_OK;
 }
 
 nsresult
 DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween(nsRange* aRangeToDelete)
 {
   if (NS_WARN_IF(!mEditorBase)) {
     return NS_ERROR_NOT_AVAILABLE;
--- a/editor/libeditor/DeleteRangeTransaction.h
+++ b/editor/libeditor/DeleteRangeTransaction.h
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef DeleteRangeTransaction_h
 #define DeleteRangeTransaction_h
 
 #include "EditAggregateTransaction.h"
+#include "mozilla/RangeBoundary.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsID.h"
 #include "nsIEditor.h"
 #include "nsISupportsImpl.h"
 #include "nsRange.h"
 #include "nscore.h"
 
 class nsINode;
@@ -45,25 +46,63 @@ public:
 
   virtual void LastRelease() override
   {
     mRangeToDelete = nullptr;
     EditAggregateTransaction::LastRelease();
   }
 
 protected:
-  nsresult CreateTxnsToDeleteBetween(nsINode* aNode,
-                                     int32_t aStartOffset,
-                                     nsIContent* aChildAtStartOffset,
-                                     int32_t aEndOffset);
+  /**
+   * CreateTxnsToDeleteBetween() creates a DeleteTextTransaction or some
+   * DeleteNodeTransactions to remove text or nodes between aStart and aEnd
+   * and appends the created transactions to the array.
+   *
+   * @param aStart      Must be set and valid point.
+   * @param aEnd        Must be set and valid point.  Additionally, the
+   *                    container must be same as aStart's container.
+   *                    And of course, this must not be before aStart in
+   *                    the DOM tree order.
+   * @return            Returns NS_OK in most cases.
+   *                    When the arguments are invalid, returns
+   *                    NS_ERROR_INVALID_ARG.
+   *                    When mEditorBase isn't available, returns
+   *                    NS_ERROR_NOT_AVAIALBLE.
+   *                    When created DeleteTextTransaction cannot do its
+   *                    transaction, returns NS_ERROR_FAILURE.
+   *                    Note that even if one of created DeleteNodeTransaction
+   *                    cannot do its transaction, this returns NS_OK.
+   */
+  nsresult CreateTxnsToDeleteBetween(const RawRangeBoundary& aStart,
+                                     const RawRangeBoundary& aEnd);
 
   nsresult CreateTxnsToDeleteNodesBetween(nsRange* aRangeToDelete);
 
-  nsresult CreateTxnsToDeleteContent(nsINode* aParent,
-                                     int32_t aOffset,
+  /**
+   * CreateTxnsToDeleteContent() creates a DeleteTextTransaction to delete
+   * text between start of aPoint.Container() and aPoint or aPoint and end of
+   * aPoint.Container() and appends the created transaction to the array.
+   *
+   * @param aPoint      Must be set and valid point.  If the container is not
+   *                    a data node, this method does nothing.
+   * @param aAction     If nsIEditor::eNext, this method creates a transaction
+   *                    to delete text from aPoint to the end of the data node.
+   *                    Otherwise, this method creates a transaction to delete
+   *                    text from start of the data node to aPoint.
+   * @return            Returns NS_OK in most cases.
+   *                    When the arguments are invalid, returns
+   *                    NS_ERROR_INVALID_ARG.
+   *                    When mEditorBase isn't available, returns
+   *                    NS_ERROR_NOT_AVAIALBLE.
+   *                    When created DeleteTextTransaction cannot do its
+   *                    transaction, returns NS_ERROR_FAILURE.
+   *                    Note that even if no character will be deleted,
+   *                    this returns NS_OK.
+   */
+  nsresult CreateTxnsToDeleteContent(const RawRangeBoundary& aPoint,
                                      nsIEditor::EDirection aAction);
 
   // The editor for this transaction.
   RefPtr<EditorBase> mEditorBase;
 
   // P1 in the range.  This is only non-null until DoTransaction is called and
   // we convert it into child transactions.
   RefPtr<nsRange> mRangeToDelete;