Bug 1184449 part.1 IMENotifiation::SelectionChangeData should store selected string r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 22 Jul 2015 12:40:32 +0900
changeset 285710 56319240dff61410b6dffc7ae80ba2cd41f9e1eb
parent 285709 621c0ceeea21d63525ce92b9dbeb1ea38429408d
child 285711 4f684751728862e83dff92bd4eee5282f9a3f78e
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1184449
milestone42.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 1184449 part.1 IMENotifiation::SelectionChangeData should store selected string r=smaug
dom/events/IMEContentObserver.cpp
widget/PuppetWidget.cpp
widget/gtk/nsGtkIMModule.cpp
widget/nsGUIEventIPC.h
widget/nsIWidget.h
widget/windows/nsTextStore.cpp
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -1069,20 +1069,18 @@ IMEContentObserver::SelectionChangeEvent
   }
 
   // The state may be changed since querying content causes flushing layout.
   if (!CanNotifyIME()) {
     return NS_OK;
   }
 
   IMENotification notification(NOTIFY_IME_OF_SELECTION_CHANGE);
-  notification.mSelectionChangeData.mOffset =
-    selection.mReply.mOffset;
-  notification.mSelectionChangeData.mLength =
-    selection.mReply.mString.Length();
+  notification.mSelectionChangeData.mOffset = selection.mReply.mOffset;
+  *notification.mSelectionChangeData.mString = selection.mReply.mString;
   notification.mSelectionChangeData.SetWritingMode(
                                       selection.GetWritingMode());
   notification.mSelectionChangeData.mReversed = selection.mReply.mReversed;
   notification.mSelectionChangeData.mCausedByComposition =
     mCausedByComposition;
   notification.mSelectionChangeData.mCausedBySelectionEvent =
     mCausedBySelectionEvent;
   IMEStateManager::NotifyIME(notification, mIMEContentObserver->mWidget);
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -790,17 +790,17 @@ PuppetWidget::NotifyIMEOfSelectionChange
   if (!mTabChild)
     return NS_ERROR_FAILURE;
 
   // Note that selection change must be notified after text change if it occurs.
   // Therefore, we don't need to query text content again here.
   mContentCache.SetSelection(
     this, 
     aIMENotification.mSelectionChangeData.mOffset,
-    aIMENotification.mSelectionChangeData.mLength,
+    aIMENotification.mSelectionChangeData.Length(),
     aIMENotification.mSelectionChangeData.mReversed,
     aIMENotification.mSelectionChangeData.GetWritingMode());
 
   mTabChild->SendNotifyIMESelection(mContentCache, aIMENotification);
   return NS_OK;
 }
 
 nsresult
