Bug 1597348: When reflow is interrupted, purge flex items' cached measurements during the same traversal that we use to mark ancestor-chain as dirty. r=emilio
authorDaniel Holbert <dholbert@cs.stanford.edu>
Mon, 18 Nov 2019 19:19:42 +0000
changeset 502457 3ec96c0dd69655f71c67e434e589fe745dc05200
parent 502456 a52abf84721ad2f0ff2f56b8f2667ab46cc716b3
child 502458 65114b5cb9ee40f8f8a168e56b090ddad54f9465
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1597348
milestone72.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 1597348: When reflow is interrupted, purge flex items' cached measurements during the same traversal that we use to mark ancestor-chain as dirty. r=emilio This means we no longer have any use for the frame state bit "NS_STATE_FLEX_MEASUREMENTS_INTERRUPTED". Now, if a flex container has N children and only the last child is interrupted, we'll only purge the last child's measurement (and we'll do it promptly at the end of the whole interrupted reflow). Differential Revision: https://phabricator.services.mozilla.com/D53687
layout/base/PresShell.cpp
layout/generic/nsFlexContainerFrame.cpp
layout/generic/nsFrameStateBits.h
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -52,16 +52,17 @@
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/PointerEventHandler.h"
 #include "mozilla/dom/PopupBlocker.h"
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/DocumentInlines.h"
 #include "mozilla/dom/UserActivation.h"
 #include "nsAnimationManager.h"
 #include "nsNameSpaceManager.h"  // for Pref-related rule management (bugs 22963,20760,31816)
+#include "nsFlexContainerFrame.h"
 #include "nsFrame.h"
 #include "FrameLayerBuilder.h"
 #include "nsViewManager.h"
 #include "nsView.h"
 #include "nsCRTGlue.h"
 #include "prinrval.h"
 #include "nsTArray.h"
 #include "nsCOMArray.h"
