Bug 1345690 part.2 Make the constructor of DeleteRangeTransaction initialize all members instead of Init() r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 09 Mar 2017 11:19:39 +0900
changeset 496444 1be2f10d340c7c48dcd677740c448fdb51d2ae86
parent 496443 f7c2ff36326ab988f2aa7dd5d29f467e1688e1a4
child 496445 ecd332bf08c824f5098dbcf590a98b9c6e668722
push id48594
push usermasayuki@d-toybox.com
push dateFri, 10 Mar 2017 04:55:38 +0000
reviewersm_kato
bugs1345690
milestone55.0a1
Bug 1345690 part.2 Make the constructor of DeleteRangeTransaction initialize all members instead of Init() r?m_kato Similar to DeleteNodeTransaction, DeleteRangeTransaction should take all necessary information as its arguments. However, different from DeleteNodeTransaction, this doesn't need to implement CanDoIt() since nobody checks the state. MozReview-Commit-ID: 2Z9fNtGeJ9c
editor/libeditor/DeleteRangeTransaction.cpp
editor/libeditor/DeleteRangeTransaction.h
editor/libeditor/EditorBase.cpp
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -19,60 +19,42 @@
 #include "nsINode.h"
 #include "nsAString.h"
 
 namespace mozilla {
 
 using namespace dom;
 
 // note that aEditorBase is not refcounted
-DeleteRangeTransaction::DeleteRangeTransaction()
-  : mEditorBase(nullptr)
-  , mRangeUpdater(nullptr)
+DeleteRangeTransaction::DeleteRangeTransaction(EditorBase& aEditorBase,
+                                               nsRange& aRangeToDelete,
+                                               RangeUpdater* aRangeUpdater)
+  : mEditorBase(aEditorBase)
+  , mRangeToDelete(aRangeToDelete.CloneRange())
+  , mRangeUpdater(aRangeUpdater)
 {
 }
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(DeleteRangeTransaction,
                                    EditAggregateTransaction,
-                                   mRange)
+                                   mRangeToDelete)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DeleteRangeTransaction)
 NS_INTERFACE_MAP_END_INHERITING(EditAggregateTransaction)
 
