Bug 599550 - Properly sync PuppetWidget IME notifications; r=blassey,cjones a=blocking-fennec GECKO20b7pre_20100929_RELBRANCH
authorJim Chen <jimnchen@gmail.com>
Fri, 01 Oct 2010 10:17:37 -0400
branchGECKO20b7pre_20100929_RELBRANCH
changeset 54874 e82996480172cb3b7d4adf373a7743518823b4b9
parent 54775 d7f2e1898c4f676e7347ae3acffbdfa2d1aab581
child 54882 4791cfde3ca0fe93f58783ac7769b601fd4798a3
push idunknown
push userunknown
push dateunknown
reviewersblassey, cjones, blocking-fennec
bugs599550
milestone2.0b7pre
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 599550 - Properly sync PuppetWidget IME notifications; r=blassey,cjones a=blocking-fennec
dom/ipc/PBrowser.ipdl
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
widget/public/nsGUIEvent.h
widget/public/nsGUIEventIPC.h
widget/src/xpwidgets/PuppetWidget.cpp
widget/src/xpwidgets/PuppetWidget.h
--- a/dom/ipc/PBrowser.ipdl
+++ b/dom/ipc/PBrowser.ipdl
@@ -87,25 +87,46 @@ parent:
     Event(RemoteDOMEvent aEvent);
 
     rpc CreateWindow() returns (PBrowser window);
 
     sync SyncMessage(nsString aMessage, nsString aJSON)
       returns (nsString[] retval);
 
     /**
+     * The IME sequence number (seqno) parameter is used to make sure
+     * that a notification is discarded if it arrives at the chrome process
+     * too late. If the notification is late and we accept it, we will have
+     * an out-of-date view of the content process, which means events that we
+     * dispatch based on this out-of-date view will be wrong also.
+     * (see Bug 599550 and Bug 591047 comments 44, 50, and 54)
+     *
+     * Chrome increments seqno and includes it in each IME event sent to
+     * content, and content sends its current seqno back to chrome with each
+     * notification. A notification is up-to-date only if the content
+     * seqno is the same as the current chrome seqno, meaning no additional
+     * event was sent to content before the notification was received
+     *
+     * On blur, chrome returns the current seqno to content, and content
+     * uses it to discard subsequent events until the content seqno and
+     * chrome seqno-on-blur match again. These events, meant for the blurred
+     * textfield, are discarded to prevent events going to the wrong target
+     */
+
+    /**
      * Notifies chrome that there is a focus change involving an editable
      * object (input, textarea, document, contentEditable. etc.)
      *
      *  focus        PR_TRUE if editable object is receiving focus
      *               PR_FALSE if losing focus
      *  preference   Native widget preference for IME updates
+     *  seqno        Current seqno value on the chrome side
      */
     sync NotifyIMEFocus(PRBool focus)
-      returns (nsIMEUpdatePreference preference);
+      returns (nsIMEUpdatePreference preference, PRUint32 seqno);
 
     /**
      * Notifies chrome that there has been a change in text content
      * One call can encompass both a delete and an insert operation
      * Only called when NotifyIMEFocus returns PR_TRUE for mWantUpdates
      *
      *  offset       Starting offset of the change
      *  end          Ending offset of the range deleted
@@ -115,20 +136,21 @@ parent:
      *  for deletion, offset == newEnd
      */
     NotifyIMETextChange(PRUint32 offset, PRUint32 end, PRUint32 newEnd);
 
     /**
      * Notifies chrome that there has been a change in selection
      * Only called when NotifyIMEFocus returns PR_TRUE for mWantUpdates
      *
+     *  seqno        Current seqno value on the content side
      *  anchor       Offset where the selection started
      *  focus        Offset where the caret is
      */
