Bug 1535945 - Don't skip invalidating frames when creating displayports for async scrollable ancestors. r=tnikkel

Differential Revision: https://phabricator.services.mozilla.com/D23816 --HG-- extra : moz-landing-system : lando
2019-05-01 04:23:07 +00:00 · 2019-05-01 04:23:07 +00:00 · 3fc4716479
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@ -263,7 +263,7 @@ static void SetDisplayPortMargins(PresShell* aPresShell, nsIContent* aContent,
                                       aDisplayPortMargins, 0);
  if (!hadDisplayPort) {
    nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-        aContent->GetPrimaryFrame(), nsLayoutUtils::RepaintMode::Repaint);
+        aContent->GetPrimaryFrame());
  }

  nsRect base(0, 0, aDisplayPortBase.width * AppUnitsPerCSSPixel(),
@ -422,11 +422,10 @@ void APZCCallbackHelper::InitializeRootDisplayport(PresShell* aPresShell) {
    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());
  }
 }

@ -736,8 +735,7 @@ static bool PrepareForSetTargetAPZCNotification(
  }

  nsIFrame* frame = do_QueryFrame(scrollAncestor);
-  nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-      frame, nsLayoutUtils::RepaintMode::Repaint);
+  nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(frame);

  return true;
 }
--- a/layout/base/GeckoMVMContext.cpp
+++ b/layout/base/GeckoMVMContext.cpp
@ -156,7 +156,7 @@ void GeckoMVMContext::UpdateDisplayPortMargins() {
                                              displayportBase);
    nsIScrollableFrame* scrollable = do_QueryFrame(root);
    nsLayoutUtils::CalculateAndSetDisplayPortMargins(
-        scrollable, nsLayoutUtils::RepaintMode::DoNotRepaint);
+        scrollable, nsLayoutUtils::RepaintMode::Repaint);
  }
 }

--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@ -3329,10 +3329,8 @@ static nscoord ComputeWhereToScroll(WhereToScroll aWhereToScroll,
 * 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());

--- a/layout/base/crashtests/1535945.html
+++ 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
@ -565,6 +565,7 @@ 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
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@ -3346,7 +3346,7 @@ nsIScrollableFrame* nsLayoutUtils::GetAsyncScrollableAncestorFrame(
 }

 void nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
-    nsIFrame* aFrame, RepaintMode aRepaintMode) {
+    nsIFrame* aFrame) {
  nsIFrame* frame = aFrame;
  while (frame) {
    frame = nsLayoutUtils::GetCrossDocParentFrame(frame);
@ -3365,7 +3365,7 @@ void nsLayoutUtils::SetZeroMarginDisplayPortOnAsyncScrollableAncestors(
        !nsLayoutUtils::HasDisplayPort(frame->GetContent())) {
      nsLayoutUtils::SetDisplayPortMargins(frame->GetContent(),
                                           frame->PresShell(), ScreenMargin(),
-                                           0, aRepaintMode);
+                                           0, RepaintMode::Repaint);
    }
  }
 }
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@ -2830,7 +2830,7 @@ class nsLayoutUtils {
   * 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
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@ -2041,10 +2041,8 @@ ScrollFrameHelper::ScrollFrameHelper(nsContainerFrame* aOuter, bool aIsRoot)
    // 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);
  }
 }

@ -3641,7 +3639,11 @@ void ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                 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);
@ -3889,7 +3891,8 @@ bool ScrollFrameHelper::DecideScrollableLayer(
    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;
@ -3928,6 +3931,10 @@ bool ScrollFrameHelper::DecideScrollableLayer(
  // 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 =
@ -7070,11 +7077,9 @@ void ScrollFrameHelper::ApzSmoothScrollTo(const nsPoint& aDestination,
    // 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