Bug 1405832 - part 4: ContentCacheInParent::OnEventNeedingAckHandled() shouldn't crash in release build r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 20 Nov 2017 23:30:18 +0900
changeset 701894 48c4741c2126c9e6664ad5ec65bf8fa6e6ca1c1b
parent 701893 1f47ff0bac04bb97961cc85bbab12db60a9ffb92
child 741298 0871abc915e08aa228e046f0cb4403833d956100
push id90306
push usermasayuki@d-toybox.com
push dateWed, 22 Nov 2017 12:43:08 +0000
reviewersm_kato
bugs1405832
milestone59.0a1
Bug 1405832 - part 4: ContentCacheInParent::OnEventNeedingAckHandled() shouldn't crash in release build r?m_kato For protecting main process, we should stop crashing main process in release build even when we detect our bug. However, we should keep crashing with MOZ_DIAGNOSTIC_ASSER which is enabled only on Night and Developer Edition. MozReview-Commit-ID: 5BQ46IFzXXj
widget/ContentCache.cpp
widget/ContentCache.h
--- a/widget/ContentCache.cpp
+++ b/widget/ContentCache.cpp
@@ -1108,19 +1108,19 @@ ContentCacheInParent::OnCompositionEvent
      "mPendingEventsNeedingAck=%u, mWidgetHasComposition=%s, "
      "mPendingCompositionCount=%u, mCommitStringByRequest=0x%p",
      this, ToChar(aEvent.mMessage),
      GetEscapedUTF8String(aEvent.mData).get(), aEvent.mData.Length(),
      aEvent.mRanges ? aEvent.mRanges->Length() : 0, mPendingEventsNeedingAck,
      GetBoolName(mWidgetHasComposition), mPendingCompositionCount,
      mCommitStringByRequest));
 
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
   mDispatchedEventMessages.AppendElement(aEvent.mMessage);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
   // We must be able to simulate the selection because
   // we might not receive selection updates in time
   if (!mWidgetHasComposition) {
     if (aEvent.mWidget && aEvent.mWidget->PluginHasFocus()) {
       // If focus is on plugin, we cannot get selection range
       mCompositionStart = 0;
     } else if (mCompositionStartInChild != UINT32_MAX) {
@@ -1188,57 +1188,59 @@ ContentCacheInParent::OnSelectionEvent(
      "mPendingCompositionCount=%u",
      this, ToChar(aSelectionEvent.mMessage),
      aSelectionEvent.mOffset, aSelectionEvent.mLength,
      GetBoolName(aSelectionEvent.mReversed),
      GetBoolName(aSelectionEvent.mExpandToClusterBoundary),
      GetBoolName(aSelectionEvent.mUseNativeLineBreak), mPendingEventsNeedingAck,
      GetBoolName(mWidgetHasComposition), mPendingCompositionCount));
 
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
   mDispatchedEventMessages.AppendElement(aSelectionEvent.mMessage);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
   mPendingEventsNeedingAck++;
 }
 
 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, mPendingCompositionCount=%" PRIu8,
      this, aWidget, ToChar(aMessage), mPendingEventsNeedingAck, mPendingCompositionCount));
 
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
   mReceivedEventMessages.AppendElement(aMessage);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
   if (WidgetCompositionEvent::IsFollowedByCompositionEnd(aMessage) ||
       aMessage == eCompositionCommitRequestHandled) {
 
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     if (mPendingCompositionCount == 1) {
       RemoveUnnecessaryEventMessageLog();
     }
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
     if (NS_WARN_IF(!mPendingCompositionCount)) {
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
       nsPrintfCString info("\nThere is no pending composition but received %s "
                            "message from the remote child\n\n",
                            ToChar(aMessage));
       AppendEventMessageLog(info);
       CrashReporter::AppendAppNotesToCrashReport(info);
-#endif // #ifdef MOZ_CRASHREPORTER
-      MOZ_CRASH("No pending composition but received unexpected commit event");
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
+      MOZ_DIAGNOSTIC_ASSERT(false,
+        "No pending composition but received unexpected commit event");
+      mPendingCompositionCount = 1;
     }
 
     mPendingCompositionCount--;
     // Forget composition string only when the latest composition string is
     // handled in the remote process because if there is 2 or more pending
     // composition, this value shouldn't be referred.
     if (!mPendingCompositionCount) {
       mCompositionString.Truncate();
@@ -1247,24 +1249,26 @@ ContentCacheInParent::OnEventNeedingAckH
     // Forget pending commit string length if it's handled in the remote
     // process.  Note that this doesn't care too old composition's commit
     // string because in such case, we cannot return proper information
     // to IME synchornously.
     mPendingCommitLength = 0;
   }
 
   if (NS_WARN_IF(!mPendingEventsNeedingAck)) {
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     nsPrintfCString info("\nThere is no pending events but received %s "
                          "message from the remote child\n\n",
                          ToChar(aMessage));
     AppendEventMessageLog(info);
     CrashReporter::AppendAppNotesToCrashReport(info);
-#endif // #ifdef MOZ_CRASHREPORTER
-    MOZ_CRASH("No pending event message but received unexpected event");
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
+    MOZ_DIAGNOSTIC_ASSERT(false,
+      "No pending event message but received unexpected event");
+    mPendingEventsNeedingAck = 1;
   }
   if (--mPendingEventsNeedingAck) {
     return;
   }
 
   FlushPendingNotifications(aWidget);
 }
 