-    NotifyIMESelection(PRUint32 anchor, PRUint32 focus);
+    NotifyIMESelection(PRUint32 seqno, PRUint32 anchor, PRUint32 focus);
 
     /**
      * Notifies chrome to refresh its text cache 
      * Only called when NotifyIMEFocus returns PR_TRUE for mWantHints
      *
      *  text         The entire content of the text field
      */
     NotifyIMETextHint(nsString text);
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -312,22 +312,24 @@ bool
 TabParent::RecvAsyncMessage(const nsString& aMessage,
                             const nsString& aJSON)
 {
   return ReceiveMessage(aMessage, PR_FALSE, aJSON, nsnull);
 }
 
 bool
 TabParent::RecvNotifyIMEFocus(const PRBool& aFocus,
-                              nsIMEUpdatePreference* aPreference)
+                              nsIMEUpdatePreference* aPreference,
+                              PRUint32* aSeqno)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget)
     return true;
 
+  *aSeqno = mIMESeqno;
   mIMETabParent = aFocus ? this : nsnull;
   mIMESelectionAnchor = 0;
   mIMESelectionFocus = 0;
   nsresult rv = widget->OnIMEFocusChange(aFocus);
 
   if (aFocus) {
     if (NS_SUCCEEDED(rv) && rv != NS_SUCCESS_IME_NO_UPDATES) {
       *aPreference = widget->GetIMEUpdatePreference();
@@ -350,26 +352,29 @@ TabParent::RecvNotifyIMETextChange(const
   if (!widget)
     return true;
 
   widget->OnIMETextChange(aStart, aEnd, aNewEnd);
   return true;
 }
 
 bool
-TabParent::RecvNotifyIMESelection(const PRUint32& aAnchor,
+TabParent::RecvNotifyIMESelection(const PRUint32& aSeqno,
+                                  const PRUint32& aAnchor,
                                   const PRUint32& aFocus)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
   if (!widget)
     return true;
 
-  mIMESelectionAnchor = aAnchor;
-  mIMESelectionFocus = aFocus;
-  widget->OnIMESelectionChange();
+  if (aSeqno == mIMESeqno) {
+    mIMESelectionAnchor = aAnchor;
+    mIMESelectionFocus = aFocus;
+    widget->OnIMESelectionChange();
+  }
   return true;
 }
 
 bool
 TabParent::RecvNotifyIMETextHint(const nsString& aText)
 {
   // Replace our cache with new text
   mIMECacheText = aText;
@@ -439,56 +444,59 @@ TabParent::HandleQueryContentEvent(nsQue
       aEvent.mSucceeded = PR_TRUE;
     }
     break;
   }
   return true;
 }
 
 bool
-TabParent::SendCompositionEvent(const nsCompositionEvent& event)
+TabParent::SendCompositionEvent(nsCompositionEvent& event)
 {
   mIMEComposing = event.message == NS_COMPOSITION_START;
   mIMECompositionStart = PR_MIN(mIMESelectionAnchor, mIMESelectionFocus);
   if (mIMECompositionEnding)
     return true;
+  event.seqno = ++mIMESeqno;
   return PBrowserParent::SendCompositionEvent(event);
 }
 
 /**
  * During ResetInputState or CancelComposition, widget usually sends a
  * NS_TEXT_TEXT event to finalize or clear the composition, respectively
  *
  * Because the event will not reach content in time, we intercept it
  * here and pass the text as the EndIMEComposition return value
  */
 bool
-TabParent::SendTextEvent(const nsTextEvent& event)
+TabParent::SendTextEvent(nsTextEvent& event)
 {
   if (mIMECompositionEnding) {
     mIMECompositionText = event.theText;
     return true;
   }
 
   // We must be able to simulate the selection because
   // we might not receive selection updates in time
   if (!mIMEComposing) {
     mIMECompositionStart = PR_MIN(mIMESelectionAnchor, mIMESelectionFocus);
   }
   mIMESelectionAnchor = mIMESelectionFocus =
       mIMECompositionStart + event.theText.Length();
 
+  event.seqno = ++mIMESeqno;
   return PBrowserParent::SendTextEvent(event);
 }
 
 bool
-TabParent::SendSelectionEvent(const nsSelectionEvent& event)
+TabParent::SendSelectionEvent(nsSelectionEvent& event)
 {
   mIMESelectionAnchor = event.mOffset + (event.mReversed ? event.mLength : 0);
   mIMESelectionFocus = event.mOffset + (!event.mReversed ? event.mLength : 0);
+  event.seqno = ++mIMESeqno;
   return PBrowserParent::SendSelectionEvent(event);
 }
 
 bool
 TabParent::RecvEndIMEComposition(const PRBool& aCancel,
                                  nsString* aComposition)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -87,21 +87,23 @@ public:
 
     virtual bool AnswerCreateWindow(PBrowserParent** retval);
     virtual bool RecvSyncMessage(const nsString& aMessage,
                                  const nsString& aJSON,
                                  nsTArray<nsString>* aJSONRetVal);
     virtual bool RecvAsyncMessage(const nsString& aMessage,
                                   const nsString& aJSON);
     virtual bool RecvNotifyIMEFocus(const PRBool& aFocus,
-                                    nsIMEUpdatePreference* aPreference);
+                                    nsIMEUpdatePreference* aPreference,
+                                    PRUint32* aSeqno);
     virtual bool RecvNotifyIMETextChange(const PRUint32& aStart,
                                          const PRUint32& aEnd,
                                          const PRUint32& aNewEnd);
