Bug 1447924 - part 4: Optimize NumbeOfUndoItems(), NumbeOfRedoItems(), CanUndo() and CanRedo() of EditorBase r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 23 Mar 2018 00:08:38 +0900
changeset 466088 08b900a071155400f2d23283cda40ee95d16bf69
parent 466087 362c6382d2657e84a7a632bf1dbc527833369617
child 466089 869a1445816be7f43f54f7c97f28e4c6273fa75f
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1447924
milestone61.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 1447924 - part 4: Optimize NumbeOfUndoItems(), NumbeOfRedoItems(), CanUndo() and CanRedo() of EditorBase r=m_kato Now, both TransactionManager.h and TransactionStack.h are exposed. So, TransactionManager::GetNumberOfUndoItems() and TransactionManager::GetNumberOfRedoItems() can be rewritten with non-virtual inline methods because they just return mUndoStack.GetSize() and mRedoStack.GetSize(). Then, we can implement EditorBase::NumbeOfUndoItems(), EditorBase::NumberOfRedoItems(), EditorBase::CanUndo() and EditorBase::CanRedo() as inline methods. MozReview-Commit-ID: 3CJd0VrlvFY
dom/html/nsTextEditorState.cpp
editor/composer/ComposerCommandsUpdater.cpp
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/EditorCommands.cpp
editor/txmgr/TransactionManager.cpp
editor/txmgr/TransactionManager.h
editor/txmgr/nsITransactionManager.idl
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -167,20 +167,17 @@ public:
   explicit AutoDisableUndo(TextEditor* aTextEditor
                            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
     : mTextEditor(aTextEditor)
     , mPreviousEnabled(true)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
     MOZ_ASSERT(mTextEditor);
 
-    bool canUndo;
-    DebugOnly<nsresult> rv = mTextEditor->CanUndo(&mPreviousEnabled, &canUndo);
-    MOZ_ASSERT(NS_SUCCEEDED(rv));
-
+    mPreviousEnabled = mTextEditor->IsUndoRedoEnabled();
     mTextEditor->EnableUndo(false);
   }
 
   ~AutoDisableUndo()
   {
     mTextEditor->EnableUndo(mPreviousEnabled);
   }
 
@@ -982,18 +979,18 @@ TextInputListener::OnEditActionHandled()
   nsTextControlFrame* frame = static_cast<nsTextControlFrame*> (frameBase);
   NS_ASSERTION(frame, "Where is our frame?");
   //
   // Update the undo / redo menus
   //
   RefPtr<TextEditor> textEditor = frame->GetTextEditor();
 
   // Get the number of undo / redo items
-  int32_t numUndoItems = textEditor->NumberOfUndoItems();
-  int32_t numRedoItems = textEditor->NumberOfRedoItems();
+  size_t numUndoItems = textEditor->NumberOfUndoItems();
+  size_t numRedoItems = textEditor->NumberOfRedoItems();
   if ((numUndoItems && !mHadUndoItems) || (!numUndoItems && mHadUndoItems) ||
       (numRedoItems && !mHadRedoItems) || (!numRedoItems && mHadRedoItems)) {
     // Modify the menu if undo or redo items are different
     UpdateTextInputCommands(NS_LITERAL_STRING("undo"));
 
     mHadUndoItems = numUndoItems != 0;
     mHadRedoItems = numRedoItems != 0;
   }
--- a/editor/composer/ComposerCommandsUpdater.cpp
+++ b/editor/composer/ComposerCommandsUpdater.cpp
@@ -2,16 +2,17 @@
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/ComposerCommandsUpdater.h"
 
 #include "mozilla/mozalloc.h"           // for operator new
+#include "mozilla/TransactionManager.h" // for TransactionManager
 #include "mozilla/dom/Selection.h"
 #include "nsAString.h"
 #include "nsComponentManagerUtils.h"    // for do_CreateInstance
 #include "nsDebug.h"                    // for NS_ENSURE_TRUE, etc
 #include "nsError.h"                    // for NS_OK, NS_ERROR_FAILURE, etc
 #include "nsICommandManager.h"          // for nsICommandManager
 #include "nsID.h"                       // for NS_GET_IID, etc
 #include "nsIDOMWindow.h"               // for nsIDOMWindow
