Bug 1405832 - part 2: TextComposition::RequestToCommit() should request IME to commit or cancel composition only when it hasn't been request it yet and hasn't received commit event yet r=m_kato

According to the log in crash reports, eCompositionCommitRequestHandled is
sent to ContentCacheInParent twice or more for a composition.  This causes
breaking mPendingCompositionCount and mPendingEventsNeedingAck management.

Currently, nsIWidget::NotifyIME() should be called only by
TextComposition::RequestToCommit().  Therefore, the method should manage if
it should request it actually.  If the composition has already received
eCompositionCommit(AsIs) event, it shouldn't request it because parent process
may have already stated new composition and it shouldn't be broken by request
for old composition.

MozReview-Commit-ID: 2ekSa6EIeRP

--HG--
extra : rebase_source : d23aa29ce7871e83b99cec8c15aff0c580e08fb4

dom/events/TextComposition.cpp

@@ -65,6 +65,7 @@ TextComposition::TextComposition(nsPresContext* aPresContext,
   , mIsRequestingCommit(false)
   , mIsRequestingCancel(false)
   , mRequestedToCommitOrCancel(false)
+  , mHasReceivedCommitEvent(false)
   , mWasNativeCompositionEndEventDiscarded(false)
   , mAllowControlCharacters(
       Preferences::GetBool("dom.compositionevent.allow_control_characters",
@@ -246,6 +247,10 @@ TextComposition::DispatchCompositionEvent(
 {
   mWasCompositionStringEmpty = mString.IsEmpty();
 
+  if (aCompositionEvent->IsFollowedByCompositionEnd()) {
+    mHasReceivedCommitEvent = true;
+  }
+
   // If this instance has requested to commit or cancel composition but
   // is not synthesizing commit event, that means that the IME commits or
   // cancels the composition asynchronously.  Typically, iBus behaves so.
@@ -563,10 +568,12 @@ nsresult
 TextComposition::RequestToCommit(nsIWidget* aWidget, bool aDiscard)
 {
   // If this composition is already requested to be committed or canceled,
-  // we don't need to request it again because even if the first request
-  // failed, new request won't success, probably.  And we shouldn't synthesize
-  // events for committing or canceling composition twice or more times.
-  if (mRequestedToCommitOrCancel) {
+  // or has already finished in IME, we don't need to request it again because
+  // request from this instance shouldn't cause committing nor canceling current
+  // composition in IME, and even if the first request failed, new request
+  // won't success, probably.  And we shouldn't synthesize events for
+  // committing or canceling composition twice or more times.
+  if (!CanRequsetIMEToCommitOrCancelComposition()) {
     return NS_OK;
   }
 

dom/events/TextComposition.h

@@ -100,6 +100,15 @@ public:
    */
   nsresult RequestToCommit(nsIWidget* aWidget, bool aDiscard);
 
+  /**
+   * IsRequestingCommitOrCancelComposition() returns true if the instance is
+   * requesting widget to commit or cancel composition.
+   */
+  bool IsRequestingCommitOrCancelComposition() const
+  {
+    return mIsRequestingCancel || mIsRequestingCommit;
+  }
+
   /**
    * Send a notification to IME.  It depends on the IME or platform spec what
    * will occur (or not occur).
@@ -247,10 +256,14 @@ private:
   // mRequestedToCommitOrCancel is true *after* we requested IME to commit or
   // cancel the composition.  In other words, we already requested of IME that
   // it commits or cancels current composition.
-  // NOTE: Before this is set true, both mIsRequestingCommit and
-  //       mIsRequestingCancel are set false.
+  // NOTE: Before this is set to true, both mIsRequestingCommit and
+  //       mIsRequestingCancel are set to false.
   bool mRequestedToCommitOrCancel;
 
+  // Before this dispatches commit event into the tree, this is set to true.
+  // So, this means if native IME already commits the composition.
+  bool mHasReceivedCommitEvent;
+
   // mWasNativeCompositionEndEventDiscarded is true if this composition was
   // requested commit or cancel itself but native compositionend event is
   // discarded by PresShell due to not safe to dispatch events.
@@ -279,12 +292,27 @@ private:
     , mIsRequestingCommit(false)
     , mIsRequestingCancel(false)
     , mRequestedToCommitOrCancel(false)
+    , mHasReceivedCommitEvent(false)
     , mWasNativeCompositionEndEventDiscarded(false)
     , mAllowControlCharacters(false)
     , mWasCompositionStringEmpty(true)
   {}
   TextComposition(const TextComposition& aOther);
 
+  /**
+   * If we're requesting IME to commit or cancel composition, or we've already
+   * requested it, or we've already known this composition has been ended in
+   * IME, we don't need to request commit nor cancel composition anymore and
+   * shouldn't do so if we're in content process for not committing/canceling
+   * "current" composition in native IME.  So, when this returns true,
+   * RequestIMEToCommit() does nothing.
+   */
+  bool CanRequsetIMEToCommitOrCancelComposition() const
+  {
+    return !mIsRequestingCommit && !mIsRequestingCancel &&
+           !mRequestedToCommitOrCancel && !mHasReceivedCommitEvent;
+  }
+
   /**
    * GetEditorBase() returns EditorBase pointer of mEditorBaseWeak.
    */

widget/PuppetWidget.cpp

@@ -652,6 +652,10 @@ PuppetWidget::RequestIMEToCommitComposition(bool aCancel)
     return NS_OK;
   }
 
+  MOZ_DIAGNOSTIC_ASSERT(composition->IsRequestingCommitOrCancelComposition(),
+    "Requesting commit or cancel composition should be requested via "
+    "TextComposition instance");
+
   bool isCommitted = false;
   nsAutoString committedString;
   if (NS_WARN_IF(!mTabChild->SendRequestIMEToCommitComposition(