Bug 1549812 - Try to assert a bit harder about stuff not flushing under our nose. r=TYLin,mats
☠☠ backed out by c81a6ac1b894 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 17 May 2019 13:22:39 +0000
changeset 536229 00afc705d4eef04a1d71cea44953d0ba232a3794
parent 536228 cbc5c04bd3e4fb6cbfa04dc02f3f1e3cfd6c05a1
child 536230 b63f61a97f1776a0909655817e6391cfcfe09324
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersTYLin, mats
bugs1549812
milestone68.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 1549812 - Try to assert a bit harder about stuff not flushing under our nose. r=TYLin,mats I think these should hold, everything that runs under them should just schedule other stuff to some later date: * Synth mouse events -> scheduled as refresh driver observers. * Scroll events -> Scheduled as well. * Caret state change events -> Also scheduled after last patch. * IME and accessibility stuff -> I don't think they can reenter layout. We can always revert this if it causes troubles, plus it shouldn't crash on release so should be fine. Differential Revision: https://phabricator.services.mozilla.com/D31090
layout/base/AccessibleCaretManager.cpp
layout/base/PresShell.cpp
layout/generic/nsGfxScrollFrame.cpp
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -581,16 +581,21 @@ nsresult AccessibleCaretManager::SelectW
 }
 
 void AccessibleCaretManager::OnScrollStart() {
   AC_LOG("%s", __FUNCTION__);
 
   AutoRestore<bool> saveAllowFlushingLayout(mAllowFlushingLayout);
   mAllowFlushingLayout = false;
 
+  Maybe<PresShell::AutoAssertNoFlush> assert;
+  if (mPresShell) {
+    assert.emplace(*mPresShell);
+  }
+
   mIsScrollStarted = true;
 
   if (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible()) {
     // Dispatch the event only if one of the carets is logically visible like in
     // HideCarets().
     DispatchCaretStateChangedEvent(CaretChangedReason::Scroll);
   }
 }
@@ -598,16 +603,21 @@ void AccessibleCaretManager::OnScrollSta
 void AccessibleCaretManager::OnScrollEnd() {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
   AutoRestore<bool> saveAllowFlushingLayout(mAllowFlushingLayout);
   mAllowFlushingLayout = false;
 
+  Maybe<PresShell::AutoAssertNoFlush> assert;
+  if (mPresShell) {
+    assert.emplace(*mPresShell);
+  }
+
   mIsScrollStarted = false;
 
   if (GetCaretMode() == CaretMode::Cursor) {
     if (!mFirstCaret->IsLogicallyVisible()) {
       // If the caret is hidden (Appearance::None) due to blur, no
       // need to update it.
       return;
     }
@@ -628,16 +638,21 @@ void AccessibleCaretManager::OnScrollEnd
 void AccessibleCaretManager::OnScrollPositionChanged() {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
   AutoRestore<bool> saveAllowFlushingLayout(mAllowFlushingLayout);
   mAllowFlushingLayout = false;
 
+  Maybe<PresShell::AutoAssertNoFlush> assert;
+  if (mPresShell) {
+    assert.emplace(*mPresShell);
+  }
+
   if (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible()) {
     if (mIsScrollStarted) {
       // We don't want extra CaretStateChangedEvents dispatched when user is
       // scrolling the page.
       AC_LOG("%s: UpdateCarets(RespectOldAppearance | DispatchNoEvent)",
              __FUNCTION__);
       UpdateCarets({UpdateCaretsHint::RespectOldAppearance,
                     UpdateCaretsHint::DispatchNoEvent});
@@ -651,16 +666,21 @@ void AccessibleCaretManager::OnScrollPos
 void AccessibleCaretManager::OnReflow() {
   if (mLastUpdateCaretMode != GetCaretMode()) {
     return;
   }
 
   AutoRestore<bool> saveAllowFlushingLayout(mAllowFlushingLayout);
   mAllowFlushingLayout = false;
 
+  Maybe<PresShell::AutoAssertNoFlush> assert;
+  if (mPresShell) {
+    assert.emplace(*mPresShell);
+  }
+
   if (mFirstCaret->IsLogicallyVisible() || mSecondCaret->IsLogicallyVisible()) {
     AC_LOG("%s: UpdateCarets(RespectOldAppearance)", __FUNCTION__);
     UpdateCarets(UpdateCaretsHint::RespectOldAppearance);
   }
 }
 
 void AccessibleCaretManager::OnBlur() {
   AC_LOG("%s: HideCarets()", __FUNCTION__);
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -9052,18 +9052,18 @@ void PresShell::WillDoReflow() {
   mPresContext->FlushFontFeatureValues();
 
   mLastReflowStart = GetPerformanceNowUnclamped();
 }
 
 void PresShell::DidDoReflow(bool aInterruptible) {
   HandlePostedReflowCallbacks(aInterruptible);
 
-  nsCOMPtr<nsIDocShell> docShell = mPresContext->GetDocShell();
-  if (docShell) {
+  AutoAssertNoFlush noReentrantFlush(*this);
+  if (nsCOMPtr<nsIDocShell> docShell = mPresContext->GetDocShell()) {
     DOMHighResTimeStamp now = GetPerformanceNowUnclamped();
     docShell->NotifyReflowObservers(aInterruptible, mLastReflowStart, now);
   }
 
   if (!mPresContext->HasPendingInterrupt()) {
     mDocument->ScheduleResizeObserversNotification();
   }
 
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -2912,16 +2912,18 @@ void ScrollFrameHelper::ScrollToImpl(nsP
     mScrolledFrame->UpdateOverflow();
 
     // Update the overflow for the outer so that we recompute scrollbars.
     mOuter->UpdateOverflow();
   }
 
   ScheduleSyntheticMouseMove();
 
+  PresShell::AutoAssertNoFlush noFlush(*mOuter->PresShell());
+
   {  // scope the AutoScrollbarRepaintSuppression
     AutoScrollbarRepaintSuppression repaintSuppression(this, !schedulePaint);
     AutoWeakFrame weakFrame(mOuter);
     UpdateScrollbarPosition();
     if (!weakFrame.IsAlive()) {
       return;
     }
   }
@@ -2941,18 +2943,17 @@ void ScrollFrameHelper::ScrollToImpl(nsP
     }
   }
 
   // notify the listeners.
   for (uint32_t i = 0; i < mListeners.Length(); i++) {
     mListeners[i]->ScrollPositionDidChange(pt.x, pt.y);
   }
 
-  nsCOMPtr<nsIDocShell> docShell = presContext->GetDocShell();
-  if (docShell) {
+  if (nsCOMPtr<nsIDocShell> docShell = presContext->GetDocShell()) {
     docShell->NotifyScrollObservers();
   }
 }
 
 static Maybe<int32_t> MaxZIndexInList(nsDisplayList* aList,
                                       nsDisplayListBuilder* aBuilder) {
   Maybe<int32_t> maxZIndex = Nothing();
   for (nsDisplayItem* item = aList->GetBottom(); item;