--- a/widget/gtk/nsGtkIMModule.cpp
+++ b/widget/gtk/nsGtkIMModule.cpp
@@ -783,21 +783,21 @@ nsGtkIMModule::OnSelectionChange(nsWindo
         return;
     }
 
     const IMENotification::SelectionChangeData& selectionChangeData =
         aIMENotification.mSelectionChangeData;
 
     MOZ_LOG(gGtkIMLog, LogLevel::Info,
         ("GtkIMModule(%p): OnSelectionChange(aCaller=0x%p, aIMENotification={ "
-         "mSelectionChangeData={ mOffset=%u, mLength=%u, mReversed=%s, "
+         "mSelectionChangeData={ mOffset=%u, Length()=%u, mReversed=%s, "
          "mWritingMode=%s, mCausedByComposition=%s, mCausedBySelectionEvent=%s "
          "} }), mCompositionState=%s, mIsDeletingSurrounding=%s",
          this, aCaller, selectionChangeData.mOffset,
-         selectionChangeData.mLength,
+         selectionChangeData.Length(),
          GetBoolName(selectionChangeData.mReversed),
          GetWritingModeName(selectionChangeData.GetWritingMode()).get(),
          GetBoolName(selectionChangeData.mCausedByComposition),
          GetBoolName(selectionChangeData.mCausedBySelectionEvent),
          GetCompositionStateName(), GetBoolName(mIsDeletingSurrounding)));
 
     if (aCaller != mLastFocusedWindow) {
         MOZ_LOG(gGtkIMLog, LogLevel::Info,
@@ -1849,17 +1849,17 @@ nsGtkIMModule::EnsureToCacheSelection(ns
  * nsGtkIMModule::Selection
  ******************************************************************************/
 
 void
 nsGtkIMModule::Selection::Assign(const IMENotification& aIMENotification)
 {
     MOZ_ASSERT(aIMENotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE);
     mOffset = aIMENotification.mSelectionChangeData.mOffset;
-    mLength = aIMENotification.mSelectionChangeData.mLength;
+    mLength = aIMENotification.mSelectionChangeData.Length();
     mWritingMode = aIMENotification.mSelectionChangeData.GetWritingMode();
 }
 
 void
 nsGtkIMModule::Selection::Assign(const WidgetQueryContentEvent& aEvent)
 {
     MOZ_ASSERT(aEvent.message == NS_QUERY_SELECTED_TEXT);
     MOZ_ASSERT(aEvent.mSucceeded);
--- a/widget/nsGUIEventIPC.h
+++ b/widget/nsGUIEventIPC.h
@@ -702,28 +702,30 @@ struct ParamTraits<mozilla::widget::IMEN
 
 template<>
 struct ParamTraits<mozilla::widget::IMENotification::SelectionChangeData>
 {
   typedef mozilla::widget::IMENotification::SelectionChangeData paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
+    MOZ_RELEASE_ASSERT(aParam.mString);
     WriteParam(aMsg, aParam.mOffset);
-    WriteParam(aMsg, aParam.mLength);
+    WriteParam(aMsg, *aParam.mString);
     WriteParam(aMsg, aParam.mWritingMode);
     WriteParam(aMsg, aParam.mReversed);
     WriteParam(aMsg, aParam.mCausedByComposition);
     WriteParam(aMsg, aParam.mCausedBySelectionEvent);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
+    aResult->mString = new nsString();
     return ReadParam(aMsg, aIter, &aResult->mOffset) &&
-           ReadParam(aMsg, aIter, &aResult->mLength) &&
+           ReadParam(aMsg, aIter, aResult->mString) &&
            ReadParam(aMsg, aIter, &aResult->mWritingMode) &&
            ReadParam(aMsg, aIter, &aResult->mReversed) &&
            ReadParam(aMsg, aIter, &aResult->mCausedByComposition) &&
            ReadParam(aMsg, aIter, &aResult->mCausedBySelectionEvent);
   }
 };
 
 template<>
--- a/widget/nsIWidget.h
+++ b/widget/nsIWidget.h
@@ -594,29 +594,39 @@ enum IMEMessage : IMEMessageType
   // Request to commit current composition to IME
   // (some platforms may not support)
   REQUEST_TO_COMMIT_COMPOSITION,
   // Request to cancel current composition to IME
   // (some platforms may not support)
   REQUEST_TO_CANCEL_COMPOSITION
 };
 
-struct IMENotification
+struct IMENotification final
 {
   IMENotification()
     : mMessage(NOTIFY_IME_OF_NOTHING)
   {}
 
+  IMENotification(const IMENotification& aOther)
+  {
+    Assign(aOther);
+  }
+
+  ~IMENotification()
+  {
+    Clear();
+  }
+
   MOZ_IMPLICIT IMENotification(IMEMessage aMessage)
     : mMessage(aMessage)
   {
     switch (aMessage) {
       case NOTIFY_IME_OF_SELECTION_CHANGE:
         mSelectionChangeData.mOffset = UINT32_MAX;
-        mSelectionChangeData.mLength = 0;
+        mSelectionChangeData.mString = new nsString();
         mSelectionChangeData.mWritingMode = 0;
         mSelectionChangeData.mReversed = false;
         mSelectionChangeData.mCausedByComposition = false;
         mSelectionChangeData.mCausedBySelectionEvent = false;
         break;
       case NOTIFY_IME_OF_TEXT_CHANGE:
         mTextChangeData.Clear();
         break;
@@ -628,39 +638,72 @@ struct IMENotification
         mMouseButtonEventData.mButton = -1;
         mMouseButtonEventData.mButtons = 0;
         mMouseButtonEventData.mModifiers = 0;
       default:
         break;
     }
   }
 
+  void Assign(const IMENotification& aOther)
+  {
+    Clear();
+    mMessage = aOther.mMessage;
+    switch (mMessage) {
+      case NOTIFY_IME_OF_SELECTION_CHANGE:
+        mSelectionChangeData = aOther.mSelectionChangeData;
+        // mString should be different instance because of ownership issue.
+        mSelectionChangeData.mString =
+          new nsString(aOther.mSelectionChangeData.String());
+        break;
+      case NOTIFY_IME_OF_TEXT_CHANGE:
+        mTextChangeData = aOther.mTextChangeData;
+        break;
+      case NOTIFY_IME_OF_MOUSE_BUTTON_EVENT:
+        mMouseButtonEventData = aOther.mMouseButtonEventData;
+        break;
+      default:
+        break;
+    }
+  }
+
+  IMENotification& operator=(const IMENotification& aOther)
+  {
+    Assign(aOther);
+    return *this;
+  }
+
   void Clear()
   {
+    if (mMessage == NOTIFY_IME_OF_SELECTION_CHANGE) {
+      MOZ_ASSERT(mSelectionChangeData.mString);
+      delete mSelectionChangeData.mString;
+      mSelectionChangeData.mString = nullptr;
+    }
     mMessage = NOTIFY_IME_OF_NOTHING;
   }
 
   bool HasNotification() const
   {
     return mMessage != NOTIFY_IME_OF_NOTHING;
   }
 
   void MergeWith(const IMENotification& aNotification)
   {
     switch (mMessage) {
       case NOTIFY_IME_OF_NOTHING:
         MOZ_ASSERT(aNotification.mMessage != NOTIFY_IME_OF_NOTHING);
-        *this = aNotification;
+        Assign(aNotification);
         break;
       case NOTIFY_IME_OF_SELECTION_CHANGE:
         MOZ_ASSERT(aNotification.mMessage == NOTIFY_IME_OF_SELECTION_CHANGE);
         mSelectionChangeData.mOffset =
           aNotification.mSelectionChangeData.mOffset;
-        mSelectionChangeData.mLength =
-          aNotification.mSelectionChangeData.mLength;
+        *mSelectionChangeData.mString =
+          aNotification.mSelectionChangeData.String();
         mSelectionChangeData.mWritingMode =
           aNotification.mSelectionChangeData.mWritingMode;
         mSelectionChangeData.mReversed =
           aNotification.mSelectionChangeData.mReversed;
         if (!mSelectionChangeData.mCausedByComposition) {
           mSelectionChangeData.mCausedByComposition =
             aNotification.mSelectionChangeData.mCausedByComposition;
         } else {
@@ -728,39 +771,49 @@ struct IMENotification
     }
   };
 
   // NOTIFY_IME_OF_SELECTION_CHANGE specific data
   struct SelectionChangeData
   {
     // Selection range.
     uint32_t mOffset;
-    uint32_t mLength;
+
+    // Selected string
+    nsString* mString;
 
     // Writing mode at the selection.
     uint8_t mWritingMode;
 
     bool mReversed;
     bool mCausedByComposition;
     bool mCausedBySelectionEvent;
 
     void SetWritingMode(const WritingMode& aWritingMode);
     WritingMode GetWritingMode() const;
 
     uint32_t StartOffset() const
     {
-      return mOffset + (mReversed ? mLength : 0);
+      return mOffset + (mReversed ? Length() : 0);
     }
     uint32_t EndOffset() const
     {
-      return mOffset + (mReversed ? 0 : mLength);
+      return mOffset + (mReversed ? 0 : Length());
+    }
+    const nsString& String() const
+    {
+      return *mString;
+    }
+    uint32_t Length() const
+    {
+      return mString->Length();
     }
     bool IsInInt32Range() const
     {
-      return mOffset + mLength <= INT32_MAX;
+      return mOffset + Length() <= INT32_MAX;
     }
   };
 
   struct TextChangeDataBase
   {
     // mStartOffset is the start offset of modified or removed text in
     // original content and inserted text in new content.
     uint32_t mStartOffset;
--- a/widget/windows/nsTextStore.cpp
+++ b/widget/windows/nsTextStore.cpp
@@ -4431,21 +4431,22 @@ nsTextStore::NotifyTSFOfTextChange(const
 
 nsresult
 nsTextStore::OnSelectionChangeInternal(const IMENotification& aIMENotification)
 {
   const IMENotification::SelectionChangeData& selectionChangeData =
     aIMENotification.mSelectionChangeData;
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
          ("TSF: 0x%p   nsTextStore::OnSelectionChangeInternal("
-          "aIMENotification={ mSelectionChangeData={ mOffset=%lu, mLength=%lu, "
-          "mReversed=%s, mWritingMode=%s, mCausedByComposition=%s, "
-          "mCausedBySelectionEvent=%s } }), mSink=0x%p, mSinkMask=%s, "
-          "mIsRecordingActionsWithoutLock=%s, mComposition.IsComposing()=%s",
-          this, selectionChangeData.mOffset, selectionChangeData.mLength,
+          "aIMENotification={ mSelectionChangeData={ mOffset=%lu, "
+          "Length()=%lu, mReversed=%s, mWritingMode=%s, "
+          "mCausedByComposition=%s, mCausedBySelectionEvent=%s } }), "
+          "mSink=0x%p, mSinkMask=%s, mIsRecordingActionsWithoutLock=%s, "
+          "mComposition.IsComposing()=%s",
+          this, selectionChangeData.mOffset, selectionChangeData.Length(),
           GetBoolName(selectionChangeData.mReversed),
           GetWritingModeName(selectionChangeData.GetWritingMode()).get(),
           GetBoolName(selectionChangeData.mCausedByComposition),
           GetBoolName(selectionChangeData.mCausedBySelectionEvent),
           mSink.get(), GetSinkMaskNameStr(mSinkMask).get(),
           GetBoolName(mIsRecordingActionsWithoutLock),
           GetBoolName(mComposition.IsComposing())));
 
@@ -4453,17 +4454,17 @@ nsTextStore::OnSelectionChangeInternal(c
 
   if (IsReadLocked()) {
     // XXX Why don't we mark mPendingOnSelectionChange as true here?
     return NS_OK;
   }
 
   mSelection.SetSelection(
     selectionChangeData.mOffset,
-    selectionChangeData.mLength,
+    selectionChangeData.Length(),
     selectionChangeData.mReversed,
     selectionChangeData.GetWritingMode());
 
   if (!selectionChangeData.mCausedBySelectionEvent) {
     // Should be notified via MaybeFlushPendingNotifications() for keeping
     // the order of change notifications.
     mPendingOnSelectionChange = true;
     if (mIsRecordingActionsWithoutLock) {