Bug 1203364 IMEContentObserver should notify IME of selection change with the latest change reason r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 14 Sep 2015 23:28:43 +0900
changeset 294927 fe70add404858840b24f81d522033959e95b5f53
parent 294926 3d25434042f28355c4d8a4b93278c449998cec9d
child 294928 ce840d7632babd00f8a2d8081ac27a06e9c99a51
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1203364
milestone43.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 1203364 IMEContentObserver should notify IME of selection change with the latest change reason r=smaug
dom/events/IMEContentObserver.cpp
dom/events/IMEContentObserver.h
widget/IMEData.h
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -191,18 +191,16 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(IMECont
 IMEContentObserver::IMEContentObserver()
   : mESM(nullptr)
   , mSuppressNotifications(0)
   , mPreCharacterDataChangeLength(-1)
   , mIsObserving(false)
   , mIMEHasFocus(false)
   , mIsFocusEventPending(false)
   , mIsSelectionChangeEventPending(false)
-  , mSelectionChangeCausedOnlyByComposition(false)
-  , mSelectionChangeCausedOnlyBySelectionEvent(false)
   , mIsPositionChangeEventPending(false)
   , mIsFlushingPendingNotifications(false)
 {
 #ifdef DEBUG
   mTextChangeData.Test();
 #endif
   if (!sIMECOLog) {
     sIMECOLog = PR_NewLogModule("IMEContentObserver");
@@ -1019,37 +1017,24 @@ IMEContentObserver::PostTextChangeNotifi
 
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::PostTextChangeNotification(), "
      "mTextChangeData=%s)",
      this, TextChangeDataToString(mTextChangeData).get()));
 }
 
 void
-IMEContentObserver::PostSelectionChangeNotification(
-                      bool aCausedByComposition,
-                      bool aCausedBySelectionEvent)
+IMEContentObserver::PostSelectionChangeNotification()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
-    ("IMECO: 0x%p IMEContentObserver::PostSelectionChangeNotification("
-     "aCausedByComposition=%s, aCausedBySelectionEvent=%s)",
-     this, ToChar(aCausedByComposition), ToChar(aCausedBySelectionEvent)));
+    ("IMECO: 0x%p IMEContentObserver::PostSelectionChangeNotification(), "
+     "mSelectionData={ mCausedByComposition=%s, mCausedBySelectionEvent=%s }",
+     this, ToChar(mSelectionData.mCausedByComposition),
+     ToChar(mSelectionData.mCausedBySelectionEvent)));
 
-  if (!mIsSelectionChangeEventPending) {
-    mSelectionChangeCausedOnlyByComposition = aCausedByComposition;
-  } else {
-    mSelectionChangeCausedOnlyByComposition =
-      mSelectionChangeCausedOnlyByComposition && aCausedByComposition;
-  }
-  if (!mSelectionChangeCausedOnlyBySelectionEvent) {
-    mSelectionChangeCausedOnlyBySelectionEvent = aCausedBySelectionEvent;
-  } else {
-    mSelectionChangeCausedOnlyBySelectionEvent =
-      mSelectionChangeCausedOnlyBySelectionEvent && aCausedBySelectionEvent;
-  }
   mIsSelectionChangeEventPending = true;
 }
 
 void
 IMEContentObserver::MaybeNotifyIMEOfFocusSet()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::MaybeNotifyIMEOfFocusSet()", this));
@@ -1076,18 +1061,19 @@ IMEContentObserver::MaybeNotifyIMEOfSele
                       bool aCausedByComposition,
                       bool aCausedBySelectionEvent)
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::MaybeNotifyIMEOfSelectionChange("
      "aCausedByComposition=%s, aCausedBySelectionEvent=%s)",
      this, ToChar(aCausedByComposition), ToChar(aCausedBySelectionEvent)));
 
