Bug 1376424 - part1: TabChild should notify TabParent of "request to commit composition" handled r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 27 Jun 2017 23:41:12 +0900
changeset 601162 03891ec1d1db1d584af1151f869e40bdb4b5334e
parent 601161 226cef11665566e8f3dd1f4938d8be283a0e6948
child 601163 5c965a6b3c0c22786ad935b7016aa2cefefbc99a
push id65974
push usermasayuki@d-toybox.com
push dateWed, 28 Jun 2017 06:08:16 +0000
reviewersm_kato
bugs1376424
milestone56.0a1
Bug 1376424 - part1: TabChild should notify TabParent of "request to commit composition" handled r?m_kato The problem is, only when requesting IME to commit or cancel composition is handled synchronously, TabParent does not send the dispatched eCompositionCommit(AsIs) event to the remote process. Therefore, TabParent (and ContentCacheInParent) never receives the message from the remote process. This patch makes TabChild notifies TabParent of eCompositionCommitRequestHandled special event message after TabChild dispatches eCompositionCommit into the DOM tree. Then, ContentCacheInParent should decrease mPendingCompositionCount and mPendingEventsNeedingAck as usual composition event messages. MozReview-Commit-ID: 7ec5HPiE687
dom/ipc/TabParent.cpp
widget/ContentCache.cpp
widget/EventMessageList.h
widget/PuppetWidget.cpp
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -1793,20 +1793,19 @@ TabParent::RecvNotifyIMEPositionChange(c
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 TabParent::RecvOnEventNeedingAckHandled(const EventMessage& aMessage)
 {
   // This is called when the child process receives WidgetCompositionEvent or
   // WidgetSelectionEvent.
+  // FYI: Don't check if widget is nullptr here because it's more important to
+  //      notify mContentCahce of this than handling something in it.
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget) {
-    return IPC_OK();
-  }
 
   // While calling OnEventNeedingAckHandled(), TabParent *might* be destroyed
   // since it may send notifications to IME.
   RefPtr<TabParent> kungFuDeathGrip(this);
   mContentCache.OnEventNeedingAckHandled(widget, aMessage);
   return IPC_OK();
 }
 
--- a/widget/ContentCache.cpp
+++ b/widget/ContentCache.cpp
@@ -1121,16 +1121,22 @@ ContentCacheInParent::OnCompositionEvent
   // we need to intercept all composition events here and pass the commit
   // string for returning to the remote process as a result of
   // RequestIMEToCommitComposition().  Then, eCommitComposition event will
   // be dispatched with the committed string in the remote process internally.
   if (mCommitStringByRequest) {
     MOZ_ASSERT(aEvent.mMessage == eCompositionChange ||
                aEvent.mMessage == eCompositionCommit);
     *mCommitStringByRequest = aEvent.mData;
+    // We need to wait eCompositionCommitRequestHandled from the remote process
+    // in this case.  Therefore, mPendingEventsNeedingAck needs to be
+    // incremented here.
+    if (!mWidgetHasComposition) {
+      mPendingEventsNeedingAck++;
+    }
     return false;
   }
 
   mPendingEventsNeedingAck++;
   return true;
 }
 
 void
@@ -1160,17 +1166,18 @@ ContentCacheInParent::OnEventNeedingAckH
   // This is called when the child process receives WidgetCompositionEvent or
   // WidgetSelectionEvent.
 
   MOZ_LOG(sContentCacheLog, LogLevel::Info,
     ("0x%p OnEventNeedingAckHandled(aWidget=0x%p, "
      "aMessage=%s), mPendingEventsNeedingAck=%u, mPendingCompositionCount=%" PRIu8,
      this, aWidget, ToChar(aMessage), mPendingEventsNeedingAck, mPendingCompositionCount));
 
-  if (WidgetCompositionEvent::IsFollowedByCompositionEnd(aMessage)) {
+  if (WidgetCompositionEvent::IsFollowedByCompositionEnd(aMessage) ||
+      aMessage == eCompositionCommitRequestHandled) {
     MOZ_RELEASE_ASSERT(mPendingCompositionCount > 0);
     mPendingCompositionCount--;
   }
 
   MOZ_RELEASE_ASSERT(mPendingEventsNeedingAck > 0);
   if (--mPendingEventsNeedingAck) {
     return;
   }
--- a/widget/EventMessageList.h
+++ b/widget/EventMessageList.h
@@ -193,16 +193,24 @@ NS_EVENT_MESSAGE(eCompositionChange)
 // Its mData and mRanges should be empty and nullptr.
 NS_EVENT_MESSAGE(eCompositionCommitAsIs)
 // eCompositionCommit is the message for representing a commit of
 // composition string with its mData value.  TextComposition will dispatch this
 // event to the DOM tree as eCompositionChange without clause information.
 // After that, eCompositionEnd will be dispatched automatically.
 // Its mRanges should be nullptr.
 NS_EVENT_MESSAGE(eCompositionCommit)
+// eCompositionCommitRequestHandled is NOT used with any Widget*Event.
+// This is used only by PBrowser.OnEventNeedingAckHandled().  If active IME
+// commits composition synchronously, TabParent returns the commit string
+// to the remote process synchronously.  Then, TabChild dispatches
+// eCompositionCommit in the remote process.  Finally, this message is sent
+// to TabParent.  (If IME commits composition asynchronously, this message is
+// not used.)
+NS_EVENT_MESSAGE(eCompositionCommitRequestHandled)
 
 // Following events are defined for deprecated DOM events which are using
 // InternalUIEvent class.
 // DOMActivate (mapped with the DOM event and used internally)
 NS_EVENT_MESSAGE(eLegacyDOMActivate)
 // DOMFocusIn (only mapped with the DOM event)
 NS_EVENT_MESSAGE(eLegacyDOMFocusIn)
 // DOMFocusOut (only mapped with the DOM event)
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -648,16 +648,19 @@ PuppetWidget::RequestIMEToCommitComposit
 
   // Dispatch eCompositionCommit event.
   WidgetCompositionEvent compositionCommitEvent(true, eCompositionCommit, this);
   InitEvent(compositionCommitEvent, nullptr);
   compositionCommitEvent.mData = committedString;
   nsEventStatus status = nsEventStatus_eIgnore;
   DispatchEvent(&compositionCommitEvent, status);
 
+  Unused <<
+    mTabChild->SendOnEventNeedingAckHandled(eCompositionCommitRequestHandled);
+
   // NOTE: PuppetWidget might be destroyed already.
   return NS_OK;
 }
 
 nsresult
 PuppetWidget::NotifyIMEInternal(const IMENotification& aIMENotification)
 {
   if (mNativeTextEventDispatcherListener) {