-    virtual bool RecvNotifyIMESelection(const PRUint32& aAnchor,
+    virtual bool RecvNotifyIMESelection(const PRUint32& aSeqno,
+                                        const PRUint32& aAnchor,
                                         const PRUint32& aFocus);
     virtual bool RecvNotifyIMETextHint(const nsString& aText);
     virtual bool RecvEndIMEComposition(const PRBool& aCancel,
                                        nsString* aComposition);
     virtual bool RecvGetIMEEnabled(PRUint32* aValue);
     virtual bool RecvSetIMEEnabled(const PRUint32& aValue);
     virtual bool RecvGetIMEOpenState(PRBool* aValue);
     virtual bool RecvSetIMEOpenState(const PRBool& aValue);
@@ -174,19 +176,19 @@ public:
     NS_DECL_NSIAUTHPROMPTPROVIDER
     NS_DECL_NSISECUREBROWSERUI
     NS_DECL_NSISSLSTATUSPROVIDER
 
     void HandleDelayedDialogs();
 
     static TabParent *GetIMETabParent() { return mIMETabParent; }
     bool HandleQueryContentEvent(nsQueryContentEvent& aEvent);
-    bool SendCompositionEvent(const nsCompositionEvent& event);
-    bool SendTextEvent(const nsTextEvent& event);
-    bool SendSelectionEvent(const nsSelectionEvent& event);
+    bool SendCompositionEvent(nsCompositionEvent& event);
+    bool SendTextEvent(nsTextEvent& event);
+    bool SendSelectionEvent(nsSelectionEvent& event);
 protected:
     bool ReceiveMessage(const nsString& aMessage,
                         PRBool aSync,
                         const nsString& aJSON,
                         nsTArray<nsString>* aJSONRetVal = nsnull);
 
     void ActorDestroy(ActorDestroyReason why);
 
@@ -227,16 +229,17 @@ protected:
     PRUint32 mIMESelectionAnchor;
     PRUint32 mIMESelectionFocus;
     PRPackedBool mIMEComposing;
     PRPackedBool mIMECompositionEnding;
     // Buffer to store composition text during ResetInputState
     // Compositions in almost all cases are small enough for nsAutoString
     nsAutoString mIMECompositionText;
     PRUint32 mIMECompositionStart;
+    PRUint32 mIMESeqno;
 
 private:
     already_AddRefed<nsFrameLoader> GetFrameLoader() const;
     already_AddRefed<nsIWidget> GetWidget() const;
 };
 
 } // namespace dom
 } // namespace mozilla
