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

--HG--
extra : rebase_source : fe3f75126a8975a0edb00a6c0709134b0a0f4d87

editor/libeditor/DeleteRangeTransaction.cpp

@@ -24,50 +24,32 @@ 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) {
@@ -95,9 +77,9 @@ DeleteRangeTransaction::DoTransaction()
 
   // 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);
@@ -110,7 +92,7 @@ DeleteRangeTransaction::DoTransaction()
 NS_IMETHODIMP
 DeleteRangeTransaction::UndoTransaction()
 {
-  MOZ_ASSERT(mRange && mEditorBase);
+  MOZ_ASSERT(mRangeToDelete);
 
   return EditAggregateTransaction::UndoTransaction();
 }
@@ -118,7 +100,7 @@ DeleteRangeTransaction::UndoTransaction()
 NS_IMETHODIMP
 DeleteRangeTransaction::RedoTransaction()
 {
-  MOZ_ASSERT(mRange && mEditorBase);
+  MOZ_ASSERT(mRangeToDelete);
 
   return EditAggregateTransaction::RedoTransaction();
 }
@@ -149,7 +131,7 @@ DeleteRangeTransaction::CreateTxnsToDeleteBetween(nsINode* aNode,
       static_cast<nsGenericDOMDataNode*>(aNode);
 
     RefPtr<DeleteTextTransaction> transaction =
-      new DeleteTextTransaction(*mEditorBase, *charDataNode, aStartOffset,
+      new DeleteTextTransaction(mEditorBase, *charDataNode, aStartOffset,
                                 numToDel, mRangeUpdater);
 
     nsresult rv = transaction->Init();
@@ -167,7 +149,7 @@ DeleteRangeTransaction::CreateTxnsToDeleteBetween(nsINode* aNode,
       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
@@ -202,7 +184,7 @@ DeleteRangeTransaction::CreateTxnsToDeleteContent(nsINode* aNode,
       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();
@@ -220,7 +202,7 @@ 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()) {
@@ -230,7 +212,7 @@ DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween()
     }
 
     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

editor/libeditor/DeleteRangeTransaction.h

@@ -28,15 +28,12 @@ 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)
@@ -48,7 +45,7 @@ public:
 
   virtual void LastRelease() override
   {
-    mRange = nullptr;
+    mRangeToDelete = nullptr;
     EditAggregateTransaction::LastRelease();
   }
 
@@ -63,11 +60,11 @@ protected:
                                      int32_t aOffset,
                                      nsIEditor::EDirection aAction);
 
-  // P1 in the range.
-  RefPtr<nsRange> mRange;
-
   // The editor for this transaction.
-  EditorBase* mEditorBase;
+  EditorBase& mEditorBase;
+
+  // P1 in the range.
+  RefPtr<nsRange> mRangeToDelete;
 
   // Range updater object.
   RangeUpdater* mRangeUpdater;

editor/libeditor/EditorBase.cpp

@@ -4368,9 +4368,10 @@ EditorBase::CreateTxnForDeleteSelection(EDirection aAction,
     // 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