Bug 1425412 - part 1: Create InsertTextTransaction::Create() and remove EditorBase::CreateTxnForInsertText() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 15 Dec 2017 17:26:52 +0900
changeset 449014 41a28234be872e4c729bb3f06b38964b33ed30cb
parent 449013 d0d8308550d80553b6f70143a9bafe56cc164cd4
child 449015 d692dc23699b887aac003eaceb25346e8919fd86
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1425412
milestone59.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 1425412 - part 1: Create InsertTextTransaction::Create() and remove EditorBase::CreateTxnForInsertText() r=m_kato EditorBase::CreateTxnForInsertText() just hides what it exactly does. Rewriting it with a static factory method, InsertTextTransaction::Create() should be clearer for its caller. MozReview-Commit-ID: Er7Zlhtbnb0
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/InsertTextTransaction.cpp
editor/libeditor/InsertTextTransaction.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2790,17 +2790,18 @@ EditorBase::InsertTextIntoTextNodeImpl(c
     mComposition->DidCreateCompositionTransaction(aStringToInsert);
     isIMETransaction = true;
     // All characters of the composition string will be replaced with
     // aStringToInsert.  So, we need to emulate to remove the composition
     // string.
     insertedTextNode = mComposition->GetContainerTextNode();
     insertedOffset = mComposition->XPOffsetInTextNode();
   } else {
-    transaction = CreateTxnForInsertText(aStringToInsert, aTextNode, aOffset);
+    transaction =
+      InsertTextTransaction::Create(*this, aStringToInsert, aTextNode, aOffset);
   }
 
   // Let listeners know what's up
   {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->WillInsertText(
         static_cast<nsIDOMCharacterData*>(insertedTextNode->AsDOMNode()),
@@ -2993,27 +2994,16 @@ EditorBase::SetTextImpl(Selection& aSele
           aString, rv);
       }
     }
   }
 
   return rv;
 }
 
