Bug 1251150. Add crash annotations if image visibility is re-entering. r=mats
authorTimothy Nikkel <tnikkel@gmail.com>
Tue, 29 Mar 2016 23:09:13 -0500
changeset 291012 6bf6edf222a15276ca24855fd9f6bae559595524
parent 291011 39a20fcf9976b408e309b68703945d063b22b41e
child 291013 06a8c115f8fa8a253b867bf798ac376a168418b5
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1251150
milestone48.0a1
Bug 1251150. Add crash annotations if image visibility is re-entering. r=mats The previous annotations only checked if the re-entrancy happened via DecrementVisibleCount. The check in RebuildApproximateFrameVisibility is not needed because we add a check in DoUpdateApproximateFrameVisibility. The check in ClearApproximatelyVisibleFramesList is not needed because we add checks in DoUpdateApproximateFrameVisibility and Destroy. The other caller, ClearApproximateFrameVisibilityVisited, is covered because it is only called from DoUpdateApproximateFrameVisibility.
layout/base/nsPresShell.cpp
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -1167,20 +1167,28 @@ PresShell::Destroy()
 
   if (mDelayedPaintTimer) {
     mDelayedPaintTimer->Cancel();
     mDelayedPaintTimer = nullptr;
   }
 
   mSynthMouseMoveEvent.Revoke();
 
+  if (mInFrameVisibilityUpdate) {
+    gfxCriticalNoteOnce << "Destroy is re-entering on "
+                        << (NS_IsMainThread() ? "" : "non-") << "main thread";
+  }
+  mInFrameVisibilityUpdate = true;
+
   mUpdateApproximateFrameVisibilityEvent.Revoke();
 
   ClearApproximatelyVisibleFramesList(Some(OnNonvisible::DISCARD_IMAGES));
 
+  mInFrameVisibilityUpdate = false;
+
   if (mCaret) {
     mCaret->Terminate();
     mCaret = nullptr;
   }
 
   if (mSelection) {
     mSelection->DisconnectFromPresShell();
   }