@@ -111,18 +112,17 @@ ComposerCommandsUpdater::WillDo(nsITrans
 }
 
 NS_IMETHODIMP
 ComposerCommandsUpdater::DidDo(nsITransactionManager* aManager,
                                nsITransaction* aTransaction,
                                nsresult aDoResult)
 {
   // only need to update if the status of the Undo menu item changes.
-  int32_t undoCount;
-  aManager->GetNumberOfUndoItems(&undoCount);
+  size_t undoCount = aManager->AsTransactionManager()->NumberOfUndoItems();
   if (undoCount == 1) {
     if (mFirstDoOfFirstUndo) {
       UpdateCommandGroup(NS_LITERAL_STRING("undo"));
     }
     mFirstDoOfFirstUndo = false;
   }
 
   return NS_OK;
@@ -137,21 +137,20 @@ ComposerCommandsUpdater::WillUndo(nsITra
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ComposerCommandsUpdater::DidUndo(nsITransactionManager* aManager,
                                  nsITransaction* aTransaction,
                                  nsresult aUndoResult)
 {
-  int32_t undoCount;
-  aManager->GetNumberOfUndoItems(&undoCount);
-  if (undoCount == 0)
+  size_t undoCount = aManager->AsTransactionManager()->NumberOfUndoItems();
+  if (!undoCount) {
     mFirstDoOfFirstUndo = true;    // reset the state for the next do
-
+  }
   UpdateCommandGroup(NS_LITERAL_STRING("undo"));
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ComposerCommandsUpdater::WillRedo(nsITransactionManager* aManager,
                                   nsITransaction* aTransaction,
                                   bool* aInterrupt)
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -823,53 +823,27 @@ EditorBase::EnableUndo(bool aEnable)
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::GetNumberOfUndoItems(int32_t* aNumItems)
 {
-  *aNumItems = NumberOfUndoItems();
-  return *aNumItems >= 0 ? NS_OK : NS_ERROR_FAILURE;
-}
-
-int32_t
-EditorBase::NumberOfUndoItems() const
-{
-  if (!mTransactionManager) {
-    return 0;
-  }
-  int32_t numItems = 0;
-  nsresult rv = mTransactionManager->GetNumberOfUndoItems(&numItems);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return -1;
-  }
-  return numItems;
+  *aNumItems = static_cast<int32_t>(NumberOfUndoItems());
+  MOZ_ASSERT(*aNumItems >= 0);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::GetNumberOfRedoItems(int32_t* aNumItems)
 {
-  *aNumItems = NumberOfRedoItems();
-  return *aNumItems >= 0 ? NS_OK : NS_ERROR_FAILURE;
-}
-
-int32_t
-EditorBase::NumberOfRedoItems() const
-{
-  if (!mTransactionManager) {
-    return 0;
-  }
-  int32_t numItems = 0;
-  nsresult rv = mTransactionManager->GetNumberOfRedoItems(&numItems);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return -1;
-  }
-  return numItems;
+  *aNumItems = static_cast<int32_t>(NumberOfRedoItems());
+  MOZ_ASSERT(*aNumItems >= 0);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::GetTransactionManager(nsITransactionManager** aTxnManager)
 {
   // NOTE: If you need to override this method, you need to make
   //       GetTransactionManager() virtual.
   if (NS_WARN_IF(!aTxnManager)) {
@@ -889,19 +863,19 @@ EditorBase::GetTransactionManager() cons
   return transactionManager.forget();
 }
 
 NS_IMETHODIMP
 EditorBase::Undo(uint32_t aCount)
 {
   ForceCompositionEnd();
 
-  bool hasTransactionManager, hasTransaction = false;
-  CanUndo(&hasTransactionManager, &hasTransaction);
-  NS_ENSURE_TRUE(hasTransaction, NS_OK);
+  if (!CanUndo()) {
+    return NS_OK;
+  }
 
   AutoRules beginRulesSniffing(this, EditAction::undo, nsIEditor::eNone);
 
   if (!mTransactionManager) {
     return NS_OK;
   }
 
   RefPtr<TransactionManager> transactionManager(mTransactionManager);
@@ -914,34 +888,30 @@ EditorBase::Undo(uint32_t aCount)
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::CanUndo(bool* aIsEnabled,
                     bool* aCanUndo)
 {
-  NS_ENSURE_TRUE(aIsEnabled && aCanUndo, NS_ERROR_NULL_POINTER);
-  *aIsEnabled = !!mTransactionManager;
-  if (*aIsEnabled) {
-    int32_t numTxns = 0;
-    mTransactionManager->GetNumberOfUndoItems(&numTxns);
-    *aCanUndo = !!numTxns;
-  } else {
-    *aCanUndo = false;
-  }
+  if (NS_WARN_IF(!aIsEnabled) || NS_WARN_IF(!aCanUndo)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  *aCanUndo = CanUndo();
+  *aIsEnabled = IsUndoRedoEnabled();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::Redo(uint32_t aCount)
 {
-  bool hasTransactionManager, hasTransaction = false;
-  CanRedo(&hasTransactionManager, &hasTransaction);
-  NS_ENSURE_TRUE(hasTransaction, NS_OK);
+  if (!CanRedo()) {
+    return NS_OK;
+  }
 
   AutoRules beginRulesSniffing(this, EditAction::redo, nsIEditor::eNone);
 
   if (!mTransactionManager) {
     return NS_OK;
   }
 
   RefPtr<TransactionManager> transactionManager(mTransactionManager);
@@ -953,26 +923,21 @@ EditorBase::Redo(uint32_t aCount)
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::CanRedo(bool* aIsEnabled, bool* aCanRedo)
 {
-  NS_ENSURE_TRUE(aIsEnabled && aCanRedo, NS_ERROR_NULL_POINTER);
-
-  *aIsEnabled = !!mTransactionManager;
-  if (*aIsEnabled) {
-    int32_t numTxns = 0;
-    mTransactionManager->GetNumberOfRedoItems(&numTxns);
-    *aCanRedo = !!numTxns;
-  } else {
-    *aCanRedo = false;
-  }
+  if (NS_WARN_IF(!aIsEnabled) || NS_WARN_IF(!aCanRedo)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  *aCanRedo = CanRedo();
+  *aIsEnabled = IsUndoRedoEnabled();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::BeginTransaction()
 {
   BeginUpdateViewBatch();
 
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -10,16 +10,17 @@
 #include "mozilla/EditorDOMPoint.h"     // for EditorDOMPoint
 #include "mozilla/Maybe.h"              // for Maybe
 #include "mozilla/OwningNonNull.h"      // for OwningNonNull
 #include "mozilla/PresShell.h"          // for PresShell
 #include "mozilla/RangeBoundary.h"      // for RawRangeBoundary, RangeBoundary
 #include "mozilla/SelectionState.h"     // for RangeUpdater, etc.
 #include "mozilla/StyleSheet.h"         // for StyleSheet
 #include "mozilla/TextEditRules.h"      // for TextEditRules
+#include "mozilla/TransactionManager.h" // for TransactionManager
 #include "mozilla/WeakPtr.h"            // for WeakPtr
 #include "mozilla/dom/Selection.h"
 #include "mozilla/dom/Text.h"
 #include "nsCOMPtr.h"                   // for already_AddRefed, nsCOMPtr
 #include "nsCycleCollectionParticipant.h"
 #include "nsGkAtoms.h"
 #include "nsIDocument.h"                // for nsIDocument
 #include "nsIEditor.h"                  // for nsIEditor, etc.
@@ -74,17 +75,16 @@ class JoinNodeTransaction;
 class PlaceholderTransaction;
 class RemoveStyleSheetTransaction;
 class SplitNodeResult;
 class SplitNodeTransaction;
 class TextComposition;
 class TextEditor;
 class TextInputListener;
 class TextServicesDocument;
-class TransactionManager;
 enum class EditAction : int32_t;
 
 namespace dom {
 class DataTransfer;
 class DragEvent;
 class Element;
 class EventTarget;
 class Text;
@@ -1095,21 +1095,46 @@ public:
   bool IsIMEComposing() const;
 
   /**
    * Returns true when inserting text should be a part of current composition.
    */
   bool ShouldHandleIMEComposition() const;
 
   /**
-   * Returns number of undo or redo items.  If TransactionManager returns
-   * unexpected error, returns -1.
+   * Returns number of undo or redo items.
+   */
+  size_t NumberOfUndoItems() const
+  {
+    return mTransactionManager ? mTransactionManager->NumberOfUndoItems() : 0;
+  }
+  size_t NumberOfRedoItems() const
+  {
+    return mTransactionManager ? mTransactionManager->NumberOfRedoItems() : 0;
+  }
+
+  /**
+   * Returns true if this editor can store transactions for undo/redo.
    */
-  int32_t NumberOfUndoItems() const;
-  int32_t NumberOfRedoItems() const;
+  bool IsUndoRedoEnabled() const
+  {
+    return !!mTransactionManager;
+  }
+
+  /**
+   * Return true if it's possible to undo/redo right now.
+   */
+  bool CanUndo() const
+  {
+    return IsUndoRedoEnabled() && NumberOfUndoItems() > 0;
+  }
+  bool CanRedo() const
+  {
+    return IsUndoRedoEnabled() && NumberOfRedoItems() > 0;
+  }
 
   /**
    * From html rules code - migration in progress.
    */
   static nsAtom* GetTag(nsIDOMNode* aNode);
 
   bool NodesSameType(nsIDOMNode* aNode1, nsIDOMNode* aNode2);
   virtual bool AreNodesSameType(nsIContent* aNode1, nsIContent* aNode2);
--- a/editor/libeditor/EditorCommands.cpp
+++ b/editor/libeditor/EditorCommands.cpp
@@ -63,18 +63,18 @@ UndoCommand::IsCommandEnabled(const char
   if (!editor) {
     return NS_OK;
   }
   TextEditor* textEditor = editor->AsTextEditor();
   MOZ_ASSERT(textEditor);
   if (!textEditor->IsSelectionEditable()) {
     return NS_OK;
   }
-  bool isEnabled = false;
-  return editor->CanUndo(&isEnabled, aIsEnabled);
+  *aIsEnabled = textEditor->CanUndo();
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 UndoCommand::DoCommand(const char* aCommandName,
                        nsISupports* aCommandRefCon)
 {
   nsCOMPtr<nsIEditor> editor = do_QueryInterface(aCommandRefCon);
   if (!editor) {
@@ -122,18 +122,18 @@ RedoCommand::IsCommandEnabled(const char
   if (!editor) {
     return NS_OK;
   }
   TextEditor* textEditor = editor->AsTextEditor();
   MOZ_ASSERT(textEditor);
   if (!textEditor->IsSelectionEditable()) {
     return NS_OK;
   }
-  bool isEnabled = false;
-  return editor->CanRedo(&isEnabled, aIsEnabled);
+  *aIsEnabled = textEditor->CanRedo();
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 RedoCommand::DoCommand(const char* aCommandName,
                        nsISupports* aCommandRefCon)
 {
   nsCOMPtr<nsIEditor> editor = do_QueryInterface(aCommandRefCon);
   if (!editor) {
--- a/editor/txmgr/TransactionManager.cpp
+++ b/editor/txmgr/TransactionManager.cpp
@@ -252,24 +252,26 @@ TransactionManager::EndBatch(bool aAllow
   // XXX The result of EndTransaction() or DidEndBatchNotify() if
   //     EndTransaction() succeeded.
   return rv;
 }
 
 NS_IMETHODIMP
 TransactionManager::GetNumberOfUndoItems(int32_t* aNumItems)
 {
-  *aNumItems = mUndoStack.GetSize();
+  *aNumItems = static_cast<int32_t>(NumberOfUndoItems());
+  MOZ_ASSERT(*aNumItems >= 0);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TransactionManager::GetNumberOfRedoItems(int32_t* aNumItems)
 {
-  *aNumItems = mRedoStack.GetSize();
+  *aNumItems = static_cast<int32_t>(NumberOfRedoItems());
+  MOZ_ASSERT(*aNumItems >= 0);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TransactionManager::GetMaxTransactionCount(int32_t* aMaxCount)
 {
   NS_ENSURE_TRUE(aMaxCount, NS_ERROR_NULL_POINTER);
   *aMaxCount = mMaxTransactionCount;
--- a/editor/txmgr/TransactionManager.h
+++ b/editor/txmgr/TransactionManager.h
@@ -31,16 +31,25 @@ public:
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(TransactionManager,
                                            nsITransactionManager)
 
   NS_DECL_NSITRANSACTIONMANAGER
 
   already_AddRefed<nsITransaction> PeekUndoStack();
   already_AddRefed<nsITransaction> PeekRedoStack();
 
+  size_t NumberOfUndoItems() const
+  {
+    return mUndoStack.GetSize();
+  }
+  size_t NumberOfRedoItems() const
+  {
+    return mRedoStack.GetSize();
+  }
+
   nsresult WillDoNotify(nsITransaction* aTransaction, bool* aInterrupt);
   nsresult DidDoNotify(nsITransaction* aTransaction, nsresult aExecuteResult);
   nsresult WillUndoNotify(nsITransaction* aTransaction, bool* aInterrupt);
   nsresult DidUndoNotify(nsITransaction* aTransaction, nsresult aUndoResult);
   nsresult WillRedoNotify(nsITransaction* aTransaction, bool *aInterrupt);
   nsresult DidRedoNotify(nsITransaction* aTransaction, nsresult aRedoResult);
   nsresult WillBeginBatchNotify(bool* aInterrupt);
   nsresult DidBeginBatchNotify(nsresult aResult);
@@ -65,9 +74,15 @@ private:
   TransactionStack mDoStack;
   TransactionStack mUndoStack;
   TransactionStack mRedoStack;
   nsCOMArray<nsITransactionListener> mListeners;
 };
 
 } // namespace mozilla
 
+mozilla::TransactionManager*
+nsITransactionManager::AsTransactionManager()
+{
+  return static_cast<mozilla::TransactionManager*>(this);
+}
+
 #endif // #ifndef mozilla_TransactionManager_h
--- a/editor/txmgr/nsITransactionManager.idl
+++ b/editor/txmgr/nsITransactionManager.idl
@@ -2,16 +2,22 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 #include "nsITransaction.idl"
 #include "nsITransactionListener.idl"
 
+%{C++
+namespace mozilla {
+class TransactionManager;
+} // namespace mozilla
+%}
+
 /**
  * The nsITransactionManager interface.
  * <P>
  * This interface is implemented by an object that wants to
  * manage/track transactions.
  */
 [scriptable, builtinclass, uuid(c77763df-0fb9-41a8-8074-8e882f605755)]
 interface nsITransactionManager : nsISupports
@@ -138,16 +144,26 @@ interface nsITransactionManager : nsISup
 
   /**
    * Removes a listener from the transaction manager's notification list.
    * <P>
    * The listener's Release() method is called.
    * @param aListener the lister to remove.
    */
   void RemoveListener(in nsITransactionListener aListener);
+
+%{C++
+  /**
+   * AsTransactionManager() returns a pointer to TransactionManager class.
+   *
+   * In order to avoid circular dependency issues, this method is defined
+   * in mozilla/TransactionManager.h.  Consumers need to #include that header.
+   */
+  inline mozilla::TransactionManager* AsTransactionManager();
+%}
 };
 
 %{ C++
 
 #define NS_TRANSACTIONMANAGER_CONTRACTID "@mozilla.org/transactionmanager;1"
 
 // 9C8F9601-801A-11d2-98BA-00805F297D89
 #define NS_TRANSACTIONMANAGER_CID                   \