Bug 1425412 - part 12: Create factory methods for DeleteRangeTransaction, EditAggregateTransaction and PlaceholderTransaction for consistency with the other transaction classes r=m_kato

Although, we don't need factory methods for DeleteRangeTransaction,
EditAggregateTransaction nor PlaceholderTransaction, for consistency with the
other transaction classes, they should have factory methods for making easier
to write the code.

For not making the performance slow down, they should be inline methods.

MozReview-Commit-ID: 7jl5yZNFYmP

--HG--
extra : rebase_source : 7cd5b5e268a670b3c8855407cc72dec12d34d8ff

editor/libeditor/DeleteRangeTransaction.cpp

@@ -24,13 +24,10 @@ namespace mozilla {
 
 using namespace dom;
 
-// note that aEditorBase is not refcounted
 DeleteRangeTransaction::DeleteRangeTransaction(EditorBase& aEditorBase,
-                                               nsRange& aRangeToDelete,
-                                               RangeUpdater* aRangeUpdater)
+                                               nsRange& aRangeToDelete)
   : mEditorBase(&aEditorBase)
   , mRangeToDelete(aRangeToDelete.CloneRange())
-  , mRangeUpdater(aRangeUpdater)
 {
 }
 

editor/libeditor/DeleteRangeTransaction.h