@@ -5713,26 +5721,22 @@ PresShell::DecApproximateVisibleCount(Vi
     nsIFrame* frame = iter.Get()->GetKey();
 
     if (MOZ_UNLIKELY(!frame)) {
       // We are about to crash, annotate crash report with some info that might
       // help debug the crash (bug 1251150)
       ReportBadStateDuringVisibilityUpdate();
     }
 
-    SetInFrameVisibilityUpdate(true);
-
     // Decrement the frame's visible count if we're still tracking its
     // visibility. (We may not be, if the frame disabled visibility tracking
     // after we added it to the visible frames list.)
     if (frame->TrackingVisibility()) {
       frame->DecApproximateVisibleCount(aNonvisibleAction);
     }
-
-    SetInFrameVisibilityUpdate(false);
   }
 }
 
 void
 PresShell::RebuildApproximateFrameVisibilityDisplayList(const nsDisplayList& aList)
 {
   MOZ_ASSERT(!mApproximateFrameVisibilityVisited, "already visited?");
   mApproximateFrameVisibilityVisited = true;
@@ -5773,20 +5777,16 @@ PresShell::ClearApproximateFrameVisibili
     ClearApproximateFrameVisibilityVisited(v, v->GetViewManager() != vm);
   }
 }
 
 void
 PresShell::ClearApproximatelyVisibleFramesList(Maybe<OnNonvisible> aNonvisibleAction
                                                  /* = Nothing() */)
 {
-  if (mInFrameVisibilityUpdate) {
-    gfxCriticalNoteOnce << "ClearApproximatelyVisibleFramesList is re-entering on "
-                        << (NS_IsMainThread() ? "" : "non-") << "main thread";
-  }
   DecApproximateVisibleCount(mApproximatelyVisibleFrames, aNonvisibleAction);
   mApproximatelyVisibleFrames.Clear();
 }
 
 void
 PresShell::MarkFramesInSubtreeApproximatelyVisible(nsIFrame* aFrame,
                                                    const nsRect& aRect,
                                                    Maybe<VisibleRegions>& aVisibleRegions,
@@ -5886,21 +5886,16 @@ PresShell::RebuildApproximateFrameVisibi
   MOZ_ASSERT(!mApproximateFrameVisibilityVisited, "already visited?");
   mApproximateFrameVisibilityVisited = true;
 
   nsIFrame* rootFrame = GetRootFrame();
   if (!rootFrame) {
     return;
   }
 
-  if (mInFrameVisibilityUpdate) {
-    gfxCriticalNoteOnce << "RebuildApproximateFrameVisibility is re-entering on "
-                        << (NS_IsMainThread() ? "" : "non-") << "main thread";
-  }
-
   // Remove the entries of the mApproximatelyVisibleFrames hashtable and put
   // them in oldApproximatelyVisibleFrames.
   VisibleFrames oldApproximatelyVisibleFrames;
   mApproximatelyVisibleFrames.SwapElements(oldApproximatelyVisibleFrames);
 
   // If we're visualizing visible regions, create a VisibleRegions object to
   // store information about them. The functions we call will populate this
   // object and send it to the compositor only if it's Some(), so we don't
@@ -5931,30 +5926,40 @@ PresShell::UpdateApproximateFrameVisibil
 void
 PresShell::DoUpdateApproximateFrameVisibility(bool aRemoveOnly)
 {
   MOZ_ASSERT(!mPresContext || mPresContext->IsRootContentDocument(),
              "Updating approximate frame visibility on a non-root content document?");
 
   mUpdateApproximateFrameVisibilityEvent.Revoke();
 
+  if (mInFrameVisibilityUpdate) {
+    gfxCriticalNoteOnce << "DoUpdateApproximateFrameVisibility is re-entering on "
+                        << (NS_IsMainThread() ? "" : "non-") << "main thread";
+  }
+  mInFrameVisibilityUpdate = true;
+
   if (mHaveShutDown || mIsDestroying) {
+    mInFrameVisibilityUpdate = false;
     return;
   }
 
   // call update on that frame
   nsIFrame* rootFrame = GetRootFrame();
   if (!rootFrame) {
     ClearApproximatelyVisibleFramesList(Some(OnNonvisible::DISCARD_IMAGES));
+    mInFrameVisibilityUpdate = false;
     return;
   }
 
   RebuildApproximateFrameVisibility(/* aRect = */ nullptr, aRemoveOnly);
   ClearApproximateFrameVisibilityVisited(rootFrame->GetView(), true);
 
+  mInFrameVisibilityUpdate = false;
+
 #ifdef DEBUG_FRAME_VISIBILITY_DISPLAY_LIST
   // This can be used to debug the frame walker by comparing beforeFrameList
   // and mApproximatelyVisibleFrames in RebuildFrameVisibilityDisplayList to see if
   // they produce the same results (mApproximatelyVisibleFrames holds the frames the
   // display list thinks are visible, beforeFrameList holds the frames the
   // frame walker thinks are visible).
   nsDisplayListBuilder builder(rootFrame, nsDisplayListBuilder::FRAME_VISIBILITY, false);
   nsRect updateRect(nsPoint(0, 0), rootFrame->GetSize());
@@ -6085,22 +6090,25 @@ PresShell::EnsureFrameInApproximatelyVis
     MOZ_ASSERT(!shell || shell == this, "wrong shell");
   }
 #endif
 
   if (mInFrameVisibilityUpdate) {
     gfxCriticalNoteOnce << "EnsureFrameInApproximatelyVisibleList is re-entering on "
                         << (NS_IsMainThread() ? "" : "non-") << "main thread";
   }
+  mInFrameVisibilityUpdate = true;
 
   if (!mApproximatelyVisibleFrames.Contains(aFrame)) {
     MOZ_ASSERT(!AssumeAllFramesVisible());
     mApproximatelyVisibleFrames.PutEntry(aFrame);
     aFrame->IncApproximateVisibleCount();
   }
+
+  mInFrameVisibilityUpdate = false;
 }
 
 void
 PresShell::RemoveFrameFromApproximatelyVisibleList(nsIFrame* aFrame)
 {
 #ifdef DEBUG
   // Make sure it's in this pres shell.
   nsCOMPtr<nsIContent> content = aFrame->GetContent();
@@ -6115,26 +6123,29 @@ PresShell::RemoveFrameFromApproximatelyV
                "Shouldn't have any frames in the table");
     return;
   }
 
   if (mInFrameVisibilityUpdate) {
     gfxCriticalNoteOnce << "RemoveFrameFromApproximatelyVisibleList is re-entering on "
                         << (NS_IsMainThread() ? "" : "non-") << "main thread";
   }
+  mInFrameVisibilityUpdate = true;
 
   uint32_t count = mApproximatelyVisibleFrames.Count();
   mApproximatelyVisibleFrames.RemoveEntry(aFrame);
 
   if (aFrame->TrackingVisibility() &&
       mApproximatelyVisibleFrames.Count() < count) {
     // aFrame was in the hashtable, and we're still tracking its visibility,
     // so we need to decrement its visible count.
     aFrame->DecApproximateVisibleCount();
   }
+
+  mInFrameVisibilityUpdate = false;
 }
 
 class nsAutoNotifyDidPaint
 {
 public:
   nsAutoNotifyDidPaint(PresShell* aShell, uint32_t aFlags)
     : mShell(aShell), mFlags(aFlags)
   {
@@ -10896,25 +10907,27 @@ PresShell::UpdateImageLockingState()
 
   nsresult rv = mDocument->SetImageLockingState(locked);
 
   if (locked) {
     if (mInFrameVisibilityUpdate) {
       gfxCriticalNoteOnce << "UpdateImageLockingState is re-entering on "
                           << (NS_IsMainThread() ? "" : "non-") << "main thread";
     }
+    mInFrameVisibilityUpdate = true;
 
     // Request decodes for visible image frames; we want to start decoding as
     // quickly as possible when we get foregrounded to minimize flashing.
     for (auto iter = mApproximatelyVisibleFrames.Iter(); !iter.Done(); iter.Next()) {
       nsImageFrame* imageFrame = do_QueryFrame(iter.Get()->GetKey());
       if (imageFrame) {
         imageFrame->MaybeDecodeForPredictedSize();
       }
     }
+    mInFrameVisibilityUpdate = false;
   }
 
   return rv;
 }
 
 PresShell*
 PresShell::GetRootPresShell()
 {