Bug 1432528 - part 2: Make EditorBase treat TextInputListener directly and make TextInputListener not derived from nsIEditorObserver r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 24 Jan 2018 12:50:01 +0900
changeset 724113 9762d3eb6c272b3166db84162f6c4ce6039e14cd
parent 724112 72f24b340408722bc80ce7ae4893b37ac529b2e6
child 724114 411355d08013bd30d27d075746eb5a1099cc1a16
push id96650
push usermasayuki@d-toybox.com
push dateWed, 24 Jan 2018 14:40:19 +0000
reviewersm_kato
bugs1432528
milestone60.0a1
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;