--- a/widget/public/nsGUIEvent.h
+++ b/widget/public/nsGUIEvent.h
@@ -1058,16 +1058,19 @@ class nsTextEvent : public nsInputEvent
 #ifdef MOZ_IPC
 private:
   friend class mozilla::dom::PBrowserParent;
   friend class mozilla::dom::PBrowserChild;
 
   nsTextEvent()
   {
   }
+
+public:
+  PRUint32 seqno;
 #endif // MOZ_IPC
 
 public:
   nsTextEvent(PRBool isTrusted, PRUint32 msg, nsIWidget *w)
     : nsInputEvent(isTrusted, msg, w, NS_TEXT_EVENT),
       rangeCount(0), rangeArray(nsnull), isChar(PR_FALSE)
   {
   }
@@ -1086,16 +1089,19 @@ class nsCompositionEvent : public nsInpu
 #ifdef MOZ_IPC
 private:
   friend class mozilla::dom::PBrowserParent;
   friend class mozilla::dom::PBrowserChild;
 
   nsCompositionEvent()
   {
   }
+
+public:
+  PRUint32 seqno;
 #endif // MOZ_IPC
 
 public:
   nsCompositionEvent(PRBool isTrusted, PRUint32 msg, nsIWidget *w)
     : nsInputEvent(isTrusted, msg, w, NS_COMPOSITION_EVENT)
   {
   }
 };
@@ -1304,16 +1310,19 @@ class nsSelectionEvent : public nsGUIEve
 #ifdef MOZ_IPC
 private:
   friend class mozilla::dom::PBrowserParent;
   friend class mozilla::dom::PBrowserChild;
 
   nsSelectionEvent()
   {
   }
+
+public:
+  PRUint32 seqno;
 #endif // MOZ_IPC
 
 public:
   nsSelectionEvent(PRBool aIsTrusted, PRUint32 aMsg, nsIWidget *aWidget) :
     nsGUIEvent(aIsTrusted, aMsg, aWidget, NS_SELECTION_EVENT),
     mExpandToClusterBoundary(PR_TRUE), mSucceeded(PR_FALSE)
   {
   }
