Bug 1432528 - part 3: Make EditorBase store IMEContentObserver directly and make it not derived from nsIEditorObserver r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 24 Jan 2018 14:10:18 +0900
changeset 724114 411355d08013bd30d27d075746eb5a1099cc1a16
parent 724113 9762d3eb6c272b3166db84162f6c4ce6039e14cd
child 724115 b65e83320f66c266109145c41393c961fc49134a
push id96650
push usermasayuki@d-toybox.com
push dateWed, 24 Jan 2018 14:40:19 +0000
reviewersm_kato
bugs1432528
milestone60.0a1
Bug 1432528 - part 3: Make EditorBase store IMEContentObserver directly and make it not derived from nsIEditorObserver r?m_kato Similar to TextInputListener, EditorBase should store IMEContentObserver directly instead of via nsIEditorObserver. Then, EditorBase::NotifyEditorObservers() can call each method directly. Additionally, we can make IMEContentObserver not derived from nsIEditorObserver. MozReview-Commit-ID: cNKWJe5eUC
dom/events/IMEContentObserver.cpp
dom/events/IMEContentObserver.h
dom/events/moz.build
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -32,16 +32,19 @@
 #include "nsIWidget.h"
 #include "nsPresContext.h"
 #include "nsRefreshDriver.h"
 #include "nsWeakReference.h"
 #include "WritingModes.h"
 
 namespace mozilla {
 
+typedef ContentEventHandler::NodePosition NodePosition;
+typedef ContentEventHandler::NodePositionBefore NodePositionBefore;
+
 using namespace widget;
 
 LazyLogModule sIMECOLog("IMEContentObserver");
 
 static const char*
 ToChar(bool aBool)
 {
   return aBool ? "true" : "false";
@@ -183,17 +186,16 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IMEContentObserver)
  NS_INTERFACE_MAP_ENTRY(nsISelectionListener)
  NS_INTERFACE_MAP_ENTRY(nsIMutationObserver)
  NS_INTERFACE_MAP_ENTRY(nsIReflowObserver)
  NS_INTERFACE_MAP_ENTRY(nsIScrollObserver)
  NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
- NS_INTERFACE_MAP_ENTRY(nsIEditorObserver)
  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsISelectionListener)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(IMEContentObserver)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(IMEContentObserver)
 
 IMEContentObserver::IMEContentObserver()
   : mESM(nullptr)
@@ -465,17 +467,17 @@ IMEContentObserver::ObserveEditableNode(
     MOZ_ASSERT(!mWidget || mNeedsToNotifyIMEOfFocusSet ||
                mSendingNotification == NOTIFY_IME_OF_FOCUS,
                "Wow, OnIMEReceivedFocus() won't be called?");
     return;
   }
 
   mIsObserving = true;
   if (mEditorBase) {
-    mEditorBase->AddEditorObserver(this);
+    mEditorBase->SetIMEContentObserver(this);
   }
 
   if (!WasInitializedWithPlugin()) {
     // Add selection change listener only when this starts to observe
     // non-plugin content since we cannot detect selection changes in
     // plugins.
     nsresult rv = mSelection->AddSelectionListener(this);
     NS_ENSURE_SUCCESS_VOID(rv);
@@ -542,17 +544,17 @@ void
 IMEContentObserver::UnregisterObservers()
 {
   if (!mIsObserving) {
     return;
   }
   mIsObserving = false;
 
   if (mEditorBase) {
-    mEditorBase->RemoveEditorObserver(this);
+    mEditorBase->SetIMEContentObserver(nullptr);
   }
 
   if (mSelection) {
     mSelection->RemoveSelectionListener(this);
     mSelectionData.Clear();
     mFocusedWidget = nullptr;
   }
 
@@ -1401,49 +1403,46 @@ IMEContentObserver::UnsuppressNotifyingI
      "mSuppressNotifications=%u", this, mSuppressNotifications));
 
   if (!mSuppressNotifications || --mSuppressNotifications) {
     return;
   }
   FlushMergeableNotifications();
 }
 
