Bug 1433345 - part 2: Make HTMLEditor store ComposerCommandsUpdater directly r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 26 Jan 2018 14:38:05 +0900
changeset 748210 56edc2197f05783ae7bffdfd00ef057de2731c31
parent 748209 aaaa7fadd796cb5db5913783dc8aa8f276bbbbf6
child 748211 421d672ccb1d3b0ae09cb583795b6750d2213b96
push id97089
push usermasayuki@d-toybox.com
push dateMon, 29 Jan 2018 07:02:13 +0000
reviewersm_kato
bugs1433345
milestone60.0a1
Bug 1433345 - part 2: Make HTMLEditor store ComposerCommandsUpdater directly r?m_kato For making ComposerCommandsUpdater not derived from nsISelectionListener, HTMLEditor needs to store it directly. This patch also makes ComposerCommandsUpdater cycle collectable because it stores a strong pointer and HTMLEditor also needs to store it with RefPtr. Therefore, ComposerCommandsUpdater becomes unnecessary to use nsWeakPtr. MozReview-Commit-ID: 2WZnLdq15FK
editor/composer/ComposerCommandsUpdater.cpp
editor/composer/ComposerCommandsUpdater.h
editor/composer/nsEditingSession.cpp
editor/composer/nsEditingSession.h
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
--- a/editor/composer/ComposerCommandsUpdater.cpp
+++ b/editor/composer/ComposerCommandsUpdater.cpp
@@ -38,22 +38,33 @@ ComposerCommandsUpdater::ComposerCommand
 ComposerCommandsUpdater::~ComposerCommandsUpdater()
 {
   // cancel any outstanding update timer
   if (mUpdateTimer) {
     mUpdateTimer->Cancel();
   }
 }
 