@@ -9231,16 +9232,19 @@ bool PresShell::DoReflow(nsIFrame* targe
   if (interrupted) {
     // Make sure target gets reflowed again.
     for (auto iter = mFramesToDirty.Iter(); !iter.Done(); iter.Next()) {
       // Mark frames dirty until target frame.
       nsPtrHashKey<nsIFrame>* p = iter.Get();
       for (nsIFrame* f = p->GetKey(); f && !NS_SUBTREE_DIRTY(f);
            f = f->GetParent()) {
         f->AddStateBits(NS_FRAME_HAS_DIRTY_CHILDREN);
+        if (f->IsFlexItem()) {
+          nsFlexContainerFrame::MarkCachedFlexMeasurementsDirty(f);
+        }
 
         if (f == target) {
           break;
         }
       }
     }
 
     NS_ASSERTION(NS_SUBTREE_DIRTY(target), "Why is the target not dirty?");
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -1789,29 +1789,18 @@ void nsFlexContainerFrame::MarkCachedFle
     nsIFrame* aItemFrame) {
   aItemFrame->DeleteProperty(CachedFlexMeasuringReflow());
 }
 
 const CachedMeasuringReflowResult&
 nsFlexContainerFrame::MeasureAscentAndBSizeForFlexItem(
     FlexItem& aItem, nsPresContext* aPresContext,
     ReflowInput& aChildReflowInput) {
-  if (HasAnyStateBits(NS_STATE_FLEX_MEASUREMENTS_INTERRUPTED) &&
-      !aPresContext->HasPendingInterrupt()) {
-    // Our measurements are from an earlier reflow which was interrupted.
-    // (and the current reflow is not [yet] interrupted, so we have a chance
-    // to maybe get a more accurate measurement now).
-    // Purge our potentially-invalid item measurements.
-    for (nsIFrame* frame : mFrames) {
-      frame->DeleteProperty(CachedFlexMeasuringReflow());
-    }
-    RemoveStateBits(NS_STATE_FLEX_MEASUREMENTS_INTERRUPTED);
-    FLEX_LOG("[perf] MeasureAscentAndBSizeForFlexItem purged cached values");
-  } else if (const auto* cachedResult =
-                 aItem.Frame()->GetProperty(CachedFlexMeasuringReflow())) {
+  if (const auto* cachedResult =
+          aItem.Frame()->GetProperty(CachedFlexMeasuringReflow())) {
     if (cachedResult->IsValidFor(aChildReflowInput)) {
       return *cachedResult;
     }
     FLEX_LOG("[perf] MeasureAscentAndBSizeForFlexItem rejected cached value");
   } else {
     FLEX_LOG(
         "[perf] MeasureAscentAndBSizeForFlexItem didn't have a cached value");
   }
@@ -1831,30 +1820,16 @@ nsFlexContainerFrame::MeasureAscentAndBS
              "We gave flex item unconstrained available height, so it "
              "should be complete");
 
   // Tell the child we're done with its initial reflow.
   // (Necessary for e.g. GetBaseline() to work below w/out asserting)
   FinishReflowChild(aItem.Frame(), aPresContext, childDesiredSize,
                     &aChildReflowInput, 0, 0, flags);
 
-  // If we got an interrupt during or before that measuring reflow, we make a
-  // note that this & other cached measurements are potentially invalid,
-  // because our descendant block frames' reflows may have bailed out early due
-  // to the interrupt.  We'll keep these invalid measurements for the rest of
-  // this reflow (to avoid repeating the same bogus measurement), and purge
-  // them on the next (non-interrupted) reflow.
-  //
-  // TODO(emilio): Can we do this only for the kids that are interrupted? We
-  // probably want to figure out what the right thing to do here is regarding
-  // interrupts, see bug 1495532.
-  if (aPresContext->HasPendingInterrupt()) {
-    AddStateBits(NS_STATE_FLEX_MEASUREMENTS_INTERRUPTED);
-  }
-
   auto result =
       new CachedMeasuringReflowResult(aChildReflowInput, childDesiredSize);
 
   aItem.Frame()->SetProperty(CachedFlexMeasuringReflow(), result);
   return *result;
 }
 
 /* virtual */
@@ -5264,16 +5239,18 @@ void nsFlexContainerFrame::ReflowFlexIte
   // internally call InitResizeFlags and stomp on mVResize & mHResize.
 
   ReflowOutput childDesiredSize(childReflowInput);
   nsReflowStatus childReflowStatus;
   ReflowChild(aItem.Frame(), aPresContext, childDesiredSize, childReflowInput,
               outerWM, aFramePos, aContainerSize, ReflowChildFlags::Default,
               childReflowStatus);
 
+  // XXXdholbert Perhaps we should call CheckForInterrupt here; see bug 1495532.
+
   // XXXdholbert Once we do pagination / splitting, we'll need to actually
   // handle incomplete childReflowStatuses. But for now, we give our kids
   // unconstrained available height, which means they should always
   // complete.
   MOZ_ASSERT(childReflowStatus.IsComplete(),
              "We gave flex item unconstrained available height, so it "
              "should be complete");
 
--- a/layout/generic/nsFrameStateBits.h
+++ b/layout/generic/nsFrameStateBits.h
@@ -349,24 +349,16 @@ FRAME_STATE_BIT(FlexContainer, 22, NS_ST
 
 // True if the container has no flex items; may lie if there is a pending reflow
 FRAME_STATE_BIT(FlexContainer, 23, NS_STATE_FLEX_SYNTHESIZE_BASELINE)
 
 // True if any flex item in the container has a line with a
 // -webkit-line-ellipsis marker.
 FRAME_STATE_BIT(FlexContainer, 24, NS_STATE_FLEX_HAS_LINE_CLAMP_ELLIPSIS)
 
-// True if the flex container's items' cached measurements are potentially
-// invalid, due to them having been computed during a reflow that was
-// interrupted.  (We may still use those invalid measurements for the rest of
-// the interrupted reflow; but as soon as we need a cached measurement in a
-// non-interrupted reflow, this bit should make us purge our flex items'
-// measurements and remeasure.)
-FRAME_STATE_BIT(FlexContainer, 25, NS_STATE_FLEX_MEASUREMENTS_INTERRUPTED)
-
 // == Frame state bits that apply to grid container frames ====================
 
 FRAME_STATE_GROUP(GridContainer, nsGridContainerFrame)
 
 // True iff the normal flow children are already in CSS 'order' in the
 // order they occur in the child frame list.
 FRAME_STATE_BIT(GridContainer, 20,
                 NS_STATE_GRID_NORMAL_FLOW_CHILDREN_IN_CSS_ORDER)