--- a/widget/public/nsGUIEventIPC.h
+++ b/widget/public/nsGUIEventIPC.h
@@ -160,26 +160,28 @@ struct ParamTraits<nsTextRange>
 template<>
 struct ParamTraits<nsTextEvent>
 {
   typedef nsTextEvent paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     WriteParam(aMsg, static_cast<nsInputEvent>(aParam));
+    WriteParam(aMsg, aParam.seqno);
     WriteParam(aMsg, aParam.theText);
     WriteParam(aMsg, aParam.isChar);
     WriteParam(aMsg, aParam.rangeCount);
     for (PRUint32 index = 0; index < aParam.rangeCount; index++)
       WriteParam(aMsg, aParam.rangeArray[index]);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
     if (!ReadParam(aMsg, aIter, static_cast<nsInputEvent*>(aResult)) ||
+        !ReadParam(aMsg, aIter, &aResult->seqno) ||
         !ReadParam(aMsg, aIter, &aResult->theText) ||
         !ReadParam(aMsg, aIter, &aResult->isChar) ||
         !ReadParam(aMsg, aIter, &aResult->rangeCount))
       return false;
 
     if (!aResult->rangeCount) {
       aResult->rangeArray = nsnull;
       return true;
@@ -207,21 +209,23 @@ struct ParamTraits<nsTextEvent>
 template<>
 struct ParamTraits<nsCompositionEvent>
 {
   typedef nsCompositionEvent paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     WriteParam(aMsg, static_cast<nsInputEvent>(aParam));
+    WriteParam(aMsg, aParam.seqno);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
-    return ReadParam(aMsg, aIter, static_cast<nsInputEvent*>(aResult));
+    return ReadParam(aMsg, aIter, static_cast<nsInputEvent*>(aResult)) &&
+           ReadParam(aMsg, aIter, &aResult->seqno);
   }
 };
 
 template<>
 struct ParamTraits<nsQueryContentEvent>
 {
   typedef nsQueryContentEvent paramType;
 
@@ -256,26 +260,28 @@ struct ParamTraits<nsQueryContentEvent>
 template<>
 struct ParamTraits<nsSelectionEvent>
 {
   typedef nsSelectionEvent paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     WriteParam(aMsg, static_cast<nsGUIEvent>(aParam));
+    WriteParam(aMsg, aParam.seqno);
     WriteParam(aMsg, aParam.mOffset);
     WriteParam(aMsg, aParam.mLength);
     WriteParam(aMsg, aParam.mReversed);
     WriteParam(aMsg, aParam.mExpandToClusterBoundary);
     WriteParam(aMsg, aParam.mSucceeded);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
     return ReadParam(aMsg, aIter, static_cast<nsGUIEvent*>(aResult)) &&
+           ReadParam(aMsg, aIter, &aResult->seqno) &&
            ReadParam(aMsg, aIter, &aResult->mOffset) &&
            ReadParam(aMsg, aIter, &aResult->mLength) &&
            ReadParam(aMsg, aIter, &aResult->mReversed) &&
            ReadParam(aMsg, aIter, &aResult->mExpandToClusterBoundary) &&
            ReadParam(aMsg, aIter, &aResult->mSucceeded);
   }
 };
 
--- a/widget/src/xpwidgets/PuppetWidget.cpp
+++ b/widget/src/xpwidgets/PuppetWidget.cpp
@@ -106,17 +106,18 @@ PuppetWidget::Create(nsIWidget        *a
   mEnabled = PR_TRUE;
   mVisible = PR_TRUE;
 
   mSurface = gfxPlatform::GetPlatform()
              ->CreateOffscreenSurface(gfxIntSize(1, 1),
                                       gfxASurface::ContentFromFormat(gfxASurface::ImageFormatARGB32));
 
   mIMEComposing = PR_FALSE;
-  mIMESuppressNotifySel = PR_FALSE;
+  mIMELastReceivedSeqno = 0;
+  mIMELastBlurSeqno = 0;
 
   PuppetWidget* parent = static_cast<PuppetWidget*>(aParent);
   if (parent) {
     parent->SetChild(this);
     mLayerManager = parent->GetLayerManager();
   }
   else {
     Resize(mBounds.x, mBounds.y, mBounds.width, mBounds.height, PR_FALSE);
@@ -272,25 +273,38 @@ PuppetWidget::DispatchEvent(nsGUIEvent* 
   debug_DumpEvent(stdout, event->widget, event,
                   nsCAutoString("PuppetWidget"), nsnull);
 #endif
 
   aStatus = nsEventStatus_eIgnore;
   if (mEventCallback) {
     if (event->message == NS_COMPOSITION_START) {
       mIMEComposing = PR_TRUE;
-    } else if (event->message == NS_SELECTION_SET) {
-      mIMESuppressNotifySel = PR_TRUE;
+    }
+    switch (event->eventStructType) {
+    case NS_COMPOSITION_EVENT:
+      mIMELastReceivedSeqno = static_cast<nsCompositionEvent*>(event)->seqno;
+      if (mIMELastReceivedSeqno < mIMELastBlurSeqno)
+        return NS_OK;
+      break;
+    case NS_TEXT_EVENT:
+      mIMELastReceivedSeqno = static_cast<nsTextEvent*>(event)->seqno;
+      if (mIMELastReceivedSeqno < mIMELastBlurSeqno)
+        return NS_OK;
+      break;
+    case NS_SELECTION_EVENT:
+      mIMELastReceivedSeqno = static_cast<nsSelectionEvent*>(event)->seqno;
+      if (mIMELastReceivedSeqno < mIMELastBlurSeqno)
+        return NS_OK;
+      break;
     }
     aStatus = (*mEventCallback)(event);
 
     if (event->message == NS_COMPOSITION_END) {
       mIMEComposing = PR_FALSE;
-    } else if (event->message == NS_SELECTION_SET) {
-      mIMESuppressNotifySel = PR_FALSE;
     }
   } else if (mChild) {
     event->widget = mChild;
     mChild->DispatchEvent(event, aStatus);
   }
 
   return NS_OK;
 }
@@ -396,26 +410,29 @@ PuppetWidget::OnIMEFocusChange(PRBool aF
     if (queryEvent.mSucceeded) {
       mTabChild->SendNotifyIMETextHint(queryEvent.mReply.mString);
     }
   } else {
     // ResetInputState might not have been called yet
     ResetInputState();
   }
 
+  PRUint32 chromeSeqno;
   mIMEPreference.mWantUpdates = PR_FALSE;
   mIMEPreference.mWantHints = PR_FALSE;
-  if (!mTabChild->SendNotifyIMEFocus(aFocus, &mIMEPreference))
+  if (!mTabChild->SendNotifyIMEFocus(aFocus, &mIMEPreference, &chromeSeqno))
     return NS_ERROR_FAILURE;
 
   if (aFocus) {
     if (!mIMEPreference.mWantUpdates && !mIMEPreference.mWantHints)
       // call OnIMEFocusChange on blur but no other updates
       return NS_SUCCESS_IME_NO_UPDATES;
     OnIMESelectionChange(); // Update selection
+  } else {
+    mIMELastBlurSeqno = chromeSeqno;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 PuppetWidget::OnIMETextChange(PRUint32 aStart, PRUint32 aEnd, PRUint32 aNewEnd)
 {
   if (!mTabChild)
@@ -439,31 +456,25 @@ PuppetWidget::OnIMETextChange(PRUint32 a
 }
 
 NS_IMETHODIMP
 PuppetWidget::OnIMESelectionChange(void)
 {
   if (!mTabChild)
     return NS_ERROR_FAILURE;
 
-  // When we send selection notifications during a composition or during a
-  // set selection event, there is a race condition where the notification
-  // arrives at chrome too late, which leads to chrome thinking the
-  // selection was elsewhere. Suppress notifications here to avoid that.
-  if (mIMEComposing || mIMESuppressNotifySel)
-    return NS_OK;
-
   if (mIMEPreference.mWantUpdates) {
     nsEventStatus status;
     nsQueryContentEvent queryEvent(PR_TRUE, NS_QUERY_SELECTED_TEXT, this);
     InitEvent(queryEvent, nsnull);
     DispatchEvent(&queryEvent, status);
 
     if (queryEvent.mSucceeded) {
-      mTabChild->SendNotifyIMESelection(queryEvent.GetSelectionStart(),
+      mTabChild->SendNotifyIMESelection(mIMELastReceivedSeqno,
+                                        queryEvent.GetSelectionStart(),
                                         queryEvent.GetSelectionEnd());
     }
   }
   return NS_OK;
 }
 
 nsresult
 PuppetWidget::DispatchPaintEvent()
--- a/widget/src/xpwidgets/PuppetWidget.h
+++ b/widget/src/xpwidgets/PuppetWidget.h
@@ -211,15 +211,21 @@ private:
   PRPackedBool mEnabled;
   PRPackedBool mVisible;
   // XXX/cjones: keeping this around until we teach LayerManager to do
   // retained-content-only transactions
   nsRefPtr<gfxASurface> mSurface;
   // IME
   nsIMEUpdatePreference mIMEPreference;
   PRPackedBool mIMEComposing;
-  PRPackedBool mIMESuppressNotifySel;
+  // Latest seqno received through events
+  PRUint32 mIMELastReceivedSeqno;
+  // Chrome's seqno value when last blur occurred
+  // arriving events with seqno up to this should be discarded
+  // Note that if seqno overflows (~50 days at 1 ms increment rate),
+  // events will be discarded until new focus/blur occurs
+  PRUint32 mIMELastBlurSeqno;
 };
 
 }  // namespace widget
 }  // namespace mozilla
 
 #endif  // mozilla_widget_PuppetWidget_h__