-NS_IMPL_ISUPPORTS(ComposerCommandsUpdater,
-                  nsISelectionListener,
-                  nsIDocumentStateListener,
-                  nsITransactionListener,
-                  nsITimerCallback,
-                  nsINamed)
+NS_IMPL_CYCLE_COLLECTING_ADDREF(ComposerCommandsUpdater)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(ComposerCommandsUpdater)
+
+NS_INTERFACE_MAP_BEGIN(ComposerCommandsUpdater)
+  NS_INTERFACE_MAP_ENTRY(nsISelectionListener)
+  NS_INTERFACE_MAP_ENTRY(nsIDocumentStateListener)
+  NS_INTERFACE_MAP_ENTRY(nsITransactionListener)
+  NS_INTERFACE_MAP_ENTRY(nsITimerCallback)
+  NS_INTERFACE_MAP_ENTRY(nsINamed)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDocumentStateListener)
+  NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(ComposerCommandsUpdater)
+NS_INTERFACE_MAP_END
+
+NS_IMPL_CYCLE_COLLECTION(ComposerCommandsUpdater,
+                         mUpdateTimer,
+                         mDOMWindow,
+                         mDocShell)
 
 #if 0
 #pragma mark -
 #endif
 
 NS_IMETHODIMP
 ComposerCommandsUpdater::NotifyDocumentCreated()
 {
@@ -220,19 +231,21 @@ ComposerCommandsUpdater::DidMerge(nsITra
 
 #if 0
 #pragma mark -
 #endif
 
 nsresult
 ComposerCommandsUpdater::Init(nsPIDOMWindowOuter* aDOMWindow)
 {
-  NS_ENSURE_ARG(aDOMWindow);
-  mDOMWindow = do_GetWeakReference(aDOMWindow);
-  mDocShell = do_GetWeakReference(aDOMWindow->GetDocShell());
+  if (NS_WARN_IF(!aDOMWindow)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  mDOMWindow = aDOMWindow;
+  mDocShell = aDOMWindow->GetDocShell();
   return NS_OK;
 }
 
 nsresult
 ComposerCommandsUpdater::PrimeUpdateTimer()
 {
   if (!mUpdateTimer) {
     mUpdateTimer = NS_NewTimer();;
@@ -339,33 +352,36 @@ ComposerCommandsUpdater::UpdateOneComman
   commandUpdater->CommandStatusChanged(aCommand);
 
   return NS_OK;
 }
 
 bool
 ComposerCommandsUpdater::SelectionIsCollapsed()
 {
-  nsCOMPtr<nsPIDOMWindowOuter> domWindow = do_QueryReferent(mDOMWindow);
-  NS_ENSURE_TRUE(domWindow, true);
+  if (NS_WARN_IF(!mDOMWindow)) {
+    return true;
+  }
 
-  nsCOMPtr<nsISelection> domSelection = domWindow->GetSelection();
+  nsCOMPtr<nsISelection> domSelection = mDOMWindow->GetSelection();
   if (NS_WARN_IF(!domSelection)) {
     return false;
   }
 
   return domSelection->AsSelection()->IsCollapsed();
 }
 
 already_AddRefed<nsPICommandUpdater>
 ComposerCommandsUpdater::GetCommandUpdater()
 {
-  nsCOMPtr<nsIDocShell> docShell = do_QueryReferent(mDocShell);
-  NS_ENSURE_TRUE(docShell, nullptr);
-  nsCOMPtr<nsICommandManager> manager = docShell->GetCommandManager();
+  if (NS_WARN_IF(!mDocShell)) {
+    return nullptr;
+  }
+
+  nsCOMPtr<nsICommandManager> manager = mDocShell->GetCommandManager();
   nsCOMPtr<nsPICommandUpdater> updater = do_QueryInterface(manager);
   return updater.forget();
 }
 
 NS_IMETHODIMP
 ComposerCommandsUpdater::GetName(nsACString& aName)
 {
   aName.AssignLiteral("ComposerCommandsUpdater");
--- a/editor/composer/ComposerCommandsUpdater.h
+++ b/editor/composer/ComposerCommandsUpdater.h
@@ -3,43 +3,47 @@
  * 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/. */
 
 #ifndef mozilla_ComposerCommandsUpdater_h
 #define mozilla_ComposerCommandsUpdater_h
 
 #include "nsCOMPtr.h"                   // for already_AddRefed, nsCOMPtr
+#include "nsCycleCollectionParticipant.h"
 #include "nsIDocumentStateListener.h"
 #include "nsINamed.h"
 #include "nsISelectionListener.h"
 #include "nsISupportsImpl.h"            // for NS_DECL_ISUPPORTS
 #include "nsITimer.h"                   // for NS_DECL_NSITIMERCALLBACK, etc
 #include "nsITransactionListener.h"     // for nsITransactionListener
 #include "nsIWeakReferenceUtils.h"      // for nsWeakPtr
 #include "nscore.h"                     // for NS_IMETHOD, nsresult, etc
 
-class nsPIDOMWindowOuter;
+class nsIDocShell;
 class nsITransaction;
 class nsITransactionManager;
+class nsPIDOMWindowOuter;
 class nsPICommandUpdater;
 
 namespace mozilla {
 
 class ComposerCommandsUpdater final : public nsISelectionListener
                                     , public nsIDocumentStateListener
                                     , public nsITransactionListener
                                     , public nsITimerCallback
                                     , public nsINamed
 {
 public:
   ComposerCommandsUpdater();
 
   // nsISupports
-  NS_DECL_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(ComposerCommandsUpdater,
+                                           nsIDocumentStateListener)
 
   // nsISelectionListener
   NS_DECL_NSISELECTIONLISTENER
 
   // nsIDocumentStateListener
   NS_DECL_NSIDOCUMENTSTATELISTENER
 
   // nsITimerCallback
@@ -69,19 +73,19 @@ protected:
   nsresult UpdateCommandGroup(const nsAString& aCommandGroup);
 
   already_AddRefed<nsPICommandUpdater> GetCommandUpdater();
 
   nsresult PrimeUpdateTimer();
   void TimerCallback();
 
   nsCOMPtr<nsITimer> mUpdateTimer;
+  nsCOMPtr<nsPIDOMWindowOuter> mDOMWindow;
+  nsCOMPtr<nsIDocShell> mDocShell;
 
-  nsWeakPtr mDOMWindow;
-  nsWeakPtr mDocShell;
   int8_t mDirtyState;
   int8_t mSelectionCollapsed;
   bool mFirstDoOfFirstUndo;
 };
 
 } // namespace mozilla
 
 #endif // #ifndef mozilla_ComposerCommandsUpdater_h
--- a/editor/composer/nsEditingSession.cpp
+++ b/editor/composer/nsEditingSession.cpp
@@ -373,26 +373,26 @@ nsEditingSession::SetupEditorOnWindow(mo
     needHTMLController = true;
   }
 
   if (mInteractive) {
     mEditorFlags |= nsIPlaintextEditor::eEditorAllowInteraction;
   }
 
   // make the UI state maintainer
-  mStateMaintainer = new ComposerCommandsUpdater();
+  mComposerCommandsUpdater = new ComposerCommandsUpdater();
 
   // now init the state maintainer
   // This allows notification of error state
   //  even if we don't create an editor
-  rv = mStateMaintainer->Init(window);
+  rv = mComposerCommandsUpdater->Init(window);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (mEditorStatus != eEditorCreationInProgress) {
-    mStateMaintainer->NotifyDocumentCreated();
+    mComposerCommandsUpdater->NotifyDocumentCreated();
     return NS_ERROR_FAILURE;
   }
 
   // Create editor and do other things
   //  only if we haven't found some error above,
   nsCOMPtr<nsIDocShell> docShell = window->GetDocShell();
   NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);
   nsCOMPtr<nsIPresShell> presShell = docShell->GetPresShell();
@@ -450,36 +450,39 @@ nsEditingSession::SetupEditorOnWindow(mo
 
   nsCOMPtr<nsIDOMDocument> domDoc;
   rv = contentViewer->GetDOMDocument(getter_AddRefs(domDoc));
   NS_ENSURE_SUCCESS(rv, rv);
   NS_ENSURE_TRUE(domDoc, NS_ERROR_FAILURE);
 
   // Set up as a doc state listener
   // Important! We must have this to broadcast the "obs_documentCreated" message
-  rv = htmlEditor->AddDocumentStateListener(mStateMaintainer);
+  rv = htmlEditor->AddDocumentStateListener(mComposerCommandsUpdater);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = htmlEditor->Init(domDoc, nullptr /* root content */,
                         nullptr, mEditorFlags, EmptyString());
   NS_ENSURE_SUCCESS(rv, rv);
 
   RefPtr<Selection> selection = htmlEditor->GetSelection();
   if (NS_WARN_IF(!selection)) {
     return NS_ERROR_FAILURE;
   }
 
-  rv = selection->AddSelectionListener(mStateMaintainer);
-  NS_ENSURE_SUCCESS(rv, rv);
+  rv = selection->AddSelectionListener(mComposerCommandsUpdater);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+  htmlEditor->SetComposerCommandsUpdater(mComposerCommandsUpdater);
 
   // and as a transaction listener
   nsCOMPtr<nsITransactionManager> txnMgr;
   htmlEditor->GetTransactionManager(getter_AddRefs(txnMgr));
   if (txnMgr) {
-    txnMgr->AddListener(mStateMaintainer);
+    txnMgr->AddListener(mComposerCommandsUpdater);
   }
 
   // Set context on all controllers to be the editor
   rv = SetEditorOnControllers(aWindow, htmlEditor);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Everything went fine!
   mEditorStatus = eEditorOK;
@@ -488,32 +491,33 @@ nsEditingSession::SetupEditorOnWindow(mo
   return htmlEditor->PostCreate();
 }
 
 // Removes all listeners and controllers from aWindow and aEditor.
 void
 nsEditingSession::RemoveListenersAndControllers(nsPIDOMWindowOuter* aWindow,
                                                 HTMLEditor* aHTMLEditor)
 {
-  if (!mStateMaintainer || !aHTMLEditor) {
+  if (!mComposerCommandsUpdater || !aHTMLEditor) {
     return;
   }
 
   // Remove all the listeners
   RefPtr<Selection> selection = aHTMLEditor->GetSelection();
   if (selection) {
-    selection->RemoveSelectionListener(mStateMaintainer);
+    selection->RemoveSelectionListener(mComposerCommandsUpdater);
   }
+  aHTMLEditor->SetComposerCommandsUpdater(nullptr);
 
-  aHTMLEditor->RemoveDocumentStateListener(mStateMaintainer);
+  aHTMLEditor->RemoveDocumentStateListener(mComposerCommandsUpdater);
 
   nsCOMPtr<nsITransactionManager> txnMgr;
   aHTMLEditor->GetTransactionManager(getter_AddRefs(txnMgr));
   if (txnMgr) {
-    txnMgr->RemoveListener(mStateMaintainer);
+    txnMgr->RemoveListener(mComposerCommandsUpdater);
   }
 
   // Remove editor controllers from the window now that we're not
   // editing in that window any more.
   RemoveEditorControllers(aWindow);
 }
 
 /*---------------------------------------------------------------------------
@@ -552,17 +556,17 @@ nsEditingSession::TearDownEditorOnWindow
   nsCOMPtr<nsIDocShell> docShell = window->GetDocShell();
   NS_ENSURE_STATE(docShell);
 
   RefPtr<HTMLEditor> htmlEditor = docShell->GetHTMLEditor();
   if (stopEditing) {
     htmlDoc->TearingDownEditor();
   }
 
-  if (mStateMaintainer && htmlEditor) {
+  if (mComposerCommandsUpdater && htmlEditor) {
     // Null out the editor on the controllers first to prevent their weak
     // references from pointing to a destroyed editor.
     SetEditorOnControllers(aWindow, nullptr);
   }
 
   // Null out the editor on the docShell to trigger PreDestroy which
   // needs to happen before document state listeners are removed below.
   docShell->SetEditor(nullptr);
@@ -1278,17 +1282,18 @@ nsEditingSession::RestoreAnimationMode(n
   presContext->SetImageAnimationMode(mImageAnimationMode);
 }
 
 nsresult
 nsEditingSession::DetachFromWindow(mozIDOMWindowProxy* aWindow)
 {
   NS_ENSURE_TRUE(mDoneSetup, NS_OK);
 
-  NS_ASSERTION(mStateMaintainer, "mStateMaintainer should exist.");
+  NS_ASSERTION(mComposerCommandsUpdater,
+               "mComposerCommandsUpdater should exist.");
 
   // Kill any existing reload timer
   if (mLoadBlankDocTimer) {
     mLoadBlankDocTimer->Cancel();
     mLoadBlankDocTimer = nullptr;
   }
 
   auto* window = nsPIDOMWindowOuter::From(aWindow);
@@ -1308,17 +1313,18 @@ nsEditingSession::DetachFromWindow(mozID
 }
 
 nsresult
 nsEditingSession::ReattachToWindow(mozIDOMWindowProxy* aWindow)
 {
   NS_ENSURE_TRUE(mDoneSetup, NS_OK);
   NS_ENSURE_TRUE(aWindow, NS_ERROR_FAILURE);
 
-  NS_ASSERTION(mStateMaintainer, "mStateMaintainer should exist.");
+  NS_ASSERTION(mComposerCommandsUpdater,
+               "mComposerCommandsUpdater should exist.");
 
   // Imitate nsEditorDocShell::MakeEditable() to reattach the
   // old editor ot the window.
   nsresult rv;
 
   auto* window = nsPIDOMWindowOuter::From(aWindow);
   nsIDocShell *docShell = window->GetDocShell();
   NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);
@@ -1345,18 +1351,18 @@ nsEditingSession::ReattachToWindow(mozID
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = SetupEditorCommandController("@mozilla.org/editor/editordocstatecontroller;1",
                                     aWindow,
                                     static_cast<nsIEditingSession*>(this),
                                     &mDocStateControllerId);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  if (mStateMaintainer) {
-    mStateMaintainer->Init(window);
+  if (mComposerCommandsUpdater) {
+    mComposerCommandsUpdater->Init(window);
   }
 
   // Get editor
   RefPtr<HTMLEditor> htmlEditor = GetHTMLEditorForWindow(aWindow);
   if (NS_WARN_IF(!htmlEditor)) {
     return NS_ERROR_FAILURE;
   }
 
--- a/editor/composer/nsEditingSession.h
+++ b/editor/composer/nsEditingSession.h
@@ -125,17 +125,17 @@ protected:
 
   bool            mProgressListenerRegistered;
 
   // The image animation mode before it was turned off.
   uint16_t        mImageAnimationMode;
 
   // THE REMAINING MEMBER VARIABLES WILL BECOME A SET WHEN WE EDIT
   // MORE THAN ONE EDITOR PER EDITING SESSION
-  RefPtr<mozilla::ComposerCommandsUpdater> mStateMaintainer;
+  RefPtr<mozilla::ComposerCommandsUpdater> mComposerCommandsUpdater;
 
   // Save the editor type so we can create the editor after loading uri
   nsCString       mEditorType;
   uint32_t        mEditorFlags;
   uint32_t        mEditorStatus;
   uint32_t        mBaseCommandControllerId;
   uint32_t        mDocStateControllerId;
   uint32_t        mHTMLCommandControllerId;
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -1,15 +1,16 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* 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/HTMLEditor.h"
 
+#include "mozilla/ComposerCommandsUpdater.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/EditAction.h"
 #include "mozilla/EditorDOMPoint.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/mozInlineSpellChecker.h"
 #include "mozilla/TextEvents.h"
 
 #include "nsCRT.h"
@@ -185,25 +186,27 @@ HTMLEditor::HideAnonymousEditingUIs()
     HideResizers();
   }
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(HTMLEditor)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLEditor, TextEditor)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mTypeInState)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mComposerCommandsUpdater)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mStyleSheets)
 
   tmp->HideAnonymousEditingUIs();
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLinkHandler)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(HTMLEditor, TextEditor)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTypeInState)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mComposerCommandsUpdater)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStyleSheets)
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTopLeftHandle)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTopHandle)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTopRightHandle)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLeftHandle)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRightHandle)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mBottomLeftHandle)
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -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/. */
 
 #ifndef mozilla_HTMLEditor_h
 #define mozilla_HTMLEditor_h
 
 #include "mozilla/Attributes.h"
+#include "mozilla/ComposerCommandsUpdater.h"
 #include "mozilla/CSSEditUtils.h"
 #include "mozilla/ManualNAC.h"
 #include "mozilla/StyleSheet.h"
 #include "mozilla/TextEditor.h"
 #include "mozilla/TextEditRules.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/File.h"
@@ -249,16 +250,30 @@ public:
                                           nsAtom* aAttr,
                                           const nsAString& aValue,
                                           bool* aFirst,
                                           bool* aAny,
                                           bool* aAll,
                                           nsAString& outValue);
   nsresult RemoveInlineProperty(nsAtom* aProperty,
                                 nsAtom* aAttribute);
+
+  /**
+   * SetComposerCommandsUpdater() sets or unsets mComposerCommandsUpdater.
+   * This will crash in debug build if the editor already has an instance
+   * but called with another instance.
+   */
+  void SetComposerCommandsUpdater(
+         ComposerCommandsUpdater* aComposerCommandsUpdater)
+  {
+    MOZ_ASSERT(!aComposerCommandsUpdater || !mComposerCommandsUpdater ||
+               aComposerCommandsUpdater == mComposerCommandsUpdater);
+    mComposerCommandsUpdater = aComposerCommandsUpdater;
+  }
+
 protected:
   virtual ~HTMLEditor();
 
   using EditorBase::IsBlockNode;
   virtual bool IsBlockNode(nsINode *aNode) override;
 
 public:
   // XXX Why don't we move following methods above for grouping by the origins?
@@ -968,16 +983,17 @@ protected:
    *                    insure we reset the caret in a table-editing method.
    */
   void SetSelectionAfterTableEdit(nsIDOMElement* aTable,
                                   int32_t aRow, int32_t aCol,
                                   int32_t aDirection, bool aSelected);
 
 protected:
   RefPtr<TypeInState> mTypeInState;
+  RefPtr<ComposerCommandsUpdater> mComposerCommandsUpdater;
 
   bool mCRInParagraphCreatesParagraph;
 
   bool mCSSAware;
   UniquePtr<CSSEditUtils> mCSSEditUtils;
 
   // Used by GetFirstSelectedCell and GetNextSelectedCell
   int32_t  mSelectedCellIndex;