Bug 1535945 - Don't skip invalidating frames when creating displayports for async scrollable ancestors. r=tnikkel
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 01 May 2019 04:23:07 +0000
changeset 472054 0e259884f052230b24601efc3cde3a5ef5d4e6ad
parent 472053 732976d3f555a01d9032ede89a28121502ee49e2
child 472055 25e1607e6f1e5345cccb8eb501b7cbdad182f9fb
push id35946
push userapavel@mozilla.com
push dateWed, 01 May 2019 15:54:31 +0000
treeherdermozilla-central@a027a998b8b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1535945
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 1535945 - Don't skip invalidating frames when creating displayports for async scrollable ancestors. r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D23816
gfx/layers/apz/util/APZCCallbackHelper.cpp
layout/base/GeckoMVMContext.cpp
layout/base/PresShell.cpp
layout/base/crashtests/1535945.html
layout/base/crashtests/crashtests.list
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/generic/nsGfxScrollFrame.cpp
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -258,17 +258,17 @@ static void SetDisplayPortMargins(PresSh
     return;
   }
 
   bool hadDisplayPort = nsLayoutUtils::HasDisplayPort(aContent);
   nsLayoutUtils::SetDisplayPortMargins(aContent, aPresShell,
                                        aDisplayPortMargins, 0);
   if (!hadDisplayPort) {
     nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-        aContent->GetPrimaryFrame(), nsLayoutUtils::RepaintMode::Repaint);
+        aContent->GetPrimaryFrame());
   }
 
   nsRect base(0, 0, aDisplayPortBase.width * AppUnitsPerCSSPixel(),
               aDisplayPortBase.height * AppUnitsPerCSSPixel());
   nsLayoutUtils::SetDisplayPortBaseIfNotSet(aContent, base);
 }
 
 static void SetPaintRequestTime(nsIContent* aContent,
@@ -417,21 +417,20 @@ void APZCCallbackHelper::InitializeRootD
       baseRect = nsRect(nsPoint(0, 0),
                         nsLayoutUtils::CalculateCompositionSizeForFrame(frame));
     } else if (pc) {
       baseRect = nsRect(nsPoint(0, 0), pc->GetVisibleArea().Size());
     }
     nsLayoutUtils::SetDisplayPortBaseIfNotSet(content, baseRect);
     // Note that we also set the base rect that goes with these margins in
     // nsRootBoxFrame::BuildDisplayList.
-    nsLayoutUtils::SetDisplayPortMargins(
-        content, aPresShell, ScreenMargin(), 0,
-        nsLayoutUtils::RepaintMode::DoNotRepaint);
+    nsLayoutUtils::SetDisplayPortMargins(content, aPresShell, ScreenMargin(),
+                                         0);
     nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-        content->GetPrimaryFrame(), nsLayoutUtils::RepaintMode::DoNotRepaint);