-nsresult
-DeleteRangeTransaction::Init(EditorBase* aEditorBase,
-                             nsRange* aRange,
-                             RangeUpdater* aRangeUpdater)
-{
-  MOZ_ASSERT(aEditorBase && aRange);
-
-  mEditorBase = aEditorBase;
-  mRange = aRange->CloneRange();
-  mRangeUpdater = aRangeUpdater;
-
-  NS_ENSURE_TRUE(mEditorBase->IsModifiableNode(mRange->GetStartParent()),
-                 NS_ERROR_FAILURE);
-  NS_ENSURE_TRUE(mEditorBase->IsModifiableNode(mRange->GetEndParent()),
-                 NS_ERROR_FAILURE);
-  NS_ENSURE_TRUE(mEditorBase->IsModifiableNode(mRange->GetCommonAncestor()),
-                 NS_ERROR_FAILURE);
-
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 DeleteRangeTransaction::DoTransaction()
 {
-  MOZ_ASSERT(mRange && mEditorBase);
+  MOZ_ASSERT(mRangeToDelete);
 
   // build the child transactions
-  nsCOMPtr<nsINode> startParent = mRange->GetStartParent();
-  int32_t startOffset = mRange->StartOffset();
-  nsCOMPtr<nsINode> endParent = mRange->GetEndParent();
-  int32_t endOffset = mRange->EndOffset();
+  nsCOMPtr<nsINode> startParent = mRangeToDelete->GetStartParent();
+  int32_t startOffset = mRangeToDelete->StartOffset();
+  nsCOMPtr<nsINode> endParent = mRangeToDelete->GetEndParent();
+  int32_t endOffset = mRangeToDelete->EndOffset();
   MOZ_ASSERT(startParent && endParent);
 
   if (startParent == endParent) {
     // the selection begins and ends in the same node
     nsresult rv =
       CreateTxnsToDeleteBetween(startParent, startOffset, endOffset);
     NS_ENSURE_SUCCESS(rv, rv);
   } else {
@@ -90,40 +72,40 @@ DeleteRangeTransaction::DoTransaction()
   }
 
   // if we've successfully built this aggregate transaction, then do it.
   nsresult rv = EditAggregateTransaction::DoTransaction();
   NS_ENSURE_SUCCESS(rv, rv);
 
   // only set selection to deletion point if editor gives permission
   bool bAdjustSelection;
-  mEditorBase->ShouldTxnSetSelection(&bAdjustSelection);
+  mEditorBase.ShouldTxnSetSelection(&bAdjustSelection);
   if (bAdjustSelection) {
-    RefPtr<Selection> selection = mEditorBase->GetSelection();
+    RefPtr<Selection> selection = mEditorBase.GetSelection();
     NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
     rv = selection->Collapse(startParent, startOffset);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   // else do nothing - dom range gravity will adjust selection
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DeleteRangeTransaction::UndoTransaction()
 {
-  MOZ_ASSERT(mRange && mEditorBase);
+  MOZ_ASSERT(mRangeToDelete);
 
   return EditAggregateTransaction::UndoTransaction();
 }
 
 NS_IMETHODIMP
 DeleteRangeTransaction::RedoTransaction()
 {
-  MOZ_ASSERT(mRange && mEditorBase);
+  MOZ_ASSERT(mRangeToDelete);
 
   return EditAggregateTransaction::RedoTransaction();
 }
 
 NS_IMETHODIMP
 DeleteRangeTransaction::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("DeleteRangeTransaction");
@@ -144,17 +126,17 @@ DeleteRangeTransaction::CreateTxnsToDele
     } else {
       numToDel = aEndOffset - aStartOffset;
     }
 
     RefPtr<nsGenericDOMDataNode> charDataNode =
       static_cast<nsGenericDOMDataNode*>(aNode);
 
     RefPtr<DeleteTextTransaction> transaction =
-      new DeleteTextTransaction(*mEditorBase, *charDataNode, aStartOffset,
+      new DeleteTextTransaction(mEditorBase, *charDataNode, aStartOffset,
                                 numToDel, mRangeUpdater);
 
     nsresult rv = transaction->Init();
     NS_ENSURE_SUCCESS(rv, rv);
 
     AppendChild(transaction);
     return NS_OK;
   }
@@ -162,17 +144,17 @@ DeleteRangeTransaction::CreateTxnsToDele
   nsCOMPtr<nsIContent> child = aNode->GetChildAt(aStartOffset);
   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;
     }
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-      new DeleteNodeTransaction(*mEditorBase, *child, mRangeUpdater);
+      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();
@@ -197,17 +179,17 @@ DeleteRangeTransaction::CreateTxnsToDele
       start = 0;
       numToDelete = aOffset;
     }
 
     if (numToDelete) {
       RefPtr<nsGenericDOMDataNode> dataNode =
         static_cast<nsGenericDOMDataNode*>(aNode);
       RefPtr<DeleteTextTransaction> transaction =
-        new DeleteTextTransaction(*mEditorBase, *dataNode, start, numToDelete,
+        new DeleteTextTransaction(mEditorBase, *dataNode, start, numToDelete,
                                   mRangeUpdater);
 
       nsresult rv = transaction->Init();
       NS_ENSURE_SUCCESS(rv, rv);
 
       AppendChild(transaction);
     }
   }