-NS_IMETHODIMP
-IMEContentObserver::EditAction()
+void
+IMEContentObserver::OnEditActionHandled()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("0x%p IMEContentObserver::EditAction()", this));
 
   mEndOfAddedTextCache.Clear();
   mStartOfRemovingTextRangeCache.Clear();
   FlushMergeableNotifications();
-  return NS_OK;
 }
 
-NS_IMETHODIMP
+void
 IMEContentObserver::BeforeEditAction()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("0x%p IMEContentObserver::BeforeEditAction()", this));
 
   mEndOfAddedTextCache.Clear();
   mStartOfRemovingTextRangeCache.Clear();
-  return NS_OK;
 }
 
-NS_IMETHODIMP
+void
 IMEContentObserver::CancelEditAction()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("0x%p IMEContentObserver::CancelEditAction()", this));
 
   mEndOfAddedTextCache.Clear();
   mStartOfRemovingTextRangeCache.Clear();
   FlushMergeableNotifications();
-  return NS_OK;
 }
 
 void
 IMEContentObserver::PostFocusSetNotification()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("0x%p IMEContentObserver::PostFocusSetNotification()", this));
 
--- a/dom/events/IMEContentObserver.h
+++ b/dom/events/IMEContentObserver.h
@@ -1,24 +1,23 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* 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_IMEContentObserver_h_
-#define mozilla_IMEContentObserver_h_
+#ifndef mozilla_IMEContentObserver_h
+#define mozilla_IMEContentObserver_h
 
 #include "mozilla/Attributes.h"
 #include "mozilla/EditorBase.h"
 #include "mozilla/dom/Selection.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsIDocShell.h" // XXX Why does only this need to be included here?
-#include "nsIEditorObserver.h"
 #include "nsIReflowObserver.h"
 #include "nsISelectionListener.h"
 #include "nsIScrollObserver.h"
 #include "nsIWidget.h"
 #include "nsStubDocumentObserver.h"
 #include "nsStubMutationObserver.h"
 #include "nsThreadUtils.h"
 #include "nsWeakReference.h"
@@ -35,33 +34,29 @@ class TextComposition;
 
 // IMEContentObserver notifies widget of any text and selection changes
 // in the currently focused editor
 class IMEContentObserver final : public nsISelectionListener
                                , public nsStubMutationObserver
                                , public nsIReflowObserver
                                , public nsIScrollObserver
                                , public nsSupportsWeakReference
-                               , public nsIEditorObserver
 {
 public:
-  typedef ContentEventHandler::NodePosition NodePosition;
-  typedef ContentEventHandler::NodePositionBefore NodePositionBefore;
   typedef widget::IMENotification::SelectionChangeData SelectionChangeData;
   typedef widget::IMENotification::TextChangeData TextChangeData;
   typedef widget::IMENotification::TextChangeDataBase TextChangeDataBase;
   typedef widget::IMENotificationRequests IMENotificationRequests;
   typedef widget::IMEMessage IMEMessage;
 
   IMEContentObserver();
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(IMEContentObserver,
                                            nsISelectionListener)
-  NS_DECL_NSIEDITOROBSERVER
   NS_DECL_NSISELECTIONLISTENER
   NS_DECL_NSIMUTATIONOBSERVER_CHARACTERDATAWILLCHANGE
   NS_DECL_NSIMUTATIONOBSERVER_CHARACTERDATACHANGED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
   NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTEWILLCHANGE
   NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
@@ -161,16 +156,26 @@ public:
   void TryToFlushPendingNotifications(bool aAllowAsync);
 
   /**
    * MaybeNotifyCompositionEventHandled() posts composition event handled
    * notification into the pseudo queue.
    */
   void MaybeNotifyCompositionEventHandled();
 
+  /**
+   * Following methods are called when the editor:
+   *   - an edit action handled.
+   *   - before handling an edit action.
+   *   - canceled handling an edit action after calling BeforeEditAction().
+   */
+  void OnEditActionHandled();
+  void BeforeEditAction();
+  void CancelEditAction();
+
 private:
   ~IMEContentObserver() {}
 
   enum State {
     eState_NotObserving,
     eState_Initializing,
     eState_StoppedObserving,
     eState_Observing
@@ -524,9 +529,9 @@ private:
   bool mNeedsToNotifyIMEOfCompositionEventHandled;
   // mIsHandlingQueryContentEvent is true when IMEContentObserver is handling
   // WidgetQueryContentEvent with ContentEventHandler.
   bool mIsHandlingQueryContentEvent;
 };
 
 } // namespace mozilla
 
