Bug 1482007 - part 1: Create a helper class to guarantee to call nsIEditor::EndTransaction() after nsIEditor::BeginTransaction() call r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 09 Aug 2018 11:45:41 +0000
changeset 485878 3392988c08941ca7e7ead7455d3574445d0acb50
parent 485877 421c12a2837ba777d295a891ff4de9637792011e
child 485879 305478da57df5ec75ea4ac7a0bbe4010f3874200
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1482007
milestone63.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 1482007 - part 1: Create a helper class to guarantee to call nsIEditor::EndTransaction() after nsIEditor::BeginTransaction() call r=m_kato This patch also creates non-virtual methods, EditorBase::BeginTransactionInternal(), EditorBase::EndTransactionInternal(), TransactionManager::BeginBatchInternal() and TransactionManager::EndBatchInternal(). Although, this could be replaced with API to use PlaceholderTransaction, we should investigate it when we have much time. Differential Revision: https://phabricator.services.mozilla.com/D2989
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/EditorUtils.h
editor/txmgr/TransactionManager.cpp
editor/txmgr/TransactionManager.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -872,37 +872,47 @@ EditorBase::CanRedo(bool* aIsEnabled, bo
   *aCanRedo = CanRedo();
   *aIsEnabled = IsUndoRedoEnabled();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::BeginTransaction()
 {
+  BeginTransactionInternal();
+  return NS_OK;
+}
+
+void
+EditorBase::BeginTransactionInternal()
+{
   BeginUpdateViewBatch();
 
   if (mTransactionManager) {
     RefPtr<TransactionManager> transactionManager(mTransactionManager);
     transactionManager->BeginBatch(nullptr);
   }
-
-  return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::EndTransaction()
 {
+  EndTransactionInternal();
+  return NS_OK;
+}
+
+void
+EditorBase::EndTransactionInternal()
+{
   if (mTransactionManager) {
     RefPtr<TransactionManager> transactionManager(mTransactionManager);
     transactionManager->EndBatch(false);
   }
 
   EndUpdateViewBatch();
-
-  return NS_OK;
 }
 
 void
 EditorBase::BeginPlaceholderTransaction(nsAtom* aTransactionName)
 {
   MOZ_ASSERT(mPlaceholderBatch >= 0, "negative placeholder batch count!");
   if (!mPlaceholderBatch) {
     NotifyEditorObservers(eNotifyEditorObserversOfBefore);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -48,16 +48,17 @@ class nsISupports;
 class nsITransaction;
 class nsITransactionListener;
 class nsIWidget;
 class nsRange;
 
 namespace mozilla {
 class AutoSelectionRestorer;
 class AutoTopLevelEditSubActionNotifier;
+class AutoTransactionBatch;
 class AutoTransactionsConserveSelection;
 class AutoUpdateViewBatch;
 class ChangeAttributeTransaction;
 class CompositionTransaction;
 class CreateElementTransaction;
 class CSSEditUtils;
 class DeleteNodeTransaction;
 class DeleteRangeTransaction;
@@ -1662,16 +1663,26 @@ protected: // Called by helper classes.
    * manager batches.
    */
   void BeginPlaceholderTransaction(nsAtom* aTransactionName);
   void EndPlaceholderTransaction();
 
   void BeginUpdateViewBatch();
   void EndUpdateViewBatch();
 
+  /**
+   * Used by AutoTransactionBatch.  After calling BeginTransactionInternal(),
+   * all transactions will be treated as an atomic transaction.  I.e.,
+   * two or more transactions are undid once.
+   * XXX What's the difference with PlaceholderTransaction? Should we always
+   *     use it instead?
+   */
+  void BeginTransactionInternal();
+  void EndTransactionInternal();
+
 protected: // Shouldn't be used by friend classes
   /**
    * The default destructor. This should suffice. Should this be pure virtual
    * for someone to derive from the EditorBase later? I don't believe so.
    */
   virtual ~EditorBase();
 
   /**
@@ -1985,16 +1996,17 @@ protected:
   // Whether spellchecker dictionary is initialized after focused.
   bool mSpellCheckerDictionaryUpdated;
   // Whether we are an HTML editor class.
   bool mIsHTMLEditorClass;
 
   friend class AutoPlaceholderBatch;
   friend class AutoSelectionRestorer;
   friend class AutoTopLevelEditSubActionNotifier;
+  friend class AutoTransactionBatch;
   friend class AutoTransactionsConserveSelection;
   friend class AutoUpdateViewBatch;
   friend class CompositionTransaction;
   friend class CreateElementTransaction;
   friend class CSSEditUtils;
   friend class DeleteNodeTransaction;
   friend class DeleteRangeTransaction;
   friend class DeleteTextTransaction;
--- a/editor/libeditor/EditorUtils.h
+++ b/editor/libeditor/EditorUtils.h
@@ -447,16 +447,41 @@ private:
   nsCOMPtr<nsIContent> mRightContent;
 
   nsresult mRv;
 
   SplitRangeOffFromNodeResult() = delete;
 };
 
 /***************************************************************************
+ * stack based helper class for calling EditorBase::EndTransaction() after
+ * EditorBase::BeginTransaction().
+ ***************************************************************************/
+class MOZ_RAII AutoTransactionBatch final
+{
+private:
+  OwningNonNull<EditorBase> mEditorBase;
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+
+public:
+  explicit AutoTransactionBatch(EditorBase& aEditorBase
+                                MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
+    : mEditorBase(aEditorBase)
+  {
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    mEditorBase->BeginTransactionInternal();
+  }
+
+  ~AutoTransactionBatch()
+  {
+    mEditorBase->EndTransactionInternal();
+  }
+};
+
+/***************************************************************************
  * stack based helper class for batching a collection of transactions inside a
  * placeholder transaction.
  */
 class MOZ_RAII AutoPlaceholderBatch final
 {
 private:
   RefPtr<EditorBase> mEditorBase;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
--- a/editor/txmgr/TransactionManager.cpp
+++ b/editor/txmgr/TransactionManager.cpp
@@ -192,16 +192,26 @@ NS_IMETHODIMP
 TransactionManager::Clear()
 {
   return ClearUndoRedo() ? NS_OK : NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 TransactionManager::BeginBatch(nsISupports* aData)
 {
+  nsresult rv = BeginBatchInternal(aData);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return NS_OK;
+}
+
+nsresult
+TransactionManager::BeginBatchInternal(nsISupports* aData)
+{
   // We can batch independent transactions together by simply pushing
   // a dummy transaction item on the do stack. This dummy transaction item
   // will be popped off the do stack, and then pushed on the undo stack
   // in EndBatch().
   bool doInterrupt = false;
   nsresult rv = WillBeginBatchNotify(&doInterrupt);
   if (NS_FAILED(rv)) {
     return rv;
@@ -220,16 +230,26 @@ TransactionManager::BeginBatch(nsISuppor
   // XXX The result of BeginTransaction() or DidBeginBatchNotify() if
   //     BeginTransaction() succeeded.
   return rv;
 }
 
 NS_IMETHODIMP
 TransactionManager::EndBatch(bool aAllowEmpty)
 {
+  nsresult rv = EndBatchInternal(aAllowEmpty);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  return NS_OK;
+}
+
+nsresult
+TransactionManager::EndBatchInternal(bool aAllowEmpty)
+{
   // XXX: Need to add some mechanism to detect the case where the transaction
   //      at the top of the do stack isn't the dummy transaction, so we can
   //      throw an error!! This can happen if someone calls EndBatch() within
   //      the DoTransaction() method of a transaction.
   //
   //      For now, we can detect this case by checking the value of the
   //      dummy transaction's mTransaction field. If it is our dummy
   //      transaction, it should be nullptr. This may not be true in the
--- a/editor/txmgr/TransactionManager.h
+++ b/editor/txmgr/TransactionManager.h
@@ -88,16 +88,22 @@ public:
   nsresult WillMergeNotify(nsITransaction* aTop,
                            nsITransaction* aTransaction,
                            bool* aInterrupt);
   nsresult DidMergeNotify(nsITransaction* aTop,
                           nsITransaction* aTransaction,
                           bool aDidMerge,
                           nsresult aMergeResult);
 
+  /**
+   * Exposing non-virtual methods of nsITransactionManager methods.
+   */
+  nsresult BeginBatchInternal(nsISupports* aData);
+  nsresult EndBatchInternal(bool aAllowEmpty);
+
 private:
   virtual ~TransactionManager() = default;
 
   nsresult BeginTransaction(nsITransaction* aTransaction,
                             nsISupports* aData);
   nsresult EndTransaction(bool aAllowEmpty);
 
   int32_t                mMaxTransactionCount;