@@ -27,14 +27,25 @@ class RangeUpdater;
  */
 class DeleteRangeTransaction final : public EditAggregateTransaction
 {
+protected:
+  DeleteRangeTransaction(EditorBase& aEditorBase,
+                         nsRange& aRangeToDelete);
+
 public:
   /**
+   * Creates a delete range transaction.  This never returns nullptr.
+   *
    * @param aEditorBase         The object providing basic editing operations.
    * @param aRangeToDelete      The range to delete.
    */
-  DeleteRangeTransaction(EditorBase& aEditorBase,
-                         nsRange& aRangeToDelete,
-                         RangeUpdater* aRangeUpdater);
+  static already_AddRefed<DeleteRangeTransaction>
+  Create(EditorBase& aEditorBase,
+         nsRange& aRangeToDelete)
+  {
+    RefPtr<DeleteRangeTransaction> transaction =
+      new DeleteRangeTransaction(aEditorBase, aRangeToDelete);
+    return transaction.forget();
+  }
 
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(DeleteRangeTransaction,
                                            EditAggregateTransaction)
@@ -44,12 +55,6 @@ public:
 
   NS_IMETHOD RedoTransaction() override;
 
-  virtual void LastRelease() override
-  {
-    mRangeToDelete = nullptr;
-    EditAggregateTransaction::LastRelease();
-  }
-
 protected:
   /**
    * CreateTxnsToDeleteBetween() creates a DeleteTextTransaction or some
@@ -106,9 +111,6 @@ protected:
   // P1 in the range.  This is only non-null until DoTransaction is called and
   // we convert it into child transactions.
   RefPtr<nsRange> mRangeToDelete;
-
-  // Range updater object.
-  RangeUpdater* mRangeUpdater;
 };
 
 } // namespace mozilla

editor/libeditor/EditAggregateTransaction.h

@@ -24,9 +24,20 @@ namespace mozilla {
  */
 class EditAggregateTransaction : public EditTransactionBase
 {
-public:
+protected:
   EditAggregateTransaction();
 
+public:
+  /**
+   * Creates an edit aggregate transaction.  This never returns nullptr.
+   */
+  static already_AddRefed<EditAggregateTransaction> Create()
+  {
+    RefPtr<EditAggregateTransaction> transaction =
+      new EditAggregateTransaction();
+    return transaction.forget();
+  }
+
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(EditAggregateTransaction,
                                            EditTransactionBase)

editor/libeditor/EditTransactionBase.cpp

@@ -23,8 +23,7 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(EditTransactionBase)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(EditTransactionBase)
-NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(EditTransactionBase,
-                                                   LastRelease())
+NS_IMPL_CYCLE_COLLECTING_RELEASE(EditTransactionBase)
 
 EditTransactionBase::~EditTransactionBase()
 {

editor/libeditor/EditTransactionBase.h

@@ -24,8 +24,6 @@ public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(EditTransactionBase, nsITransaction)
 
-  virtual void LastRelease() {}
-
   NS_IMETHOD RedoTransaction(void) override;
   NS_IMETHOD GetIsTransient(bool* aIsTransient) override;
   NS_IMETHOD Merge(nsITransaction* aTransaction, bool* aDidMerge) override;

editor/libeditor/EditorBase.cpp

@@ -703,7 +703,7 @@ EditorBase::DoTransaction(Selection* aSelection, nsITransaction* aTxn)
 {
   if (mPlaceholderBatch && !mPlaceholderTransaction) {
     mPlaceholderTransaction =
-      new PlaceholderTransaction(*this, mPlaceholderName, Move(mSelState));
+      PlaceholderTransaction::Create(*this, mPlaceholderName, Move(mSelState));
     MOZ_ASSERT(mSelState.isNothing());
 
     // We will recurse, but will not hit this case in the nested call
@@ -4601,7 +4601,7 @@ EditorBase::CreateTxnForDeleteSelection(EDirection aAction,
 
   // allocate the out-param transaction
   RefPtr<EditAggregateTransaction> aggregateTransaction =
-    new EditAggregateTransaction();
+    EditAggregateTransaction::Create();
 
   for (uint32_t rangeIdx = 0; rangeIdx < selection->RangeCount(); ++rangeIdx) {
     RefPtr<nsRange> range = selection->GetRangeAt(rangeIdx);
@@ -4613,7 +4613,7 @@ EditorBase::CreateTxnForDeleteSelection(EDirection aAction,
     // is eNone, do nothing.
     if (!range->Collapsed()) {
       RefPtr<DeleteRangeTransaction> deleteRangeTransaction =
-        new DeleteRangeTransaction(*this, *range, &mRangeUpdater);
+        DeleteRangeTransaction::Create(*this, *range);
       // XXX Oh, not checking if deleteRangeTransaction can modify the range...
       aggregateTransaction->AppendChild(deleteRangeTransaction);
     } else if (aAction != eNone) {

editor/libeditor/PlaceholderTransaction.cpp

@@ -20,16 +20,13 @@ PlaceholderTransaction::PlaceholderTransaction(
                           EditorBase& aEditorBase,
                           nsAtom* aName,
                           Maybe<SelectionState>&& aSelState)
-  : mAbsorb(true)
+  : mEditorBase(&aEditorBase)
   , mForwarding(nullptr)
   , mCompositionTransaction(nullptr)
+  , mStartSel(*Move(aSelState))
+  , mAbsorb(true)
   , mCommitted(false)
-  , mEditorBase(&aEditorBase)
 {
-  // Make sure to move aSelState into a local variable to null out the original
-  // Maybe<SelectionState> variable.
-  Maybe<SelectionState> selState(Move(aSelState));
-  mStartSel = *selState;
   mName = aName;
 }
 

editor/libeditor/PlaceholderTransaction.h

@@ -29,12 +29,34 @@ class PlaceholderTransaction final
  : public EditAggregateTransaction
  , public nsIAbsorbingTransaction
 {
-public:
-  NS_DECL_ISUPPORTS_INHERITED
-
-  PlaceholderTransaction(EditorBase& aEditorBase, nsAtom* aName,
+protected:
+  PlaceholderTransaction(EditorBase& aEditorBase,
+                         nsAtom* aName,
                          Maybe<SelectionState>&& aSelState);
 
+public:
+  /**
+   * Creates a placeholder transaction.  This never returns nullptr.
+   *
+   * @param aEditorBase     The editor.
+   * @param aName           The name of creating transaction.
+   * @param aSelState       The selection state of aEditorBase.
+   */
+  static already_AddRefed<PlaceholderTransaction>
+  Create(EditorBase& aEditorBase,
+         nsAtom* aName,
+         Maybe<SelectionState>&& aSelState)
+  {
+    // Make sure to move aSelState into a local variable to null out the original
+    // Maybe<SelectionState> variable.
+    Maybe<SelectionState> selState(Move(aSelState));
+    RefPtr<PlaceholderTransaction> transaction =
+      new PlaceholderTransaction(aEditorBase, aName, Move(selState));
+    return transaction.forget();
+  }
+
+  NS_DECL_ISUPPORTS_INHERITED
+
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(PlaceholderTransaction,
                                            EditAggregateTransaction)
 // ------------ EditAggregateTransaction -----------------------
@@ -68,13 +90,12 @@ public:
 protected:
   virtual ~PlaceholderTransaction();
 
-  // Do we auto absorb any and all transaction?
-  bool mAbsorb;
+  // The editor for this transaction.
+  RefPtr<EditorBase> mEditorBase;
+
   nsWeakPtr mForwarding;
   // First IME txn in this placeholder - used for IME merging.
   mozilla::CompositionTransaction* mCompositionTransaction;
-  // Do we stop auto absorbing any matching placeholder transactions?
-  bool mCommitted;
 
   // These next two members store the state of the selection in a safe way.
   // Selection at the start of the transaction is stored, as is the selection
@@ -84,8 +105,10 @@ protected:
   SelectionState mStartSel;
   SelectionState mEndSel;
 
-  // The editor for this transaction.
-  RefPtr<EditorBase> mEditorBase;
+  // Do we auto absorb any and all transaction?
+  bool mAbsorb;
+  // Do we stop auto absorbing any matching placeholder transactions?
+  bool mCommitted;
 };
 
 } // namespace mozilla