-already_AddRefed<InsertTextTransaction>
-EditorBase::CreateTxnForInsertText(const nsAString& aStringToInsert,
-                                   Text& aTextNode,
-                                   int32_t aOffset)
-{
-  RefPtr<InsertTextTransaction> transaction =
-    new InsertTextTransaction(aTextNode, aOffset, aStringToInsert, *this,
-                              &mRangeUpdater);
-  return transaction.forget();
-}
-
 nsresult
 EditorBase::DeleteText(nsGenericDOMDataNode& aCharData,
                        uint32_t aOffset,
                        uint32_t aLength)
 {
   RefPtr<DeleteTextTransaction> transaction =
     CreateTxnForDeleteText(aCharData, aOffset, aLength);
   NS_ENSURE_STATE(transaction);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -492,16 +492,18 @@ public:
    * to native IME.  Therefore, when there is composition, this can do anything.
    * For example, the editor instance, the widget or the process itself may
    * be destroyed.
    */
   nsresult CommitComposition();
 
   void SwitchTextDirectionTo(uint32_t aDirection);
 
+  RangeUpdater& RangeUpdaterRef() { return mRangeUpdater; }
+
 protected:
   nsresult DetermineCurrentDirection();
   void FireInputEvent();
 
   /**
    * Create a transaction for setting aAttribute to aValue on aElement.  Never
    * returns null.
    */
@@ -609,24 +611,16 @@ protected:
   already_AddRefed<EditTransactionBase>
     CreateTxnForDeleteRange(nsRange* aRangeToDelete,
                             EDirection aAction,
                             nsINode** aRemovingNode,
                             int32_t* aOffset,
                             int32_t* aLength);
 
   /**
-   * Create a transaction for inserting aStringToInsert into aTextNode.  Never
-   * returns null.
-   */
-  already_AddRefed<mozilla::InsertTextTransaction>
-    CreateTxnForInsertText(const nsAString& aStringToInsert, Text& aTextNode,
-                           int32_t aOffset);
-
-  /**
    * Never returns null.
    */
   already_AddRefed<mozilla::CompositionTransaction>
     CreateTxnForComposition(const nsAString& aStringToInsert);
 
   /**
    * Create a transaction for adding a style sheet.
    */
--- a/editor/libeditor/InsertTextTransaction.cpp
+++ b/editor/libeditor/InsertTextTransaction.cpp
@@ -13,26 +13,36 @@
 #include "nsDebug.h"                    // for NS_ASSERTION, etc.
 #include "nsError.h"                    // for NS_OK, etc.
 #include "nsQueryObject.h"              // for do_QueryObject
 
 namespace mozilla {
 
 using namespace dom;
 
-InsertTextTransaction::InsertTextTransaction(Text& aTextNode,
-                                             uint32_t aOffset,
+// static
+already_AddRefed<InsertTextTransaction>
+InsertTextTransaction::Create(EditorBase& aEditorBase,
+                              const nsAString& aStringToInsert,
+                              Text& aTextNode,
+                              uint32_t aOffset)
+{
+  RefPtr<InsertTextTransaction> transaction =
+    new InsertTextTransaction(aEditorBase, aStringToInsert, aTextNode, aOffset);
+  return transaction.forget();
+}
+
+InsertTextTransaction::InsertTextTransaction(EditorBase& aEditorBase,
                                              const nsAString& aStringToInsert,
-                                             EditorBase& aEditorBase,
-                                             RangeUpdater* aRangeUpdater)
+                                             Text& aTextNode,
+                                             uint32_t aOffset)
   : mTextNode(&aTextNode)
   , mOffset(aOffset)
   , mStringToInsert(aStringToInsert)
   , mEditorBase(&aEditorBase)
-  , mRangeUpdater(aRangeUpdater)
 {
 }
 
 InsertTextTransaction::~InsertTextTransaction()
 {
 }
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(InsertTextTransaction, EditTransactionBase,
@@ -51,30 +61,35 @@ NS_INTERFACE_MAP_END_INHERITING(EditTran
 NS_IMETHODIMP
 InsertTextTransaction::DoTransaction()
 {
   if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mTextNode)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsresult rv = mTextNode->InsertData(mOffset, mStringToInsert);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   // Only set selection to insertion point if editor gives permission
   if (mEditorBase->GetShouldTxnSetSelection()) {
     RefPtr<Selection> selection = mEditorBase->GetSelection();
-    NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
+    if (NS_WARN_IF(!selection)) {
+      return NS_ERROR_FAILURE;
+    }
     DebugOnly<nsresult> rv =
       selection->Collapse(mTextNode, mOffset + mStringToInsert.Length());
     NS_ASSERTION(NS_SUCCEEDED(rv),
                  "Selection could not be collapsed after insert");
   } else {
     // Do nothing - DOM Range gravity will adjust selection
   }
-  mRangeUpdater->SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
+  mEditorBase->RangeUpdaterRef().
+                 SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 InsertTextTransaction::UndoTransaction()
 {
   return mTextNode->DeleteData(mOffset, mStringToInsert.Length());
--- a/editor/libeditor/InsertTextTransaction.h
+++ b/editor/libeditor/InsertTextTransaction.h
@@ -17,40 +17,49 @@ class nsITransaction;
 
 #define NS_INSERTTEXTTXN_IID \
 { 0x8c9ad77f, 0x22a7, 0x4d01, \
   { 0xb1, 0x59, 0x8a, 0x0f, 0xdb, 0x1d, 0x08, 0xe9 } }
 
 namespace mozilla {
 
 class EditorBase;
-class RangeUpdater;
 
 namespace dom {
 class Text;
 } // namespace dom
 
 /**
  * A transaction that inserts text into a content node.
  */
 class InsertTextTransaction final : public EditTransactionBase
 {
+protected:
+  InsertTextTransaction(EditorBase& aEditorBase,
+                        const nsAString& aStringToInsert,
+                        dom::Text& aTextNode,
+                        uint32_t aOffset);
+
 public:
-  NS_DECLARE_STATIC_IID_ACCESSOR(NS_INSERTTEXTTXN_IID)
-
   /**
-   * @param aElement        The text content node.
-   * @param aOffset         The location in aElement to do the insertion.
-   * @param aString         The new text to insert.
-   * @param aPresShell      Used to get and set the selection.
-   * @param aRangeUpdater   The range updater
+   * Creates new InsertTextTransaction instance.  This never returns nullptr.
+   *
+   * @param aEditorBase     The editor which manages the transaction.
+   * @param aTextNode       The text content node to be inserted
+   *                        aStringToInsert.
+   * @param aOffset         The offset in aTextNode to do the insertion.
+   * @param aStringToInsert The new string to insert.
    */
-  InsertTextTransaction(dom::Text& aTextNode, uint32_t aOffset,
-                        const nsAString& aString, EditorBase& aEditorBase,
-                        RangeUpdater* aRangeUpdater);
+  static already_AddRefed<InsertTextTransaction>
+  Create(EditorBase& aEditorBase,
+         const nsAString& aStringToInsert,
+         dom::Text& aTextNode,
+         uint32_t aOffset);
+
+  NS_DECLARE_STATIC_IID_ACCESSOR(NS_INSERTTEXTTXN_IID)
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(InsertTextTransaction,
                                            EditTransactionBase)
 
   NS_DECL_EDITTRANSACTIONBASE
 
   NS_IMETHOD Merge(nsITransaction* aTransaction, bool* aDidMerge) override;
@@ -72,17 +81,15 @@ private:
   // The offset into mTextNode where the insertion is to take place.
   uint32_t mOffset;
 
   // The text to insert into mTextNode at mOffset.
   nsString mStringToInsert;
 
   // The editor, which we'll need to get the selection.
   RefPtr<EditorBase> mEditorBase;
-
-  RangeUpdater* mRangeUpdater;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(InsertTextTransaction, NS_INSERTTEXTTXN_IID)
 
 } // namespace mozilla
 
 #endif // #ifndef InsertTextTransaction_h