Bug 1432528 - part 2: Make EditorBase treat TextInputListener directly and make TextInputListener not derived from nsIEditorObserver r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 24 Jan 2018 12:50:01 +0900
changeset 455667 39095009ebddbb7b657553d6fa880addc70c3cf3
parent 455666 02c94b3356098364e0a2811a5c229fe4dce3f433
child 455668 a356d22ebcce1b4ec033f1e2cb2c35df0d93b13f
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1432528
milestone60.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 1432528 - part 2: Make EditorBase treat TextInputListener directly and make TextInputListener not derived from nsIEditorObserver r=m_kato Now, EditorBase can store TextInputListener directly instead of as nsIEditorObserver. And then, EditorBase can call its EditAction() method directly. Therefore, we can make TextInputListener not derived from nsIEditorObserver. MozReview-Commit-ID: 4qPnnvReLKy
dom/html/TextInputListener.h
dom/html/nsTextEditorState.cpp
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
--- a/dom/html/TextInputListener.h
+++ b/dom/html/TextInputListener.h
@@ -2,32 +2,31 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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_TextInputListener_h
 #define mozilla_TextInputListener_h
 
+#include "nsCycleCollectionParticipant.h"
 #include "nsIDOMEventListener.h"
-#include "nsIEditorObserver.h"
 #include "nsISelectionListener.h"
 #include "nsStringFwd.h"
 #include "nsWeakReference.h"
 
 class nsIFrame;
 class nsISelection;
 class nsITextControlElement;
 class nsTextControlFrame;
 
 namespace mozilla {
 
 class TextInputListener final : public nsISelectionListener
                               , public nsIDOMEventListener
-                              , public nsIEditorObserver
                               , public nsSupportsWeakReference
 {
 public:
   explicit TextInputListener(nsITextControlElement* aTextControlElement);
 
   void SetFrame(nsIFrame* aTextControlFrame)
   {
     mFrame = aTextControlFrame;
@@ -36,24 +35,32 @@ public:
   {
     mSettingValue = aValue;
   }
   void SetValueChanged(bool aSetValueChanged)
   {
     mSetValueChanged = aSetValueChanged;
   }
 
-  // aFrame is an optional pointer to our frame, if not passed the method will
-  // use mFrame to compute it lazily.
+  /**
+   * aFrame is an optional pointer to our frame, if not passed the method will
+   * use mFrame to compute it lazily.
+   */
   void HandleValueChanged(nsTextControlFrame* aFrame = nullptr);
 
-  NS_DECL_ISUPPORTS
+  /**
+   * OnEditActionHandled() is called when the editor handles each edit action.
+   */
+  void OnEditActionHandled();
+
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(TextInputListener,
+                                           nsISelectionListener)
   NS_DECL_NSISELECTIONLISTENER
   NS_DECL_NSIDOMEVENTLISTENER
-  NS_DECL_NSIEDITOROBSERVER
 
 protected:
   virtual ~TextInputListener() = default;
 
   nsresult UpdateTextInputCommands(const nsAString& aCommandsToUpdate,
                                    nsISelection* aSelection = nullptr,
                                    int16_t aReason = 0);
 
--- a/dom/html/nsTextEditorState.cpp
+++ b/dom/html/nsTextEditorState.cpp
@@ -800,21 +800,28 @@ TextInputListener::TextInputListener(nsI
   , mSelectionWasCollapsed(true)
   , mHadUndoItems(false)
   , mHadRedoItems(false)
   , mSettingValue(false)
   , mSetValueChanged(true)
 {
 }
 
-NS_IMPL_ISUPPORTS(TextInputListener,
-                  nsISelectionListener,
-                  nsIEditorObserver,
-                  nsISupportsWeakReference,
-                  nsIDOMEventListener)
+NS_IMPL_CYCLE_COLLECTING_ADDREF(TextInputListener)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(TextInputListener)
+
+NS_INTERFACE_MAP_BEGIN(TextInputListener)
+  NS_INTERFACE_MAP_ENTRY(nsISelectionListener)
+  NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
+  NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener)
+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsISelectionListener)
+  NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(TextInputListener)
+NS_INTERFACE_MAP_END
+
+NS_IMPL_CYCLE_COLLECTION_0(TextInputListener)
 
 // BEGIN nsIDOMSelectionListener
 
 NS_IMETHODIMP
 TextInputListener::NotifySelectionChanged(nsIDOMDocument* aDOMDocument,
                                           nsISelection* aSelection,
                                           int16_t aReason)
 {
@@ -963,25 +970,23 @@ TextInputListener::HandleEvent(nsIDOMEve
   keyEvent->mWidget = widget;
   if (keyEvent->ExecuteEditCommands(nativeKeyBindingsType,
                                     DoCommandCallback, mFrame)) {
     aEvent->PreventDefault();
   }
   return NS_OK;
 }
 
-// BEGIN nsIEditorObserver
-
-NS_IMETHODIMP
-TextInputListener::EditAction()
+void
+TextInputListener::OnEditActionHandled()
 {
   if (!mFrame) {
     // We've been disconnected from the nsTextEditorState object, nothing to do
     // here.
-    return NS_OK;
+    return;
   }
 
   AutoWeakFrame weakFrame = mFrame;
 
   nsITextControlFrame* frameBase = do_QueryFrame(mFrame);
   nsTextControlFrame* frame = static_cast<nsTextControlFrame*> (frameBase);
   NS_ASSERTION(frame, "Where is our frame?");
   //
@@ -997,22 +1002,20 @@ TextInputListener::EditAction()
     // Modify the menu if undo or redo items are different
     UpdateTextInputCommands(NS_LITERAL_STRING("undo"));
 
     mHadUndoItems = numUndoItems != 0;
     mHadRedoItems = numRedoItems != 0;
   }
 
   if (!weakFrame.IsAlive()) {
-    return NS_OK;
+    return;
   }
 
   HandleValueChanged(frame);
-
-  return NS_OK;
 }
 
 void
 TextInputListener::HandleValueChanged(nsTextControlFrame* aFrame)
 {
   // Make sure we know we were changed (do NOT set this to false if there are
   // no undo items; JS could change the value and we'd still need to save it)
   if (mSetValueChanged) {
@@ -1025,31 +1028,16 @@ TextInputListener::HandleValueChanged(ns
   }
 
   if (!mSettingValue) {
     mTxtCtrlElement->OnValueChanged(/* aNotify = */ true,
                                     /* aWasInteractiveUserChange = */ true);
   }
 }
 
-NS_IMETHODIMP
-TextInputListener::BeforeEditAction()
-{
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-TextInputListener::CancelEditAction()
-{
-  return NS_OK;
-}
-
-// END nsIEditorObserver
-
-
 nsresult
 TextInputListener::UpdateTextInputCommands(const nsAString& aCommandsToUpdate,
                                            nsISelection* aSelection,
                                            int16_t aReason)
 {
   nsIContent* content = mFrame->GetContent();
   NS_ENSURE_TRUE(content, NS_ERROR_FAILURE);
 
@@ -1543,17 +1531,17 @@ nsTextEditorState::PrepareEditor(const n
 
   if (!mEditorInitialized) {
     newTextEditor->PostCreate();
     mEverInited = true;
     mEditorInitialized = true;
   }
 
   if (mTextListener) {
-    newTextEditor->AddEditorObserver(mTextListener);
+    newTextEditor->SetTextInputListener(mTextListener);
   }
 
   // Restore our selection after being bound to a new frame
   HTMLInputElement* number = GetParentNumberControl(mBoundFrame);
   if (number ? number->IsSelectionCached() : mSelectionCached) {
     if (mRestoringSelection) // paranoia
       mRestoringSelection->Revoke();
     mRestoringSelection = new RestoreSelectionState(this, mBoundFrame);
@@ -2013,19 +2001,16 @@ nsTextEditorState::GetParentNumberContro
   return nullptr;
 }
 
 void
 nsTextEditorState::DestroyEditor()
 {
   // notify the editor that we are going away
   if (mEditorInitialized) {
-    if (mTextListener) {
-      mTextEditor->RemoveEditorObserver(mTextListener);
-    }
     mTextEditor->PreDestroy(true);
     mEditorInitialized = false;
   }
 }
 
 void
 nsTextEditorState::UnbindFromFrame(nsTextControlFrame* aFrame)
 {
@@ -2035,17 +2020,17 @@ nsTextEditorState::UnbindFromFrame(nsTex
   MOZ_ASSERT(aFrame == mBoundFrame, "Unbinding from the wrong frame");
   NS_ENSURE_TRUE_VOID(!aFrame || aFrame == mBoundFrame);
 
   // If the editor is modified but nsIEditorObserver::EditAction() hasn't been
   // called yet, we need to notify it here because editor may be destroyed
   // before EditAction() is called if selection listener causes flushing layout.
   if (mTextListener && mTextEditor && mEditorInitialized &&
       mTextEditor->IsInEditAction()) {
-    mTextListener->EditAction();
+    mTextListener->OnEditActionHandled();
   }
 
   // We need to start storing the value outside of the editor if we're not
   // going to use it anymore, so retrieve it for now.
   nsAutoString value;
   GetValue(value, true);
 
   if (mRestoringSelection) {
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -38,16 +38,17 @@
 #include "mozilla/mozalloc.h"           // for operator new, etc.
 #include "mozilla/mozInlineSpellChecker.h" // for mozInlineSpellChecker
 #include "mozilla/mozSpellChecker.h"    // for mozSpellChecker
 #include "mozilla/Preferences.h"        // for Preferences
 #include "mozilla/RangeBoundary.h"      // for RawRangeBoundary, RangeBoundary
 #include "mozilla/dom/Selection.h"      // for Selection, etc.
 #include "mozilla/Services.h"           // for GetObserverService
 #include "mozilla/TextComposition.h"    // for TextComposition
+#include "mozilla/TextInputListener.h"  // for TextInputListener
 #include "mozilla/TextServicesDocument.h" // for TextServicesDocument
 #include "mozilla/TextEvents.h"
 #include "mozilla/dom/Element.h"        // for Element, nsINode::AsElement
 #include "mozilla/dom/HTMLBodyElement.h"
 #include "mozilla/dom/Text.h"
 #include "mozilla/dom/Event.h"
 #include "nsAString.h"                  // for nsAString::Length, etc.
 #include "nsCCUncollectableMarker.h"    // for nsCCUncollectableMarker
@@ -164,16 +165,17 @@ EditorBase::~EditorBase()
 NS_IMPL_CYCLE_COLLECTION_CLASS(EditorBase)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(EditorBase)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mRootElement)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mSelectionController)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocument)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mInlineSpellChecker)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mTextServicesDocument)
+ NS_IMPL_CYCLE_COLLECTION_UNLINK(mTextInputListener)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mTxnMgr)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mActionListeners)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mEditorObservers)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocStateListeners)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mEventTarget)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mEventListener)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mPlaceholderTransaction)
  NS_IMPL_CYCLE_COLLECTION_UNLINK(mSavedSel);
