Bug 1376424 - part0: Backout the patch for bug 1368554 r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 27 Jun 2017 22:02:07 +0900
changeset 366442 fa41e128cb5c
parent 366441 5e2b8234a6b5
child 366443 111ba549dd0a
push id32104
push usercbook@mozilla.com
push dateThu, 29 Jun 2017 13:46:04 +0000
treeherdermozilla-central@d2aff6fc075d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1376424, 1368554
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 1376424 - part0: Backout the patch for bug 1368554 r=m_kato TextComposition in the main process is destroyed when the main process sends eCompositionCommit(AsIs) to focused remote process. Therefore, ContentCacheInParent::mCompositionPendingCount is never 2 or more now. It may cause ContentCacheInParent::Assign() setting older composition's start offset to current composition's start offset in the main process. For making uplift the following patch easier, the wrong patch should be backed out first. MozReview-Commit-ID: IHWc7qZBQtc
dom/events/IMEStateManager.cpp
dom/events/TextComposition.cpp
dom/events/TextComposition.h
dom/ipc/TabParent.cpp
dom/ipc/TabParent.h
widget/ContentCache.cpp
widget/ContentCache.h
--- a/dom/events/IMEStateManager.cpp
+++ b/dom/events/IMEStateManager.cpp
@@ -272,19 +272,18 @@ IMEStateManager::OnDestroyPresContext(ns
   if (sTextCompositions) {
     TextCompositionArray::index_type i =
       sTextCompositions->IndexOf(aPresContext);
     if (i != TextCompositionArray::NoIndex) {
       MOZ_LOG(sISMLog, LogLevel::Debug,
         ("  OnDestroyPresContext(), "
          "removing TextComposition instance from the array (index=%" PRIuSIZE ")", i));
       // there should be only one composition per presContext object.
-      RefPtr<TextComposition> composition = sTextCompositions->ElementAt(i);
+      sTextCompositions->ElementAt(i)->Destroy();
       sTextCompositions->RemoveElementAt(i);
-      composition->Destroy();
       if (sTextCompositions->IndexOf(aPresContext) !=
             TextCompositionArray::NoIndex) {
         MOZ_LOG(sISMLog, LogLevel::Error,
           ("  OnDestroyPresContext(), FAILED to remove "
            "TextComposition instance from the array"));
         MOZ_CRASH("Failed to remove TextComposition instance from the array");
       }
     }
@@ -1289,21 +1288,20 @@ IMEStateManager::DispatchCompositionEven
   if ((!aIsSynthesized ||
        composition->WasNativeCompositionEndEventDiscarded()) &&
       aCompositionEvent->CausesDOMCompositionEndEvent()) {
     TextCompositionArray::index_type i =
       sTextCompositions->IndexOf(aCompositionEvent->mWidget);
     if (i != TextCompositionArray::NoIndex) {
       MOZ_LOG(sISMLog, LogLevel::Debug,
         ("  DispatchCompositionEvent(), "
-         "removing TextComposition from the array since eCompositionEnd "
+         "removing TextComposition from the array since NS_COMPOSTION_END "
          "was dispatched"));
-      RefPtr<TextComposition> composition = sTextCompositions->ElementAt(i);
+      sTextCompositions->ElementAt(i)->Destroy();
       sTextCompositions->RemoveElementAt(i);
-      composition->Destroy();
     }
   }
 }
 
 // static
 IMEContentObserver*
 IMEStateManager::GetActiveContentObserver()
 {
--- a/dom/events/TextComposition.cpp
+++ b/dom/events/TextComposition.cpp
@@ -65,40 +65,26 @@ TextComposition::TextComposition(nsPresC
   , mIsRequestingCommit(false)
   , mIsRequestingCancel(false)
   , mRequestedToCommitOrCancel(false)
   , mWasNativeCompositionEndEventDiscarded(false)
   , mAllowControlCharacters(
       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;
-  if (mTabParent) {
-    RefPtr<TabParent> tabParent = mTabParent.forget();
-    if (mHasDispatchedCompositionEvents) {
-      tabParent->OnDestroyTextComposition();
-    }
-  }
+  mTabParent = nullptr;
   // TODO: If the editor is still alive and this is held by it, we should tell
   //       this being destroyed for cleaning up the stuff.
 }
 
 bool
 TextComposition::IsValidStateForComposition(nsIWidget* aWidget) const
 {
   return !Destroyed() && aWidget && !aWidget->Destroyed() &&
@@ -160,17 +146,16 @@ void
 TextComposition::DispatchEvent(WidgetCompositionEvent* aDispatchEvent,
                                nsEventStatus* aStatus,
                                EventDispatchingCallback* aCallBack,
                                const WidgetCompositionEvent *aOriginalEvent)
 {
   nsPluginInstanceOwner::GeneratePluginEvent(aOriginalEvent,
                                              aDispatchEvent);
 
-  mHasDispatchedCompositionEvents = true;
   EventDispatcher::Dispatch(mNode, mPresContext,
                             aDispatchEvent, nullptr, aStatus, aCallBack);
 
   OnCompositionEventDispatched(aDispatchEvent);
 }
 
 void
 TextComposition::OnCompositionEventDiscarded(
@@ -259,17 +244,16 @@ TextComposition::DispatchCompositionEven
                    EventDispatchingCallback* aCallBack,
                    bool aIsSynthesized)
 {
   mWasCompositionStringEmpty = mString.IsEmpty();
 
   // 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()) {
       mLastData = aCompositionEvent->mData;
       mLastRanges = aCompositionEvent->mRanges;
       // Although, the composition event hasn't been actually handled yet,
       // emulate an editor to be handling the composition event.
       EditorWillHandleCompositionChangeEvent(aCompositionEvent);
--- a/dom/events/TextComposition.h
+++ b/dom/events/TextComposition.h
@@ -172,17 +172,20 @@ public:
     RefPtr<TextComposition> mComposition;
     CompositionChangeEventHandlingMarker();
     CompositionChangeEventHandlingMarker(
       const CompositionChangeEventHandlingMarker& aOther);
   };
 
 private:
   // Private destructor, to discourage deletion outside of Release():
-  ~TextComposition();
+  ~TextComposition()
+  {
+    // WARNING: mPresContext may be destroying, so, be careful if you touch it.
+  }
 
   // sHandlingSelectionEvent is true while TextComposition sends a selection
   // event to ContentEventHandler.
   static bool sHandlingSelectionEvent;
 
   // This class holds nsPresContext weak.  This instance shouldn't block
   // destroying it.  When the presContext is being destroyed, it's notified to
   // IMEStateManager::OnDestroyPresContext(), and then, it destroy
@@ -254,36 +257,31 @@ private:
   // both composition string and data attribute of compositionupdate
   // and compositionend events.
   bool mAllowControlCharacters;
 
   // mWasCompositionStringEmpty is true if the composition string was empty
   // 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)
     , mNativeContext(nullptr)
     , mCompositionStartOffset(0)
     , mTargetClauseOffsetInComposition(0)
     , mIsSynthesizedForTests(false)
     , mIsComposing(false)
     , mIsEditorHandlingEvent(false)
     , mIsRequestingCommit(false)
     , mIsRequestingCancel(false)
     , mRequestedToCommitOrCancel(false)
     , mWasNativeCompositionEndEventDiscarded(false)
     , mAllowControlCharacters(false)
     , mWasCompositionStringEmpty(true)
-    , mHasDispatchedCompositionEvents(false)
   {}
   TextComposition(const TextComposition& aOther);
 
   /**
    * GetEditorBase() returns EditorBase pointer of mEditorBaseWeak.
    */
   already_AddRefed<EditorBase> GetEditorBase() const;
 
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -2079,22 +2079,16 @@ TabParent::SendPasteTransferable(const I
                                  const bool& aIsPrivateData,
                                  const IPC::Principal& aRequestingPrincipal)
 {
   return PBrowserParent::SendPasteTransferable(aDataTransfer,
                                                aIsPrivateData,
                                                aRequestingPrincipal);
 }
 
-void
-TabParent::OnDestroyTextComposition()
-{
-  mContentCache.OnDestroyTextComposition();
-}
-
 /*static*/ TabParent*
 TabParent::GetFrom(nsFrameLoader* aFrameLoader)
 {
   if (!aFrameLoader) {
     return nullptr;
   }
   PBrowserParent* remoteBrowser = aFrameLoader->GetRemoteBrowser();
   return static_cast<TabParent*>(remoteBrowser);
--- a/dom/ipc/TabParent.h
+++ b/dom/ipc/TabParent.h
@@ -499,22 +499,16 @@ public:
   bool SendCompositionEvent(mozilla::WidgetCompositionEvent& aEvent);
 
   bool SendSelectionEvent(mozilla::WidgetSelectionEvent& aEvent);
 
   bool SendPasteTransferable(const IPCDataTransfer& aDataTransfer,
                              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);
 
   static TabParent* GetFrom(nsITabParent* aTabParent);
 
   static TabParent* GetFrom(PBrowserParent* aTabParent);
 
--- a/widget/ContentCache.cpp
+++ b/widget/ContentCache.cpp
@@ -1157,38 +1157,32 @@ void
 ContentCacheInParent::OnEventNeedingAckHandled(nsIWidget* aWidget,
                                                 EventMessage aMessage)
 {
   // 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",
-     this, aWidget, ToChar(aMessage), mPendingEventsNeedingAck));
+     "aMessage=%s), mPendingEventsNeedingAck=%u, mPendingCompositionCount=%" PRIu8,
+     this, aWidget, ToChar(aMessage), mPendingEventsNeedingAck, mPendingCompositionCount));
+
+  if (WidgetCompositionEvent::IsFollowedByCompositionEnd(aMessage)) {
+    MOZ_RELEASE_ASSERT(mPendingCompositionCount > 0);
+    mPendingCompositionCount--;
+  }
 
   MOZ_RELEASE_ASSERT(mPendingEventsNeedingAck > 0);
   if (--mPendingEventsNeedingAck) {
     return;
   }
 
   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,
                                                     nsAString& aCommittedString)
 {
   MOZ_LOG(sContentCacheLog, LogLevel::Info,
     ("0x%p RequestToCommitComposition(aWidget=%p, "
      "aCancel=%s), mWidgetHasComposition=%s, mCommitStringByRequest=%p",
--- a/widget/ContentCache.h
+++ b/widget/ContentCache.h
@@ -370,23 +370,16 @@ public:
    *
    * WARNING: This may send notifications to IME.  That might cause destroying
    *          TabParent or aWidget.  Therefore, the caller must not destroy
    *          this instance during a call of this method.
    */
   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.
    *
    * @param aWidget     The widget to be requested to commit or cancel
    *                    the composition.
    * @param aCancel     When the caller tries to cancel the composition, true.
    *                    Otherwise, i.e., tries to commit the composition, false.
    * @param aCommittedString    The committed string (i.e., the last data of