Bug 1380652 - ContentCacheInParent::RequestIMEToCommitComposition() shouldn't handle the request synchronously when its TabParent has already sent eCompositionCommit(AsIs) event of the composition r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 21 Jul 2017 21:22:23 +0900
changeset 419412 ef4d1461791e38d3730fc8e43aee0e871e83333c
parent 419411 a05e8758316f1cdfc078cd30866cda481c94c115
child 419413 aebaa2a545ba19f6667859b0afaa237364f61863
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1380652
milestone56.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 1380652 - ContentCacheInParent::RequestIMEToCommitComposition() shouldn't handle the request synchronously when its TabParent has already sent eCompositionCommit(AsIs) event of the composition r=m_kato If ContentCacheInParent::RequestIMEToCommitComposition() returns true after its TabParent has already sent eCompositionCommit(AsIs) event, ContentCacheInParent::OnEventNeedingAckHandled() will receive both eCompositionCommit(AsIs) and eCompositionCommitRequestHandled for a composition. Then, that causes making mPendingCompositionCount decreased twice. That causes the crash. So, even if its TabParent already lost focus, it should return false after its TabParent sent eCompositionCommit(AsIs). MozReview-Commit-ID: 6OylQtc8kgC
widget/ContentCache.cpp
--- a/widget/ContentCache.cpp
+++ b/widget/ContentCache.cpp
@@ -1248,35 +1248,34 @@ ContentCacheInParent::RequestIMEToCommit
   // eCompositionCommit(AsIs) to the remote process.  So, this request is
   // too late for IME.  The remote process should wait following
   // composition events for cleaning up TextComposition and handle the
   // request as it's handled asynchronously.
   if (mPendingCompositionCount > 1) {
     return false;
   }
 
+  // If there is no pending composition, we may have already sent
+  // eCompositionCommit(AsIs) event for the active composition.  If so, the
+  // remote process will receive composition events which causes cleaning up
+  // TextComposition.  So, this shouldn't do nothing and TextComposition
+  // should handle the request as it's handled asynchronously.
+  if (mIsPendingLastCommitEvent) {
+    return false;
+  }
+
   // If TabParent which has IME focus was already changed to different one, the
   // request shouldn't be sent to IME because it's too late.
   if (!IMEStateManager::DoesTabParentHaveIMEFocus(&mTabParent)) {
     // Use the latest composition string which may not be handled in the
     // remote process for avoiding data loss.
     aCommittedString = mCompositionString;
     return true;
   }
 
-  // Even if the remote process has IME focus and there is no pending
-  // composition, we may have already sent eCompositionCommit(AsIs) event
-  // to it.  If so, the remote process will receive composition events
-  // which causes cleaning up TextComposition.  So, this shouldn't do nothing
-  // and TextComposition should handle the request as it's handled
-  // asynchronously.
-  if (mIsPendingLastCommitEvent) {
-    return false;
-  }
-
   RefPtr<TextComposition> composition =
     IMEStateManager::GetTextCompositionFor(aWidget);
   if (NS_WARN_IF(!composition)) {
     MOZ_LOG(sContentCacheLog, LogLevel::Warning,
       ("  0x%p RequestToCommitComposition(), "
        "does nothing due to no composition", this));
     return false;
   }