@@ -1285,68 +1289,68 @@ ContentCacheInParent::RequestIMEToCommit
   MOZ_ASSERT(!mCommitStringByRequest);
 
   // If there are 2 or more pending compositions, we already sent
   // 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) {
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     mRequestIMEToCommitCompositionResults.
       AppendElement(RequestIMEToCommitCompositionResult::
                       eToOldCompositionReceived);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     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) {
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     mRequestIMEToCommitCompositionResults.
       AppendElement(RequestIMEToCommitCompositionResult::
                       eToCommittedCompositionReceived);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     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.
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     mRequestIMEToCommitCompositionResults.
       AppendElement(RequestIMEToCommitCompositionResult::
                       eReceivedAfterTabParentBlur);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     aCommittedString = mCompositionString;
     // After we return true from here, i.e., without actually requesting IME
     // to commit composition, we will receive eCompositionCommitRequestHandled
     // pseudo event message from the remote process.  So, we need to increment
     // mPendingEventsNeedingAck here.
     mPendingEventsNeedingAck++;
     return true;
   }
 
   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));
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     mRequestIMEToCommitCompositionResults.
       AppendElement(RequestIMEToCommitCompositionResult::
                       eReceivedButNoTextComposition);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     return false;
   }
 
   mCommitStringByRequest = &aCommittedString;
 
   // Request commit or cancel composition with TextComposition because we may
   // have already requested to commit or cancel the composition or we may
   // have already received eCompositionCommit(AsIs) event.  Those status are
