Bug 1553772 - Bug 1549812 - Try to assert a bit harder about stuff not flushing under our nose. r=TYLin,mats
☠☠ backed out by 781c2bb366d3 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 23 May 2019 09:45:56 +0000
changeset 475173 7286e18fbc1775558b8653bb63576a47b31a5f3e
parent 475172 58d40da713559f200d7ea651f2df0c434acee108
child 475174 7cce412dd61b40d0ceda37ed32a64dda7d7b5e8d
push id36057
push useraciure@mozilla.com
push dateThu, 23 May 2019 21:52:03 +0000
treeherdermozilla-central@d551d37b9ad0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersTYLin, mats
bugs1553772, 1549812
milestone69.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 1553772 - 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
@@ -9059,18 +9059,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
@@ -2903,16 +2903,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;
     }
   }
@@ -2932,18 +2934,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;