-#endif // mozilla_IMEContentObserver_h_
+#endif // mozilla_IMEContentObserver_h
--- a/dom/events/moz.build
+++ b/dom/events/moz.build
@@ -24,16 +24,17 @@ XPIDL_MODULE = 'content_events'
 EXPORTS.mozilla += [
     'AsyncEventDispatcher.h',
     'DOMEventTargetHelper.h',
     'EventDispatcher.h',
     'EventListenerManager.h',
     'EventNameList.h',
     'EventStateManager.h',
     'EventStates.h',
+    'IMEContentObserver.h',
     'IMEStateManager.h',
     'InternalMutationEvent.h',
     'JSEventHandler.h',
     'KeyNameList.h',
     'PhysicalKeyCodeNameList.h',
     'TextComposition.h',
     'VirtualKeyCodeList.h',
     'WheelHandlingHelper.h',
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -29,16 +29,17 @@
 #include "TextEditUtils.h"              // for TextEditUtils
 #include "mozilla/CheckedInt.h"         // for CheckedInt
 #include "mozilla/EditAction.h"         // for EditAction
 #include "mozilla/EditorDOMPoint.h"     // for EditorDOMPoint
 #include "mozilla/EditorSpellCheck.h"   // for EditorSpellCheck
 #include "mozilla/EditorUtils.h"        // for AutoRules, etc.
 #include "mozilla/EditTransactionBase.h" // for EditTransactionBase
 #include "mozilla/FlushType.h"          // for FlushType::Frames
+#include "mozilla/IMEContentObserver.h" // for IMEContentObserver
 #include "mozilla/IMEStateManager.h"    // for IMEStateManager
 #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
@@ -163,16 +164,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(mIMEContentObserver)
  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)