-  PostSelectionChangeNotification(aCausedByComposition,
-                                  aCausedBySelectionEvent);
+  mSelectionData.AssignReason(aCausedByComposition,
+                              aCausedBySelectionEvent);
+  PostSelectionChangeNotification();
   FlushMergeableNotifications();
 }
 
 void
 IMEContentObserver::MaybeNotifyIMEOfPositionChange()
 {
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::MaybeNotifyIMEOfPositionChange()", this));
@@ -1099,34 +1085,34 @@ bool
 IMEContentObserver::UpdateSelectionCache()
 {
   MOZ_ASSERT(IsSafeToNotifyIME());
 
   if (!mUpdatePreference.WantSelectionChange()) {
     return false;
   }
 
-  mSelectionData.Clear();
+  mSelectionData.ClearSelectionData();
 
   // XXX Cannot we cache some information for reducing the cost to compute
   //     selection offset and writing mode?
   WidgetQueryContentEvent selection(true, eQuerySelectedText, mWidget);
   ContentEventHandler handler(GetPresContext());
   handler.OnQuerySelectedText(&selection);
   if (NS_WARN_IF(!selection.mSucceeded)) {
     return false;
   }
 
   mFocusedWidget = selection.mReply.mFocusedWidget;
   mSelectionData.mOffset = selection.mReply.mOffset;
   *mSelectionData.mString = selection.mReply.mString;
   mSelectionData.SetWritingMode(selection.GetWritingMode());
   mSelectionData.mReversed = selection.mReply.mReversed;
-  mSelectionData.mCausedByComposition = false;
-  mSelectionData.mCausedBySelectionEvent = false;
+
+  // WARNING: Don't modify the reason of selection change here.
 
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::UpdateSelectionCache(), "
      "mSelectionData=%s",
      this, SelectionChangeDataToString(mSelectionData).get()));
 
   return mSelectionData.IsValid();
 }
@@ -1243,19 +1229,17 @@ IMEContentObserver::FlushMergeableNotifi
   // Be aware, PuppetWidget depends on the order of this. A selection change
   // notification should not be sent before a text change notification because
   // PuppetWidget shouldn't query new text content every selection change.
   if (mIsSelectionChangeEventPending) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::FlushMergeableNotifications(), "
        "creating SelectionChangeEvent...", this));
     mIsSelectionChangeEventPending = false;
-    nsContentUtils::AddScriptRunner(
-      new SelectionChangeEvent(this, mSelectionChangeCausedOnlyByComposition,
-                               mSelectionChangeCausedOnlyBySelectionEvent));
+    nsContentUtils::AddScriptRunner(new SelectionChangeEvent(this));
   }
 
   if (mIsPositionChangeEventPending) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::FlushMergeableNotifications(), "
        "creating PositionChangeEvent...", this));
     mIsPositionChangeEventPending = false;
     nsContentUtils::AddScriptRunner(new PositionChangeEvent(this));
@@ -1380,48 +1364,47 @@ IMEContentObserver::SelectionChangeEvent
        "due to impossible to notify IME of selection change", this));
     return NS_OK;
   }
 
   if (!IsSafeToNotifyIME()) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p   IMEContentObserver::SelectionChangeEvent::Run(), "
        "retrying to send NOTIFY_IME_OF_SELECTION_CHANGE...", this));
-    mIMEContentObserver->PostSelectionChangeNotification(
-                           mCausedByComposition, mCausedBySelectionEvent);
+    mIMEContentObserver->PostSelectionChangeNotification();
     return NS_OK;
   }
 
   SelectionChangeData lastSelChangeData = mIMEContentObserver->mSelectionData;
   if (NS_WARN_IF(!mIMEContentObserver->UpdateSelectionCache())) {
     MOZ_LOG(sIMECOLog, LogLevel::Error,
       ("IMECO: 0x%p IMEContentObserver::SelectionChangeEvent::Run(), FAILED, "
        "due to UpdateSelectionCache() failure", this));
     return NS_OK;
   }
 
   // If the IME doesn't want selection change notifications caused by
   // composition, we should do nothing anymore.
-  if (mCausedByComposition &&
+  SelectionChangeData& newSelChangeData = mIMEContentObserver->mSelectionData;
+  if (newSelChangeData.mCausedByComposition &&
       !mIMEContentObserver->
         mUpdatePreference.WantChangesCausedByComposition()) {
     return NS_OK;
   }
 
   // The state may be changed since querying content causes flushing layout.
   if (!CanNotifyIME()) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::SelectionChangeEvent::Run(), FAILED, "
        "due to flushing layout having changed something", this));
     return NS_OK;
   }
 
   // If the selection isn't changed actually, we shouldn't notify IME of
   // selection change.
