Bug 1447924 - part 7: Implement AddTransactionListener() and RemoveTransactionListener() in EditorBase and TransactionManager r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 23 Mar 2018 18:55:56 +0900
changeset 772896 71a3dc0cc8a64624a11c7d6ac1db9ccc093aea3d
parent 772895 87048f97ca914d7989009246394bb904fd00a422
child 773020 6318aca5a52403a9b947439c6bc4fd3f39cd6174
push id104076
push usermasayuki@d-toybox.com
push dateTue, 27 Mar 2018 04:30:26 +0000
reviewersm_kato
bugs1447924
milestone61.0a1
Bug 1447924 - part 7: Implement AddTransactionListener() and RemoveTransactionListener() in EditorBase and TransactionManager r?m_kato nsITransactionListener::AddListener() and nsITransactionListener::RemoveListener() are of course virtual methods. Additionally, they are safe to call without grabbing the TransactionManager's instance with local variable. Therefore, if EditorBase has methods to add/remove transaction listener to/from its transaction manager, we don't need EditorBase::GetTransactionManager() anymore. So, this patch adds AddTransactionListener() and RemoveTransactionListener() to EditorBase and TransactionManager, and remove EditorBase::GetTransactionManager() (not nsIEditor's one). MozReview-Commit-ID: FkPa1YgfagD
editor/composer/nsEditingSession.cpp
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/txmgr/TransactionManager.cpp
editor/txmgr/TransactionManager.h
--- a/editor/composer/nsEditingSession.cpp
+++ b/editor/composer/nsEditingSession.cpp
@@ -465,21 +465,21 @@ nsEditingSession::SetupEditorOnWindow(mo
   RefPtr<Selection> selection = htmlEditor->GetSelection();
   if (NS_WARN_IF(!selection)) {
     return NS_ERROR_FAILURE;
   }
 
   htmlEditor->SetComposerCommandsUpdater(mComposerCommandsUpdater);
 
   // and as a transaction listener
-  nsCOMPtr<nsITransactionManager> txnMgr;
-  htmlEditor->GetTransactionManager(getter_AddRefs(txnMgr));
-  if (txnMgr) {
-    txnMgr->AddListener(mComposerCommandsUpdater);
-  }
+  MOZ_ASSERT(mComposerCommandsUpdater);
+  DebugOnly<bool> addedTransactionListener =
+    htmlEditor->AddTransactionListener(*mComposerCommandsUpdater);
+  NS_WARNING_ASSERTION(addedTransactionListener,
+    "Failed to add transaction listener to the editor");
 
   // Set context on all controllers to be the editor
   rv = SetEditorOnControllers(aWindow, htmlEditor);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Everything went fine!
   mEditorStatus = eEditorOK;
 
@@ -493,24 +493,21 @@ nsEditingSession::RemoveListenersAndCont
                                                 HTMLEditor* aHTMLEditor)
 {
   if (!mComposerCommandsUpdater || !aHTMLEditor) {
     return;
   }
 
   // Remove all the listeners
   aHTMLEditor->SetComposerCommandsUpdater(nullptr);
-
   aHTMLEditor->RemoveDocumentStateListener(mComposerCommandsUpdater);
-
-  nsCOMPtr<nsITransactionManager> txnMgr;
-  aHTMLEditor->GetTransactionManager(getter_AddRefs(txnMgr));
-  if (txnMgr) {
-    txnMgr->RemoveListener(mComposerCommandsUpdater);
-  }
+  DebugOnly<bool> removedTransactionListener =
+    aHTMLEditor->RemoveTransactionListener(*mComposerCommandsUpdater);
+  NS_WARNING_ASSERTION(removedTransactionListener,
+    "Failed to remove transaction listener from the editor");
 
   // Remove editor controllers from the window now that we're not
   // editing in that window any more.
   RemoveEditorControllers(aWindow);
 }
 
 /*---------------------------------------------------------------------------
 
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -839,37 +839,28 @@ NS_IMETHODIMP
 EditorBase::GetNumberOfRedoItems(int32_t* aNumItems)
 {
   *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)) {
+EditorBase::GetTransactionManager(nsITransactionManager** aTransactionManager)
+{
+  if (NS_WARN_IF(!aTransactionManager)) {
     return NS_ERROR_INVALID_ARG;
   }
-  *aTxnManager = GetTransactionManager().take();
-  if (NS_WARN_IF(!*aTxnManager)) {
+  if (NS_WARN_IF(!mTransactionManager)) {
     return NS_ERROR_FAILURE;
   }
+  NS_IF_ADDREF(*aTransactionManager = mTransactionManager);
   return NS_OK;
 }
 
-already_AddRefed<nsITransactionManager>
-EditorBase::GetTransactionManager() const
-{
-  nsCOMPtr<nsITransactionManager> transactionManager(mTransactionManager);
-  return transactionManager.forget();
-}
-
 NS_IMETHODIMP
 EditorBase::Undo(uint32_t aCount)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 EditorBase::CanUndo(bool* aIsEnabled,
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -45,16 +45,17 @@ class nsIDOMEventTarget;
 class nsIDOMNode;
 class nsIDocumentStateListener;
 class nsIEditActionListener;
 class nsIEditorObserver;
 class nsINode;
 class nsIPresShell;
 class nsISupports;
 class nsITransaction;
+class nsITransactionListener;
 class nsIWidget;
 class nsRange;
 
 namespace mozilla {
 class AddStyleSheetTransaction;
 class AutoRules;
 class AutoSelectionRestorer;
 class AutoTransactionsConserveSelection;
@@ -1155,16 +1156,37 @@ public:
   {
     if (!mTransactionManager) {
       return true;
     }
     return mTransactionManager->ClearUndoRedo();
   }
 
   /**
+   * Adds or removes transaction listener to or from the transaction manager.
+   * Note that TransactionManager does not check if the listener is in the
+   * array.  So, caller of AddTransactionListener() needs to manage if it's
+   * already been registered to the transaction manager.
+   */
+  bool AddTransactionListener(nsITransactionListener& aListener)
+  {
+    if (!mTransactionManager) {
+      return false;
+    }
+    return mTransactionManager->AddTransactionListener(aListener);
+  }
+  bool RemoveTransactionListener(nsITransactionListener& aListener)
+  {
+    if (!mTransactionManager) {
+      return false;
+    }
+    return mTransactionManager->RemoveTransactionListener(aListener);
+  }
+
+  /**
    * 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);
 
   static bool IsTextNode(nsIDOMNode* aNode);
@@ -1446,22 +1468,16 @@ public:
   bool OutputsMozDirty() const
   {
     // Return true for Composer (!IsInteractionAllowed()) or mail
     // (IsMailEditor()), but false for webpages.
     return !IsInteractionAllowed() || IsMailEditor();
   }
 
   /**
-   * GetTransactionManager() returns transaction manager associated with the
-   * editor.  This may return nullptr if undo/redo hasn't been enabled.
-   */
-  already_AddRefed<nsITransactionManager> GetTransactionManager() const;
-
-  /**
    * Get the input event target. This might return null.
    */
   virtual already_AddRefed<nsIContent> GetInputEventTargetContent() = 0;
 
   /**
    * Get the focused content, if we're focused.  Returns null otherwise.
    */
   virtual nsIContent* GetFocusedContent();
--- a/editor/txmgr/TransactionManager.cpp
+++ b/editor/txmgr/TransactionManager.cpp
@@ -430,25 +430,29 @@ TransactionManager::RemoveTopUndo()
 
   RefPtr<TransactionItem> lastUndo = mUndoStack.Pop();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TransactionManager::AddListener(nsITransactionListener* aListener)
 {
-  NS_ENSURE_TRUE(aListener, NS_ERROR_NULL_POINTER);
-  return mListeners.AppendObject(aListener) ? NS_OK : NS_ERROR_FAILURE;
+  if (NS_WARN_IF(!aListener)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  return AddTransactionListener(*aListener) ? NS_OK : NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 TransactionManager::RemoveListener(nsITransactionListener* aListener)
 {
-  NS_ENSURE_TRUE(aListener, NS_ERROR_NULL_POINTER);
-  return mListeners.RemoveObject(aListener) ? NS_OK : NS_ERROR_FAILURE;
+  if (NS_WARN_IF(!aListener)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  return RemoveTransactionListener(*aListener) ? NS_OK : NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 TransactionManager::ClearUndoStack()
 {
   if (NS_WARN_IF(!mDoStack.IsEmpty())) {
     return NS_ERROR_FAILURE;
   }
--- a/editor/txmgr/TransactionManager.h
+++ b/editor/txmgr/TransactionManager.h
@@ -58,16 +58,26 @@ public:
     if (NS_WARN_IF(!mDoStack.IsEmpty())) {
       return false;
     }
     mUndoStack.Clear();
     mRedoStack.Clear();
     return true;
   }
 
+  bool AddTransactionListener(nsITransactionListener& aListener)
+  {
+    // XXX Shouldn't we check if aListener has already been in mListeners?
+    return mListeners.AppendObject(&aListener);
+  }
+  bool RemoveTransactionListener(nsITransactionListener& aListener)
+  {
+    return mListeners.RemoveObject(&aListener);
+  }
+
   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);