Bug 1368554 ContentCacheInParent::mPendingCompositionCount should be …

…decreased when TextCompositin which has dispatched composition events to corresponding remote process r=m_kato

ContentCacheInParent::mPendingCompositionCount is now managed with composition events which TabParent received. However, TextComposition doesn't dispatch composition events after coming request to commit active composition.  Therefore, composition is committed forcibly in a remote process over 255 times, the main process crashes.

It's the safest way to use TextComposition to manage ContentCacheInParent::mPendingCompositionCount.

MozReview-Commit-ID: DEhzYcK1zcW

--HG--
extra : rebase_source : a47891b1d620bbe4e380e73134ec6da5d21f4ea9

dom/events/IMEStateManager.cpp

@@ -277,8 +277,9 @@ IMEStateManager::OnDestroyPresContext(nsPresContext* aPresContext)
         ("  OnDestroyPresContext(), "
          "removing TextComposition instance from the array (index=%" PRIuSIZE ")", i));
       // there should be only one composition per presContext object.
-      sTextCompositions->ElementAt(i)->Destroy();
+      RefPtr<TextComposition> composition = sTextCompositions->ElementAt(i);
       sTextCompositions->RemoveElementAt(i);
+      composition->Destroy();
       if (sTextCompositions->IndexOf(aPresContext) !=
             TextCompositionArray::NoIndex) {
         MOZ_LOG(sISMLog, LogLevel::Error,
@@ -1293,10 +1294,11 @@ IMEStateManager::DispatchCompositionEvent(
     if (i != TextCompositionArray::NoIndex) {
       MOZ_LOG(sISMLog, LogLevel::Debug,
         ("  DispatchCompositionEvent(), "
-         "removing TextComposition from the array since NS_COMPOSTION_END "
+         "removing TextComposition from the array since eCompositionEnd "
          "was dispatched"));
-      sTextCompositions->ElementAt(i)->Destroy();
+      RefPtr<TextComposition> composition = sTextCompositions->ElementAt(i);
       sTextCompositions->RemoveElementAt(i);
+      composition->Destroy();
     }
   }
 }

dom/events/TextComposition.cpp

@@ -70,16 +70,30 @@ TextComposition::TextComposition(nsPresContext* aPresContext,
       Preferences::GetBool("dom.compositionevent.allow_control_characters",
                            false))
   , mWasCompositionStringEmpty(true)
+  , mHasDispatchedCompositionEvents(false)
 {
   MOZ_ASSERT(aCompositionEvent->mNativeIMEContext.IsValid());
 }
 
+TextComposition::~TextComposition()
+{
+  // WARNING: mPresContext may be destroying, so, be careful if you touch it.
+  if (NS_WARN_IF(mTabParent)) {
+    Destroy();
+  }
+}
+
 void
 TextComposition::Destroy()
 {
   mPresContext = nullptr;
   mNode = nullptr;
-  mTabParent = nullptr;
+  if (mTabParent) {
+    RefPtr<TabParent> tabParent = mTabParent.forget();
+    if (mHasDispatchedCompositionEvents) {
+      tabParent->OnDestroyTextComposition();
+    }
+  }
   // TODO: If the editor is still alive and this is held by it, we should tell
   //       this being destroyed for cleaning up the stuff.
 }
@@ -151,6 +165,7 @@ TextComposition::DispatchEvent(WidgetCompositionEvent* aDispatchEvent,
   nsPluginInstanceOwner::GeneratePluginEvent(aOriginalEvent,
                                              aDispatchEvent);
 
+  mHasDispatchedCompositionEvents = true;
   EventDispatcher::Dispatch(mNode, mPresContext,
                             aDispatchEvent, nullptr, aStatus, aCallBack);
 
@@ -249,6 +264,7 @@ TextComposition::DispatchCompositionEvent(
   // If the content is a container of TabParent, composition should be in the
   // remote process.
   if (mTabParent) {
+    mHasDispatchedCompositionEvents = true;
     Unused << mTabParent->SendCompositionEvent(*aCompositionEvent);
     aCompositionEvent->StopPropagation();
     if (aCompositionEvent->CausesDOMTextEvent()) {

dom/events/TextComposition.h

@@ -178,10 +178,7 @@ class TextComposition final
 
 private:
   // Private destructor, to discourage deletion outside of Release():
-  ~TextComposition()
-  {
-    // WARNING: mPresContext may be destroying, so, be careful if you touch it.
-  }
+  ~TextComposition();
 
   // sHandlingSelectionEvent is true while TextComposition sends a selection
   // event to ContentEventHandler.
@@ -262,6 +259,10 @@ class TextComposition final
   // when DispatchCompositionEvent() is called.
   bool mWasCompositionStringEmpty;
 
+  // mHasDispatchedCompositionEvents is true if the instance has dispatched
+  // one or more composition events.
+  bool mHasDispatchedCompositionEvents;
+
   // Hide the default constructor and copy constructor.
   TextComposition()
     : mPresContext(nullptr)
@@ -277,6 +278,7 @@ class TextComposition final
     , mWasNativeCompositionEndEventDiscarded(false)
     , mAllowControlCharacters(false)
     , mWasCompositionStringEmpty(true)
+    , mHasDispatchedCompositionEvents(false)
   {}
   TextComposition(const TextComposition& aOther);
 

dom/ipc/TabParent.cpp

@@ -2067,6 +2067,12 @@ TabParent::SendPasteTransferable(const IPCDataTransfer& aDataTransfer,
                                                aRequestingPrincipal);
 }
 
+void
+TabParent::OnDestroyTextComposition()
+{
+  mContentCache.OnDestroyTextComposition();
+}
+
 /*static*/ TabParent*
 TabParent::GetFrom(nsFrameLoader* aFrameLoader)
 {

dom/ipc/TabParent.h

@@ -507,6 +507,12 @@ class TabParent final : public PBrowserParent
                              const bool& aIsPrivateData,
                              const IPC::Principal& aRequestingPrincipal);
 
+  /**
+   * OnDestroyTextComposition() should be called when TextComposition instance
+   * which dispatched composition events to this instance is being destroyed.
+   */
+  void OnDestroyTextComposition();
+
   static TabParent* GetFrom(nsFrameLoader* aFrameLoader);
 
   static TabParent* GetFrom(nsIFrameLoader* aFrameLoader);

widget/ContentCache.cpp

@@ -1162,13 +1162,8 @@ ContentCacheInParent::OnEventNeedingAckHandled(nsIWidget* aWidget,
 
   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)) {
-    MOZ_RELEASE_ASSERT(mPendingCompositionCount > 0);
-    mPendingCompositionCount--;
-  }
+     "aMessage=%s), mPendingEventsNeedingAck=%u",
+     this, aWidget, ToChar(aMessage), mPendingEventsNeedingAck));
 
   MOZ_RELEASE_ASSERT(mPendingEventsNeedingAck > 0);
   if (--mPendingEventsNeedingAck) {
@@ -1178,6 +1173,17 @@ ContentCacheInParent::OnEventNeedingAckHandled(nsIWidget* aWidget,
   FlushPendingNotifications(aWidget);
 }
 
+void
+ContentCacheInParent::OnDestroyTextComposition()
+{
+  MOZ_LOG(sContentCacheLog, LogLevel::Info,
+    ("0x%p OnDestroyTextComposition(), "
+     "mPendingEventsNeedingAck=%u, mPendingCompositionCount=%" PRIu8,
+     this, mPendingEventsNeedingAck, mPendingCompositionCount));
+  MOZ_RELEASE_ASSERT(mPendingCompositionCount > 0);
+  mPendingCompositionCount--;
+}
+
 bool
 ContentCacheInParent::RequestIMEToCommitComposition(nsIWidget* aWidget,
                                                     bool aCancel,

widget/ContentCache.h

@@ -374,6 +374,13 @@ class ContentCacheInParent final : public ContentCache
    */
   void OnEventNeedingAckHandled(nsIWidget* aWidget, EventMessage aMessage);
 
+  /**
+   * OnDestroyTextComposition() should be called when TextComposition instance
+   * which dispatched composition events to the owner of this instance is being
+   * destroyed.
+   */
+  void OnDestroyTextComposition();
+
   /**
    * RequestIMEToCommitComposition() requests aWidget to commit or cancel
    * composition.  If it's handled synchronously, this returns true.