@@ -187,16 +189,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
      nsCCUncollectableMarker::InGeneration(cb, currentDoc->GetMarkedCCGeneration())) {
    return NS_SUCCESS_INTERRUPTED_TRAVERSE;
  }
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRootElement)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSelectionController)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocument)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mInlineSpellChecker)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTextServicesDocument)
+ NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTextInputListener)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTxnMgr)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mActionListeners)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEditorObservers)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocStateListeners)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEventTarget)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEventListener)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPlaceholderTransaction)
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSavedSel);
@@ -339,16 +342,24 @@ EditorBase::PostCreate()
 
   // FYI: This call might cause destroying this editor.
   IMEStateManager::OnEditorInitialized(*this);
 
   return NS_OK;
 }
 
 void
+EditorBase::SetTextInputListener(TextInputListener* aTextInputListener)
+{
+  MOZ_ASSERT(!mTextInputListener || !aTextInputListener ||
+             mTextInputListener == aTextInputListener);
+  mTextInputListener = aTextInputListener;
+}
+
+void
 EditorBase::CreateEventListeners()
 {
   // Don't create the handler twice
   if (!mEventListener) {
     mEventListener = new EditorEventListener();
   }
 }
 
@@ -466,16 +477,17 @@ EditorBase::PreDestroy(bool aDestroyingF
   RemoveEventListeners();
   // If this editor is still hiding the caret, we need to restore it.
   HideCaret(false);
   mActionListeners.Clear();
   mEditorObservers.Clear();
   mDocStateListeners.Clear();
   mInlineSpellChecker = nullptr;
   mTextServicesDocument = nullptr;
+  mTextInputListener = nullptr;
   mSpellcheckCheckboxState = eTriUnset;
   mRootElement = nullptr;
 
   // Transaction may grab this instance.  Therefore, they should be released
   // here for stopping the circular reference with this instance.
   if (mTxnMgr) {
     mTxnMgr->Clear();
     mTxnMgr = nullptr;
@@ -2136,44 +2148,60 @@ private:
   RefPtr<EditorBase> mEditorBase;
   nsCOMPtr<nsIContent> mTarget;
   bool mIsComposing;
 };
 
 void
 EditorBase::NotifyEditorObservers(NotificationForEditorObservers aNotification)
 {
-  // Copy the observers since EditAction()s can modify mEditorObservers.
-  AutoEditorObserverArray observers(mEditorObservers);
   switch (aNotification) {
     case eNotifyEditorObserversOfEnd:
       mIsInEditAction = false;
-      for (auto& observer : observers) {
-        observer->EditAction();
+
+      if (mTextInputListener) {
+        RefPtr<TextInputListener> listener = mTextInputListener;
+        listener->OnEditActionHandled();
+      }
+
+      if (!mEditorObservers.IsEmpty()) {
+        // Copy the observers since EditAction()s can modify mEditorObservers.
+        AutoEditorObserverArray observers(mEditorObservers);
+        for (auto& observer : observers) {
+          observer->EditAction();
+        }
       }
 
       if (!mDispatchInputEvent) {
         return;
       }
 
       FireInputEvent();
       break;
     case eNotifyEditorObserversOfBefore:
       if (NS_WARN_IF(mIsInEditAction)) {
         break;
       }
       mIsInEditAction = true;
-      for (auto& observer : observers) {
-        observer->BeforeEditAction();
+      if (!mEditorObservers.IsEmpty()) {
+        // Copy the observers since EditAction()s can modify mEditorObservers.
+        AutoEditorObserverArray observers(mEditorObservers);
+        for (auto& observer : observers) {
+          observer->BeforeEditAction();
+        }
       }
       break;
     case eNotifyEditorObserversOfCancel:
       mIsInEditAction = false;
-      for (auto& observer : observers) {
-        observer->CancelEditAction();
+      if (!mEditorObservers.IsEmpty()) {
+        // Copy the observers since EditAction()s can modify mEditorObservers.
+        AutoEditorObserverArray observers(mEditorObservers);
+        for (auto& observer : observers) {
+          observer->CancelEditAction();
+        }
       }
       break;
     default:
       MOZ_CRASH("Handle all notifications here");
       break;
   }
 }
 
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -71,16 +71,17 @@ class InsertNodeTransaction;
 class InsertTextTransaction;
 class JoinNodeTransaction;
 class PlaceholderTransaction;
 class RemoveStyleSheetTransaction;
 class SplitNodeResult;
 class SplitNodeTransaction;
 class TextComposition;
 class TextEditor;
+class TextInputListener;
 class TextServicesDocument;
 enum class EditAction : int32_t;
 
 namespace dom {
 class DataTransfer;
 class Element;
 class EventTarget;
 class Text;
@@ -247,16 +248,22 @@ public:
     eNotifyEditorObserversOfBefore,
     eNotifyEditorObserversOfCancel
   };
   void NotifyEditorObservers(NotificationForEditorObservers aNotification);
 
   // nsIEditor methods
   NS_DECL_NSIEDITOR
 
+  /**
+   * Set or unset TextInputListener.  If setting non-nullptr when the editor
+   * already has a TextInputListener, this will crash in debug build.
+   */
+  void SetTextInputListener(TextInputListener* aTextInputListener);
+
 public:
   virtual bool IsModifiableNode(nsINode* aNode);
 
   /**
    * InsertTextImpl() inserts aStringToInsert to aPointToInsert or better
    * insertion point around it.  If aPointToInsert isn't in a text node,
    * this method looks for the nearest point in a text node with
    * FindBetterInsertionPoint().  If there is no text node, this creates
@@ -1402,16 +1409,18 @@ protected:
   // Saved selection state for placeholder transaction batching.
   mozilla::Maybe<SelectionState> mSelState;
   // IME composition this is not null between compositionstart and
   // compositionend.
   RefPtr<TextComposition> mComposition;
 
   RefPtr<TextEditRules> mRules;
 
+  RefPtr<TextInputListener> mTextInputListener;
+
   // Listens to all low level actions on the doc.
   typedef AutoTArray<OwningNonNull<nsIEditActionListener>, 5>
             AutoActionListenerArray;
   AutoActionListenerArray mActionListeners;
   // Just notify once per high level change.
   typedef AutoTArray<OwningNonNull<nsIEditorObserver>, 3>
             AutoEditorObserverArray;
   AutoEditorObserverArray mEditorObservers;