-  SelectionChangeData& newSelChangeData = mIMEContentObserver->mSelectionData;
   if (lastSelChangeData.IsValid() &&
       lastSelChangeData.mOffset == newSelChangeData.mOffset &&
       lastSelChangeData.String() == newSelChangeData.String() &&
       lastSelChangeData.GetWritingMode() == newSelChangeData.GetWritingMode() &&
       lastSelChangeData.mReversed == newSelChangeData.mReversed) {
     MOZ_LOG(sIMECOLog, LogLevel::Debug,
       ("IMECO: 0x%p IMEContentObserver::SelectionChangeEvent::Run(), not "
        "notifying IME of NOTIFY_IME_OF_SELECTION_CHANGE due to not changed "
@@ -1430,18 +1413,17 @@ IMEContentObserver::SelectionChangeEvent
   }
 
   MOZ_LOG(sIMECOLog, LogLevel::Info,
     ("IMECO: 0x%p IMEContentObserver::SelectionChangeEvent::Run(), "
      "sending NOTIFY_IME_OF_SELECTION_CHANGE... newSelChangeData=%s",
      this, SelectionChangeDataToString(newSelChangeData).get()));
 
   IMENotification notification(NOTIFY_IME_OF_SELECTION_CHANGE);
-  notification.SetData(mIMEContentObserver->mSelectionData,
-                       mCausedByComposition, mCausedBySelectionEvent);
+  notification.SetData(mIMEContentObserver->mSelectionData);
   IMEStateManager::NotifyIME(notification, mIMEContentObserver->mWidget);
 
   MOZ_LOG(sIMECOLog, LogLevel::Debug,
     ("IMECO: 0x%p IMEContentObserver::SelectionChangeEvent::Run(), "
      "sent NOTIFY_IME_OF_SELECTION_CHANGE", this));
   return NS_OK;
 }
 
--- a/dom/events/IMEContentObserver.h
+++ b/dom/events/IMEContentObserver.h
@@ -116,18 +116,17 @@ private:
                           nsIContent* aContent) const;
   bool IsReflowLocked() const;
   bool IsSafeToNotifyIME() const;
 
   void PostFocusSetNotification();
   void MaybeNotifyIMEOfFocusSet();
   void PostTextChangeNotification(const TextChangeDataBase& aTextChangeData);
   void MaybeNotifyIMEOfTextChange(const TextChangeDataBase& aTextChangeData);
-  void PostSelectionChangeNotification(bool aCausedByComposition,
-                                       bool aCausedBySelectionEvent);
+  void PostSelectionChangeNotification();
   void MaybeNotifyIMEOfSelectionChange(bool aCausedByComposition,
                                        bool aCausedBySelectionEvent);
   void PostPositionChangeNotification();
   void MaybeNotifyIMEOfPositionChange();
 
   void NotifyContentAdded(nsINode* aContainer, int32_t aStart, int32_t aEnd);
   void ObserveEditableNode();
   /**
@@ -218,34 +217,32 @@ private:
   FlatTextCache mEndOfAddedTextCache;
   // mStartOfRemovingTextRangeCache caches text length from the start of content
   // to the start of the last removed content only while an edit action is being
   // handled by the editor and no other mutation (e.g., adding node) occur.
   FlatTextCache mStartOfRemovingTextRangeCache;
 
   TextChangeData mTextChangeData;
 
-  // mSelectionData is the last selection data which was notified.  This is
-  // modified by UpdateSelectionCache().  Note that mCausedBy* are always
-  // false.  Do NOT refer them.
+  // mSelectionData is the last selection data which was notified.  The
+  // selection information is modified by UpdateSelectionCache().  The reason
+  // of the selection change is modified by MaybeNotifyIMEOfSelectionChange().
   SelectionChangeData mSelectionData;
 
   EventStateManager* mESM;
 
   nsIMEUpdatePreference mUpdatePreference;
   uint32_t mPreAttrChangeLength;
   uint32_t mSuppressNotifications;
   int64_t mPreCharacterDataChangeLength;
 
   bool mIsObserving;
   bool mIMEHasFocus;
   bool mIsFocusEventPending;
   bool mIsSelectionChangeEventPending;
-  bool mSelectionChangeCausedOnlyByComposition;
-  bool mSelectionChangeCausedOnlyBySelectionEvent;
   bool mIsPositionChangeEventPending;
   bool mIsFlushingPendingNotifications;
 
 
   /**
    * Helper classes to notify IME.
    */
 
