Bug 1447924 - part 4: Optimize NumbeOfUndoItems(), NumbeOfRedoItems(), CanUndo() and CanRedo() of EditorBase r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 23 Mar 2018 00:08:38 +0900
changeset 772893 8afcf0e9ec2e0d245f369e35cbb9cc7c4c57e19b
parent 772892 87f46f9c789eb2a48c2d645c7648e8cef2426ab1
child 772894 882d1d3633531c49301a308309d595cf32b30a73
push id104076
push usermasayuki@d-toybox.com
push dateTue, 27 Mar 2018 04:30:26 +0000
reviewersm_kato
bugs1447924
milestone61.0a1
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                   \