Bug 1425412 - part 6: Create DeleteNodeTransaction::MaybeCreate() and remove EditorBaseTransaction::CreateTxnForDeleteNode() r=m_kato

EditorBaseTransaction::CreateTxnForDeleteNode() just hides what it does.
Instead, let's create a factory method, DeleteNodeTransaction::MaybeCreate()
for making callers clearer.

MozReview-Commit-ID: 8WUYN0BjKSU

--HG--
extra : rebase_source : e0ff8b8434b720dc124c770cd7371d84b949ca8d

editor/libeditor/DeleteNodeTransaction.cpp

@@ -12,18 +12,25 @@
 
 namespace mozilla {
 
+// static
+already_AddRefed<DeleteNodeTransaction>
+DeleteNodeTransaction::MaybeCreate(EditorBase& aEditorBase,
+                              nsINode& aNodeToDelete)
+{
+  RefPtr<DeleteNodeTransaction> transaction =
+    new DeleteNodeTransaction(aEditorBase, aNodeToDelete);
+  if (NS_WARN_IF(!transaction->CanDoIt())) {
+    return nullptr;
+  }
+  return transaction.forget();
+}
+
 DeleteNodeTransaction::DeleteNodeTransaction(EditorBase& aEditorBase,
-                                             nsINode& aNodeToDelete,
-                                             RangeUpdater* aRangeUpdater)
+                                             nsINode& aNodeToDelete)
   : mEditorBase(&aEditorBase)
   , mNodeToDelete(&aNodeToDelete)
   , mParentNode(aNodeToDelete.GetParentNode())
-  , mRangeUpdater(aRangeUpdater)
 {
-  // XXX We're not sure if this is really necessary.
-  if (!CanDoIt()) {
-    mRangeUpdater = nullptr;
-  }
 }
 
 DeleteNodeTransaction::~DeleteNodeTransaction()
@@ -65,9 +72,7 @@ DeleteNodeTransaction::DoTransaction()
   // give range updater a chance.  SelAdjDeleteNode() needs to be called
   // *before* we do the action, unlike some of the other RangeItem update
   // methods.
-  if (mRangeUpdater) {
-    mRangeUpdater->SelAdjDeleteNode(mNodeToDelete);
-  }
+  mEditorBase->RangeUpdaterRef().SelAdjDeleteNode(mNodeToDelete);
 
   ErrorResult error;
   mParentNode->RemoveChild(*mNodeToDelete, error);
@@ -95,9 +100,7 @@ DeleteNodeTransaction::RedoTransaction()
     return NS_OK;
   }
 
-  if (mRangeUpdater) {
-    mRangeUpdater->SelAdjDeleteNode(mNodeToDelete);
-  }
+  mEditorBase->RangeUpdaterRef().SelAdjDeleteNode(mNodeToDelete);
 
   ErrorResult error;
   mParentNode->RemoveChild(*mNodeToDelete, error);

editor/libeditor/DeleteNodeTransaction.h