@@ -291,31 +288,21 @@ private:
     {
     }
     NS_IMETHOD Run() override;
   };
 
   class SelectionChangeEvent : public AChangeEvent
   {
   public:
-    SelectionChangeEvent(IMEContentObserver* aIMEContentObserver,
-                         bool aCausedByComposition,
-                         bool aCausedBySelectionEvent)
+    explicit SelectionChangeEvent(IMEContentObserver* aIMEContentObserver)
       : AChangeEvent(eChangeEventType_Selection, aIMEContentObserver)
-      , mCausedByComposition(aCausedByComposition)
-      , mCausedBySelectionEvent(aCausedBySelectionEvent)
     {
-      aIMEContentObserver->mSelectionChangeCausedOnlyByComposition = false;
-      aIMEContentObserver->mSelectionChangeCausedOnlyBySelectionEvent = false;
     }
     NS_IMETHOD Run() override;
-
-  private:
-    bool mCausedByComposition;
-    bool mCausedBySelectionEvent;
   };
 
   class TextChangeEvent : public AChangeEvent
   {
   public:
     TextChangeEvent(IMEContentObserver* aIMEContentObserver,
                     TextChangeDataBase& aTextChangeData)
       : AChangeEvent(eChangeEventType_Text, aIMEContentObserver)
--- a/widget/IMEData.h
+++ b/widget/IMEData.h
@@ -577,37 +577,47 @@ struct IMENotification final
     bool IsInInt32Range() const
     {
       return mOffset + Length() <= INT32_MAX;
     }
     bool IsCollapsed() const
     {
       return mString->IsEmpty();
     }
-    void Clear()
+    void ClearSelectionData()
     {
       mOffset = UINT32_MAX;
       mString->Truncate();
       mWritingMode = 0;
       mReversed = false;
+    }
+    void Clear()
+    {
+      ClearSelectionData();
       mCausedByComposition = false;
       mCausedBySelectionEvent = false;
     }
     bool IsValid() const
     {
       return mOffset != UINT32_MAX;
     }
     void Assign(const SelectionChangeDataBase& aOther)
     {
       mOffset = aOther.mOffset;
       *mString = aOther.String();
       mWritingMode = aOther.mWritingMode;
       mReversed = aOther.mReversed;
-      mCausedByComposition = aOther.mCausedByComposition;
-      mCausedBySelectionEvent = aOther.mCausedBySelectionEvent;
+      AssignReason(aOther.mCausedByComposition,
+                   aOther.mCausedBySelectionEvent);
+    }
+    void AssignReason(bool aCausedByComposition,
+                      bool aCausedBySelectionEvent)
+    {
+      mCausedByComposition = aCausedByComposition;
+      mCausedBySelectionEvent = aCausedBySelectionEvent;
     }
   };
 
   // SelectionChangeDataBase cannot have constructors because it's used in
   // the union.  Therefore, SelectionChangeData should only implement
   // constructors.  In other words, add other members to
   // SelectionChangeDataBase.
   struct SelectionChangeData final : public SelectionChangeDataBase
@@ -762,25 +772,16 @@ struct IMENotification final
     MouseButtonEventData mMouseButtonEventData;
   };
 
   void SetData(const SelectionChangeDataBase& aSelectionChangeData)
   {
     MOZ_RELEASE_ASSERT(mMessage == NOTIFY_IME_OF_SELECTION_CHANGE);
     mSelectionChangeData.Assign(aSelectionChangeData);
   }
-  void SetData(const SelectionChangeDataBase& aSelectionChangeData,
-               bool aCausedByComposition,
-               bool aCausedBySelectionEvent)
-  {
-    MOZ_RELEASE_ASSERT(mMessage == NOTIFY_IME_OF_SELECTION_CHANGE);
-    mSelectionChangeData.Assign(aSelectionChangeData);
-    mSelectionChangeData.mCausedByComposition = aCausedByComposition;
-    mSelectionChangeData.mCausedBySelectionEvent = aCausedBySelectionEvent;
-  }
 
   void SetData(const TextChangeDataBase& aTextChangeData)
   {
     MOZ_RELEASE_ASSERT(mMessage == NOTIFY_IME_OF_TEXT_CHANGE);
     mTextChangeData = aTextChangeData;
   }
 
   bool IsCausedByComposition() const