@@ -187,16 +189,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(
    tmp->mRootElement ? tmp->mRootElement->GetUncomposedDoc() : nullptr;
  if (currentDoc &&
      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(mIMEContentObserver)
  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)
@@ -350,16 +353,24 @@ void
 EditorBase::SetTextInputListener(TextInputListener* aTextInputListener)
 {
   MOZ_ASSERT(!mTextInputListener || !aTextInputListener ||
              mTextInputListener == aTextInputListener);
   mTextInputListener = aTextInputListener;
 }
 
 void
+EditorBase::SetIMEContentObserver(IMEContentObserver* aIMEContentObserver)
+{
+  MOZ_ASSERT(!mIMEContentObserver || !aIMEContentObserver ||
+             mIMEContentObserver == aIMEContentObserver);
+  mIMEContentObserver = aIMEContentObserver;
+}
+
+void
 EditorBase::CreateEventListeners()
 {
   // Don't create the handler twice
   if (!mEventListener) {
     mEventListener = new EditorEventListener();
   }
 }
 
@@ -2080,26 +2091,30 @@ EditorBase::AddEditorObserver(nsIEditorO
   // we don't keep ownership of the observers.  They must
   // remove themselves as observers before they are destroyed.
 
   NS_ENSURE_TRUE(aObserver, NS_ERROR_NULL_POINTER);
 
   // Make sure the listener isn't already on the list
   if (!mEditorObservers.Contains(aObserver)) {
     mEditorObservers.AppendElement(*aObserver);
+    NS_WARNING_ASSERTION(mEditorObservers.Length() != 1,
+      "nsIEditorObserver installed, this editor becomes slower");
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::RemoveEditorObserver(nsIEditorObserver* aObserver)
 {
   NS_ENSURE_TRUE(aObserver, NS_ERROR_FAILURE);
 
+  NS_WARNING_ASSERTION(mEditorObservers.Length() != 1,
+    "All nsIEditorObservers have been removed, this editor becomes faster");
   mEditorObservers.RemoveElement(aObserver);
 
   return NS_OK;
 }
 
 class EditorInputEventDispatcher final : public Runnable
 {
 public:
@@ -2157,16 +2172,21 @@ EditorBase::NotifyEditorObservers(Notifi
     case eNotifyEditorObserversOfEnd:
       mIsInEditAction = false;
 
       if (mTextInputListener) {
         RefPtr<TextInputListener> listener = mTextInputListener;
         listener->OnEditActionHandled();
       }
 
+      if (mIMEContentObserver) {
+        RefPtr<IMEContentObserver> observer = mIMEContentObserver;
+        observer->OnEditActionHandled();
+      }
+
       if (!mEditorObservers.IsEmpty()) {
         // Copy the observers since EditAction()s can modify mEditorObservers.
         AutoEditorObserverArray observers(mEditorObservers);
         for (auto& observer : observers) {
           observer->EditAction();
         }
       }
 
@@ -2175,27 +2195,40 @@ EditorBase::NotifyEditorObservers(Notifi
       }
 
       FireInputEvent();
       break;
     case eNotifyEditorObserversOfBefore:
       if (NS_WARN_IF(mIsInEditAction)) {
         break;
       }
+
       mIsInEditAction = true;
+
+      if (mIMEContentObserver) {
+        RefPtr<IMEContentObserver> observer = mIMEContentObserver;
+        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;
+
+      if (mIMEContentObserver) {
+        RefPtr<IMEContentObserver> observer = mIMEContentObserver;
+        observer->CancelEditAction();
+      }
+
       if (!mEditorObservers.IsEmpty()) {
         // Copy the observers since EditAction()s can modify mEditorObservers.
         AutoEditorObserverArray observers(mEditorObservers);
         for (auto& observer : observers) {
           observer->CancelEditAction();
         }
       }
       break;
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -62,16 +62,17 @@ class ChangeAttributeTransaction;
 class CompositionTransaction;
 class CreateElementTransaction;
 class DeleteNodeTransaction;
 class DeleteTextTransaction;
 class EditAggregateTransaction;
 class EditTransactionBase;
 class ErrorResult;
 class HTMLEditor;
+class IMEContentObserver;
 class InsertNodeTransaction;
 class InsertTextTransaction;
 class JoinNodeTransaction;
 class PlaceholderTransaction;
 class RemoveStyleSheetTransaction;
 class SplitNodeResult;
 class SplitNodeTransaction;
 class TextComposition;
@@ -254,16 +255,22 @@ public:
   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);
 
+  /**
+   * Set or unset IMEContentObserver.  If setting non-nullptr when the editor
+   * already has an IMEContentObserver, this will crash in debug build.
+   */
+  void SetIMEContentObserver(IMEContentObserver* aIMEContentObserver);
+
 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
@@ -1411,16 +1418,18 @@ protected:
   // IME composition this is not null between compositionstart and
   // compositionend.
   RefPtr<TextComposition> mComposition;
 
   RefPtr<TextEditRules> mRules;
 
   RefPtr<TextInputListener> mTextInputListener;
 
+  RefPtr<IMEContentObserver> mIMEContentObserver;
+
   // 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;