+        content->GetPrimaryFrame());
   }
 }
 
 nsPresContext* APZCCallbackHelper::GetPresContextForContent(
     nsIContent* aContent) {
   dom::Document* doc = aContent->GetComposedDoc();
   if (!doc) {
     return nullptr;
@@ -731,18 +730,17 @@ static bool PrepareForSetTargetAPZCNotif
              dpElement.get());
   bool activated = nsLayoutUtils::CalculateAndSetDisplayPortMargins(
       scrollAncestor, nsLayoutUtils::RepaintMode::Repaint);
   if (!activated) {
     return false;
   }
 
   nsIFrame* frame = do_QueryFrame(scrollAncestor);
-  nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-      frame, nsLayoutUtils::RepaintMode::Repaint);
+  nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(frame);
 
   return true;
 }
 
 static void SendLayersDependentApzcTargetConfirmation(
     PresShell* aPresShell, uint64_t aInputBlockId,
     const nsTArray<SLGuidAndRenderRoot>& aTargets) {
   LayerManager* lm = aPresShell->GetLayerManager();
--- a/layout/base/GeckoMVMContext.cpp
+++ b/layout/base/GeckoMVMContext.cpp
@@ -151,17 +151,17 @@ void GeckoMVMContext::UpdateDisplayPortM
     // We only create MobileViewportManager for root content documents. If that
     // ever changes we'd need to limit the size of this displayport base rect
     // because non-toplevel documents have no limit on their size.
     MOZ_ASSERT(mPresShell->GetPresContext()->IsRootContentDocument());
     nsLayoutUtils::SetDisplayPortBaseIfNotSet(root->GetContent(),
                                               displayportBase);
     nsIScrollableFrame* scrollable = do_QueryFrame(root);
     nsLayoutUtils::CalculateAndSetDisplayPortMargins(
-        scrollable, nsLayoutUtils::RepaintMode::DoNotRepaint);
+        scrollable, nsLayoutUtils::RepaintMode::Repaint);
   }
 }
 
 void GeckoMVMContext::Reflow(const CSSSize& aNewSize, const CSSSize& aOldSize,
                              ResizeEventFlag aResizeEventFlag) {
   MOZ_ASSERT(mPresShell);
 
   ResizeReflowOptions reflowOptions = ResizeReflowOptions::NoOption;
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -3324,20 +3324,18 @@ static nscoord ComputeWhereToScroll(Wher
  *
  * Note that, since we are performing a layout scroll, it's possible that
  * this fnction will sometimes be unsuccessful; the content will move as
  * fast as it can on the screen using layout viewport scrolling, and then
  * stop there, even if it could get closer to the desired position by
  * moving the visual viewport within the layout viewport.
  */
 static void ScrollToShowRect(nsIScrollableFrame* aFrameAsScrollable,
-                             const nsRect& aRect,
-                             ScrollAxis aVertical,
-                             ScrollAxis aHorizontal,
-                             ScrollFlags aScrollFlags) {
+                             const nsRect& aRect, ScrollAxis aVertical,
+                             ScrollAxis aHorizontal, ScrollFlags aScrollFlags) {
   nsPoint scrollPt = aFrameAsScrollable->GetVisualViewportOffset();
   nsRect visibleRect(scrollPt, aFrameAsScrollable->GetVisualViewportSize());
 
   nsSize lineSize;
   // Don't call GetLineScrollAmount unless we actually need it. Not only
   // does this save time, but it's not safe to call GetLineScrollAmount
   // during reflow (because it depends on font size inflation and doesn't
   // use the in-reflow-safe font-size inflation path). If we did call it,
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1535945.html
@@ -0,0 +1,26 @@
+<html class="reftest-wait">
+<style>
+.c {
+  scroll-behavior: smooth;
+  border-style: groove;
+}
+* {
+  margin-right: 52vh;
+  width: 1vh
+}
+:not(feTile) {
+  overflow-x: scroll;
+}
+</style>
+<script>
+function go() {
+  a.scrollTo({left: 1, top: 80})
+  document.documentElement.removeAttribute("class");
+}
+</script>
+<body onload=go()>
+<table background="A">
+A
+<dl>
+<dd id="a" contenteditable="true" class="c">
+</html>
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -560,12 +560,13 @@ pref(layout.css.column-span.enabled,true
 pref(layout.css.column-span.enabled,true) load 1507244.html
 load 1510080.html
 load 1510485.html
 pref(layout.css.column-span.enabled,true) load 1511535.html
 load 1511563.html
 pref(layout.css.column-span.enabled,true) load 1524382.html
 pref(layout.css.column-span.enabled,true) load 1533885.html
 pref(layout.css.column-span.enabled,true) load 1534146.html
+load 1535945.html
 pref(layout.css.column-span.enabled,true) load 1539017.html
 load 1539303.html
 pref(layout.css.column-span.enabled,true) load 1541679.html
 load 1547261.html
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -3341,17 +3341,17 @@ nsIScrollableFrame* nsLayoutUtils::GetAs
     nsIFrame* aTarget) {
   uint32_t flags = nsLayoutUtils::SCROLLABLE_ALWAYS_MATCH_ROOT |
                    nsLayoutUtils::SCROLLABLE_ONLY_ASYNC_SCROLLABLE |
                    nsLayoutUtils::SCROLLABLE_FIXEDPOS_FINDS_ROOT;
   return nsLayoutUtils::GetNearestScrollableFrame(aTarget, flags);
 }
 
 void nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-    nsIFrame* aFrame, RepaintMode aRepaintMode) {
+    nsIFrame* aFrame) {
   nsIFrame* frame = aFrame;
   while (frame) {
     frame = nsLayoutUtils::GetCrossDocParentFrame(frame);
     if (!frame) {
       break;
     }
     nsIScrollableFrame* scrollAncestor = GetAsyncScrollableAncestorFrame(frame);
     if (!scrollAncestor) {
@@ -3360,17 +3360,17 @@ void nsLayoutUtils::SetZeroMarginDisplay
     frame = do_QueryFrame(scrollAncestor);
     MOZ_ASSERT(frame);
     MOZ_ASSERT(scrollAncestor->WantAsyncScroll() ||
                frame->PresShell()->GetRootScrollFrame() == frame);
     if (nsLayoutUtils::AsyncPanZoomEnabled(frame) &&
         !nsLayoutUtils::HasDisplayPort(frame->GetContent())) {
       nsLayoutUtils::SetDisplayPortMargins(frame->GetContent(),
                                            frame->PresShell(), ScreenMargin(),
-                                           0, aRepaintMode);
+                                           0, RepaintMode::Repaint);
     }
   }
 }
 
 bool nsLayoutUtils::MaybeCreateDisplayPortInFirstScrollFrameEncountered(
     nsIFrame* aFrame, nsDisplayListBuilder& aBuilder) {
   nsIScrollableFrame* sf = do_QueryFrame(aFrame);
   if (sf) {
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -2825,17 +2825,17 @@ class nsLayoutUtils {
 
   static nsIScrollableFrame* GetAsyncScrollableAncestorFrame(nsIFrame* aTarget);
 
   /**
    * Sets a zero margin display port on all proper ancestors of aFrame that
    * are async scrollable.
    */
   static void SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-      nsIFrame* aFrame, RepaintMode aRepaintMode);
+      nsIFrame* aFrame);
   /**
    * Finds the closest ancestor async scrollable frame from aFrame that has a
    * displayport and attempts to trigger the displayport expiry on that
    * ancestor.
    */
   static void ExpireDisplayPortOnAsyncScrollableAncestor(nsIFrame* aFrame);
 
   static bool IsOutlineStyleAutoEnabled();
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -2036,20 +2036,18 @@ ScrollFrameHelper::ScrollFrameHelper(nsC
   EnsureFrameVisPrefsCached();
 
   if (IsAlwaysActive() && gfxPrefs::LayersTilesEnabled() &&
       !nsLayoutUtils::UsesAsyncScrolling(mOuter) && mOuter->GetContent()) {
     // If we have tiling but no APZ, then set a 0-margin display port on
     // active scroll containers so that we paint by whole tile increments
     // when scrolling.
     nsLayoutUtils::SetDisplayPortMargins(
-        mOuter->GetContent(), mOuter->PresShell(), ScreenMargin(), 0,
-        nsLayoutUtils::RepaintMode::DoNotRepaint);
-    nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-        mOuter, nsLayoutUtils::RepaintMode::DoNotRepaint);
+        mOuter->GetContent(), mOuter->PresShell(), ScreenMargin(), 0);
+    nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(mOuter);
   }
 }
 
 ScrollFrameHelper::~ScrollFrameHelper() {
   if (mScrollEvent) {
     mScrollEvent->Revoke();
   }
   if (mScrollEndEvent) {
@@ -3636,17 +3634,21 @@ void ScrollFrameHelper::BuildDisplayList
       // handoff chain which is subject to the out-of-flow-frames caveat noted
       // above (where the idSetter variable is created).
       //
       // This is not compatible when using containes for root scrollframes.
       MOZ_ASSERT(couldBuildLayer && mScrolledFrame->GetContent() &&
                  aBuilder->IsPaintingToWindow());
       if (!mWillBuildScrollableLayer) {
         // Set a displayport so next paint we don't have to force layerization
-        // after the fact.
+        // after the fact. It's ok to pass DoNotRepaint here, since we've
+        // already painted the change and we're just optimizing it to be
+        // detected earlier. We also won't confuse RetainedDisplayLists
+        // with the silent change, since we explicitly request partial updates
+        // to be disabled on the next paint.
         nsLayoutUtils::SetDisplayPortMargins(
             mOuter->GetContent(), mOuter->PresShell(), ScreenMargin(), 0,
             nsLayoutUtils::RepaintMode::DoNotRepaint);
         // Call DecideScrollableLayer to recompute mWillBuildScrollableLayer
         // and recompute the current animated geometry root if needed. It's
         // too late to change the dirty rect so pass a copy.
         nsRect copyOfDirtyRect = dirtyRect;
         nsRect copyOfVisibleRect = visibleRect;
@@ -3884,17 +3886,18 @@ bool ScrollFrameHelper::DecideScrollable
     MOZ_ASSERT(content->GetProperty(nsGkAtoms::DisplayPortBase));
     nsRect displayPort;
     usingDisplayPort = nsLayoutUtils::GetDisplayPort(content, &displayPort,
                                                      RelativeTo::ScrollFrame);
 
     if (usingDisplayPort) {
       // Override the dirty rectangle if the displayport has been set.
       *aVisibleRect = displayPort;
-      if (!aBuilder->IsPartialUpdate() || aBuilder->InInvalidSubtree()) {
+      if (!aBuilder->IsPartialUpdate() || aBuilder->InInvalidSubtree() ||
+          mOuter->IsFrameModified()) {
         *aDirtyRect = displayPort;
         if (aDirtyRectHasBeenOverriden) {
           *aDirtyRectHasBeenOverriden = true;
         }
       } else if (mOuter->HasOverrideDirtyRegion()) {
         nsRect* rect = mOuter->GetProperty(
             nsDisplayListBuilder::DisplayListBuildingDisplayPortRect());
         if (rect) {
@@ -3923,16 +3926,20 @@ bool ScrollFrameHelper::DecideScrollable
   // so that APZ can implement scroll grabbing.
   mWillBuildScrollableLayer =
       usingDisplayPort || nsContentUtils::HasScrollgrab(content);
 
   // The cached animated geometry root for the display builder is out of
   // date if we just introduced a new animated geometry root.
   if (oldWillBuildScrollableLayer != mWillBuildScrollableLayer) {
     aBuilder->RecomputeCurrentAnimatedGeometryRoot();
+    MOZ_DIAGNOSTIC_ASSERT(!aBuilder->IsPartialUpdate() ||
+                              aBuilder->InInvalidSubtree() ||
+                              mOuter->IsFrameModified(),
+                          "Displayport changed without an invalidation");
   }
 
   mIsScrollableLayerInRootContainer =
       gfxPrefs::LayoutUseContainersForRootFrames() &&
       mWillBuildScrollableLayer && mIsRoot;
   return mWillBuildScrollableLayer;
 }
 
@@ -7065,21 +7072,19 @@ void ScrollFrameHelper::ApzSmoothScrollT
   mScrollGeneration = ++sScrollGenerationCounter;
 
   if (!nsLayoutUtils::HasDisplayPort(mOuter->GetContent())) {
     // If this frame doesn't have a displayport then there won't be an
     // APZC instance for it and so there won't be anything to process
     // this smooth scroll request. We should set a displayport on this
     // frame to force an APZC which can handle the request.
     nsLayoutUtils::CalculateAndSetDisplayPortMargins(
-        mOuter->GetScrollTargetFrame(),
-        nsLayoutUtils::RepaintMode::DoNotRepaint);
+        mOuter->GetScrollTargetFrame(), nsLayoutUtils::RepaintMode::Repaint);
     nsIFrame* frame = do_QueryFrame(mOuter->GetScrollTargetFrame());
-    nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-        frame, nsLayoutUtils::RepaintMode::DoNotRepaint);
+    nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(frame);
   }
 
   // Schedule a paint to ensure that the frame metrics get updated on
   // the compositor thread.
   mOuter->SchedulePaint();
 }
 
 bool ScrollFrameHelper::SmoothScrollVisual(