@@ -17,16 +17,25 @@
 namespace mozilla {
 
 class EditorBase;
-class RangeUpdater;
 
 /**
  * A transaction that deletes a single element
  */
 class DeleteNodeTransaction final : public EditTransactionBase
 {
+protected:
+  DeleteNodeTransaction(EditorBase& aEditorBase, nsINode& aNodeToDelete);
+
 public:
-  DeleteNodeTransaction(EditorBase& aEditorBase, nsINode& aNodeToDelete,
-                        RangeUpdater* aRangeUpdater);
+  /**
+   * Creates a delete node transaction instance.  This returns nullptr if
+   * it cannot remove the node from its parent.
+   *
+   * @param aEditorBase         The editor.
+   * @param aNodeToDelete       The node to be removed from the DOM tree.
+   */
+  static already_AddRefed<DeleteNodeTransaction>
+  MaybeCreate(EditorBase& aEditorBase, nsINode& aNodeToDelete);
 
   /**
    * CanDoIt() returns true if there are enough members and can modify the
@@ -56,9 +65,6 @@ protected:
 
   // Next sibling to remember for undo/redo purposes.
   nsCOMPtr<nsIContent> mRefNode;
-
-  // Range updater object.
-  RangeUpdater* mRangeUpdater;
 };
 
 } // namespace mozilla

editor/libeditor/DeleteRangeTransaction.cpp

@@ -177,12 +177,12 @@ DeleteRangeTransaction::CreateTxnsToDeleteBetween(
        child && child != aEnd.GetChildAtOffset();
        child = child->GetNextSibling()) {
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-      new DeleteNodeTransaction(*mEditorBase, *child, mRangeUpdater);
+      DeleteNodeTransaction::MaybeCreate(*mEditorBase, *child);
     // 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()) {
+    if (deleteNodeTransaction) {
       AppendChild(deleteNodeTransaction);
     }
   }
@@ -255,12 +255,12 @@ DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween(nsRange* aRangeToDelete)
     }
 
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-      new DeleteNodeTransaction(*mEditorBase, *node, mRangeUpdater);
+      DeleteNodeTransaction::MaybeCreate(*mEditorBase, *node);
     // 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())) {
+    if (NS_WARN_IF(!deleteNodeTransaction)) {
       return NS_ERROR_FAILURE;
     }
     AppendChild(deleteNodeTransaction);

editor/libeditor/EditorBase.cpp

@@ -1662,6 +1662,10 @@ EditorBase::DeleteNode(nsIDOMNode* aNode)
 nsresult
 EditorBase::DeleteNode(nsINode* aNode)
 {
+  if (NS_WARN_IF(!aNode)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   AutoRules beginRulesSniffing(this, EditAction::createNode,
                                nsIEditor::ePrevious);
 
@@ -1674,7 +1678,7 @@ EditorBase::DeleteNode(nsINode* aNode)
   }
 
   RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-    CreateTxnForDeleteNode(aNode);
+    DeleteNodeTransaction::MaybeCreate(*this, *aNode);
   nsresult rv = deleteNodeTransaction ? DoTransaction(deleteNodeTransaction) :
                                         NS_ERROR_FAILURE;
 
@@ -4619,23 +4623,6 @@ EditorBase::CreateTxnForRemoveAttribute(Element& aElement,
   return transaction.forget();
 }
 
-already_AddRefed<DeleteNodeTransaction>
-EditorBase::CreateTxnForDeleteNode(nsINode* aNode)
-{
-  if (NS_WARN_IF(!aNode)) {
-    return nullptr;
-  }
-
-  RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-    new DeleteNodeTransaction(*this, *aNode, &mRangeUpdater);
-  // This should be OK because if currently it cannot delete the node,
-  // it should never be able to undo/redo.
-  if (!deleteNodeTransaction->CanDoIt()) {
-    return nullptr;
-  }
-  return deleteNodeTransaction.forget();
-}
-
 already_AddRefed<AddStyleSheetTransaction>
 EditorBase::CreateTxnForAddStyleSheet(StyleSheet* aSheet)
 {
@@ -4773,7 +4760,7 @@ EditorBase::CreateTxnForDeleteRange(nsRange* aRangeToDelete,
 
     // priorNode is not chardata, so tell its parent to delete it
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-      CreateTxnForDeleteNode(priorNode);
+      DeleteNodeTransaction::MaybeCreate(*this, *priorNode);
     if (NS_WARN_IF(!deleteNodeTransaction)) {
       return nullptr;
     }
@@ -4813,7 +4800,7 @@ EditorBase::CreateTxnForDeleteRange(nsRange* aRangeToDelete,
 
     // nextNode is not chardata, so tell its parent to delete it
     RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-      CreateTxnForDeleteNode(nextNode);
+      DeleteNodeTransaction::MaybeCreate(*this, *nextNode);
     if (NS_WARN_IF(!deleteNodeTransaction)) {
       return nullptr;
     }
@@ -4895,7 +4882,7 @@ EditorBase::CreateTxnForDeleteRange(nsRange* aRangeToDelete,
   }
 
   RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
-    CreateTxnForDeleteNode(selectedNode);
+    DeleteNodeTransaction::MaybeCreate(*this, *selectedNode);
   if (NS_WARN_IF(!deleteNodeTransaction)) {
     return nullptr;
   }

editor/libeditor/EditorBase.h

@@ -536,12 +536,6 @@ protected:
   already_AddRefed<Element> CreateNode(nsAtom* aTag,
                                        const EditorRawDOMPoint& aPointToInsert);
 
-  /**
-   * Create a transaction for removing aNode from its parent.
-   */
-  already_AddRefed<DeleteNodeTransaction>
-    CreateTxnForDeleteNode(nsINode* aNode);
-
   /**
    * Create an aggregate transaction for delete selection.  The result may
    * include DeleteNodeTransactions and/or DeleteTextTransactions as its