@@ -215,27 +197,27 @@ DeleteRangeTransaction::CreateTxnsToDele
   return NS_OK;
 }
 
 nsresult
 DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween()
 {
   nsCOMPtr<nsIContentIterator> iter = NS_NewContentSubtreeIterator();
 
-  nsresult rv = iter->Init(mRange);
+  nsresult rv = iter->Init(mRangeToDelete);
   NS_ENSURE_SUCCESS(rv, rv);
 
   while (!iter->IsDone()) {
     nsCOMPtr<nsINode> node = iter->GetCurrentNode();
     if (NS_WARN_IF(!node)) {
       return NS_ERROR_NULL_POINTER;
     }
 
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-      new DeleteNodeTransaction(*mEditorBase, *node, mRangeUpdater);
+      new DeleteNodeTransaction(mEditorBase, *node, mRangeUpdater);
     // XXX This is odd handling.  Even if some nodes in the range are not
     //     editable, editor should append transactions because they could
     //     at undoing/redoing.  Additionally, if the transaction needs to
     //     delete/restore all nodes, it should at undoing/redoing.
     if (NS_WARN_IF(!deleteNodeTransaction->CanDoIt())) {
       return NS_ERROR_FAILURE;
     }
     AppendChild(deleteNodeTransaction);
--- a/editor/libeditor/DeleteRangeTransaction.h
+++ b/editor/libeditor/DeleteRangeTransaction.h
@@ -23,56 +23,53 @@ class RangeUpdater;
 
 /**
  * A transaction that deletes an entire range in the content tree
  */
 class DeleteRangeTransaction final : public EditAggregateTransaction
 {
 public:
   /**
-   * Initialize the transaction.
-   * @param aEditorBase The object providing basic editing operations.
-   * @param aRange      The range to delete.
+   * @param aEditorBase         The object providing basic editing operations.
+   * @param aRangeToDelete      The range to delete.
    */
-  nsresult Init(EditorBase* aEditorBase,
-                nsRange* aRange,
-                RangeUpdater* aRangeUpdater);
-
-  DeleteRangeTransaction();
+  DeleteRangeTransaction(EditorBase& aEditorBase,
+                         nsRange& aRangeToDelete,
+                         RangeUpdater* aRangeUpdater);
 
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(DeleteRangeTransaction,
                                            EditAggregateTransaction)
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
 
   NS_DECL_EDITTRANSACTIONBASE
 
   NS_IMETHOD RedoTransaction() override;
 
   virtual void LastRelease() override
   {
-    mRange = nullptr;
+    mRangeToDelete = nullptr;
     EditAggregateTransaction::LastRelease();
   }
 
 protected:
   nsresult CreateTxnsToDeleteBetween(nsINode* aNode,
                                      int32_t aStartOffset,
                                      int32_t aEndOffset);
 
   nsresult CreateTxnsToDeleteNodesBetween();
 
   nsresult CreateTxnsToDeleteContent(nsINode* aParent,
                                      int32_t aOffset,
                                      nsIEditor::EDirection aAction);
 
-  // P1 in the range.
-  RefPtr<nsRange> mRange;
+  // The editor for this transaction.
+  EditorBase& mEditorBase;
 
-  // The editor for this transaction.
-  EditorBase* mEditorBase;
+  // P1 in the range.
+  RefPtr<nsRange> mRangeToDelete;
 
   // Range updater object.
   RangeUpdater* mRangeUpdater;
 };
 
 } // namespace mozilla
 
 #endif // #ifndef DeleteRangeTransaction_h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -4363,19 +4363,20 @@ EditorBase::CreateTxnForDeleteSelection(
 
   for (uint32_t rangeIdx = 0; rangeIdx < selection->RangeCount(); ++rangeIdx) {
     RefPtr<nsRange> range = selection->GetRangeAt(rangeIdx);
     NS_ENSURE_STATE(range);
 
     // Same with range as with selection; if it is collapsed and action
     // is eNone, do nothing.
     if (!range->Collapsed()) {
-      RefPtr<DeleteRangeTransaction> transaction = new DeleteRangeTransaction();
-      transaction->Init(this, range, &mRangeUpdater);
-      aggregateTransaction->AppendChild(transaction);
+      RefPtr<DeleteRangeTransaction> deleteRangeTransaction =
+        new DeleteRangeTransaction(*this, *range, &mRangeUpdater);
+      // XXX Oh, not checking if deleteRangeTransaction can modify the range...
+      aggregateTransaction->AppendChild(deleteRangeTransaction);
     } else if (aAction != eNone) {
       // we have an insertion point.  delete the thing in front of it or
       // behind it, depending on aAction
       nsresult rv = CreateTxnForDeleteInsertionPoint(range, aAction,
                                                      aggregateTransaction,
                                                      aNode, aOffset, aLength);
       NS_ENSURE_SUCCESS(rv, rv);
     }