@@ -1367,36 +1371,36 @@ ContentCacheInParent::RequestIMEToCommit
     // TextComposition instance will synthesize commit events and wait to
     // receive delayed composition events.  When TextComposition instances both
     // in this process and the remote process will be destroyed when delayed
     // composition events received. TextComposition instance in the parent
     // process will dispatch following composition events and be destroyed
     // normally. On the other hand, TextComposition instance in the remote
     // process won't dispatch following composition events and will be
     // destroyed by IMEStateManager::DispatchCompositionEvent().
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     mRequestIMEToCommitCompositionResults.
       AppendElement(RequestIMEToCommitCompositionResult::
                       eHandledAsynchronously);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
     return false;
   }
 
   // When the composition is committed synchronously, the commit string will be
   // returned to the remote process. Then, PuppetWidget will dispatch
   // eCompositionCommit event with the returned commit string (i.e., the value
   // is aCommittedString of this method).  Finally, TextComposition instance in
   // the remote process will be destroyed by
   // IMEStateManager::DispatchCompositionEvent() at receiving the
   // eCompositionCommit event (Note that TextComposition instance in this
   // process was already destroyed).
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
   mRequestIMEToCommitCompositionResults.
     AppendElement(RequestIMEToCommitCompositionResult::eHandledSynchronously);
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
   return true;
 }
 
 void
 ContentCacheInParent::MaybeNotifyIME(nsIWidget* aWidget,
                                      const IMENotification& aNotification)
 {
   if (!mPendingEventsNeedingAck) {
@@ -1483,17 +1487,17 @@ ContentCacheInParent::FlushPendingNotifi
       (mPendingTextChange.HasNotification() ||
        mPendingSelectionChange.HasNotification() ||
        mPendingLayoutChange.HasNotification() ||
        mPendingCompositionUpdate.HasNotification())) {
     FlushPendingNotifications(widget);
   }
 }
 
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
 void
 ContentCacheInParent::RemoveUnnecessaryEventMessageLog()
 {
   bool foundLastCompositionStart = false;
   for (size_t i = mDispatchedEventMessages.Length(); i > 1; i--) {
     if (mDispatchedEventMessages[i - 1] != eCompositionStart) {
       continue;
@@ -1572,17 +1576,17 @@ ContentCacheInParent::AppendEventMessage
          mRequestIMEToCommitCompositionResults) {
     aLog.AppendLiteral("  ");
     aLog.Append(ToReadableText(result));
     aLog.AppendLiteral("\n");
   }
   aLog.AppendLiteral("\n");
 }
 
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
 /*****************************************************************************
  * mozilla::ContentCache::TextRectArray
  *****************************************************************************/
 
 LayoutDeviceIntRect
 ContentCache::TextRectArray::GetRect(uint32_t aOffset) const
 {
--- a/widget/ContentCache.h
+++ b/widget/ContentCache.h
@@ -405,17 +405,17 @@ public:
                       const IMENotification& aNotification);
 
 private:
   IMENotification mPendingSelectionChange;
   IMENotification mPendingTextChange;
   IMENotification mPendingLayoutChange;
   IMENotification mPendingCompositionUpdate;
 
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
   // Log of event messages to be output to crash report.
   nsTArray<EventMessage> mDispatchedEventMessages;
   nsTArray<EventMessage> mReceivedEventMessages;
   // Log of RequestIMEToCommitComposition() in the last 2 compositions.
   enum class RequestIMEToCommitCompositionResult : uint8_t
   {
     eToOldCompositionReceived,
     eToCommittedCompositionReceived,
@@ -445,17 +445,17 @@ private:
       case RequestIMEToCommitCompositionResult::eHandledSynchronously:
         return "Commit reqeust is handled synchronously";
       default:
         return "Unknown reason";
     }
   }
   nsTArray<RequestIMEToCommitCompositionResult>
     mRequestIMEToCommitCompositionResults;
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
 
   // mTabParent is owner of the instance.
   dom::TabParent& MOZ_NON_OWNING_REF mTabParent;
   // mCompositionString is composition string which were sent to the remote
   // process but not yet committed in the remote process.
   nsString mCompositionString;
   // This is not nullptr only while the instance is requesting IME to
   // composition.  Then, data value of dispatched composition events should
@@ -502,25 +502,25 @@ private:
                    LayoutDeviceIntRect& aTextRect) const;
   bool GetUnionTextRects(uint32_t aOffset,
                          uint32_t aLength,
                          bool aRoundToExistingOffset,
                          LayoutDeviceIntRect& aUnionTextRect) const;
 
   void FlushPendingNotifications(nsIWidget* aWidget);
 
-#ifdef MOZ_CRASHREPORTER
+#if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
   /**
    * Remove unnecessary messages from mDispatchedEventMessages and
    * mReceivedEventMessages.
    */
   void RemoveUnnecessaryEventMessageLog();
 
   /**
    * Append event message log to aLog.
    */
   void AppendEventMessageLog(nsACString& aLog) const;
-#endif // #ifdef MOZ_CRASHREPORTER
+#endif // #if defined(MOZ_CRASHREPORTER) && MOZ_DIAGNOSTIC_ASSERT_ENABLED
 };
 
 } // namespace mozilla
 
 #endif // mozilla_ContentCache_h