[BGPT] Fix position:fixed NonFastScrollRegion

Blink generates NonFastScrollRegions to tell the compositor where there
are uncomposited scrollers and regions so that it knows to forward input
to the main thread. In https://crrev.com/618633b41898eb98e78cd9c9904fb2
I amended this code to store the regions for position:fixed layers in a
separate cc::Region and Layer. The linked bug is a result of a bug in
this new code.

The above CL didn't account for the fact that the rects are calculated
and stored in document coordinates. This means that when the regions are
recalculated while the page is scrolled, the fixed regions will be
pushed down on the inner viewport scroll layer, since the fixed layers
are now further down the page relative to the document origin. Scrolls
over a fixed layer now don't hit the intended region.

The reason the tests didn't catch this is because the regions are only
recalculated when invalidated. A compositor scroll doesn't invalidate
them so if nothing on the page changes after load, the issue doesn't
appear.

This CL fixes the issue by converting the fixed regions into root frame
coordinates, which is the coordinate space of the inner scrolling
layer. I've also fixed the layout tests to check that the rects remain
correct after a scroll when they're recalculated.

This CL relies on a prerequisite fix in https://crrev.com/c/1592637
which ensures we correctly determine whether a scroller is considered
"fixed" or "scrollable" with respect to the root frame.

Bug: 951271
Change-Id: Ib741fa95ae8a9c88afc2ae0f99f298679ea32e7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1589108Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#656232}

third_party/blink/renderer/core/frame/local_frame_view.cc

@@ -3109,6 +3109,12 @@ FloatPoint LocalFrameView::RootFrameToDocument(
   return local_frame + layout_viewport->GetScrollOffset();
 }
 
+IntRect LocalFrameView::DocumentToFrame(const IntRect& rect_in_document) const {
+  IntRect rect_in_frame = rect_in_document;
+  rect_in_frame.SetLocation(DocumentToFrame(rect_in_document.Location()));
+  return rect_in_frame;
+}
+
 DoublePoint LocalFrameView::DocumentToFrame(
     const DoublePoint& point_in_document) const {
   ScrollableArea* layout_viewport = LayoutViewport();
@@ -3118,6 +3124,11 @@ DoublePoint LocalFrameView::DocumentToFrame(
   return point_in_document - layout_viewport->GetScrollOffset();
 }
 
+IntPoint LocalFrameView::DocumentToFrame(
+    const IntPoint& point_in_document) const {
+  return FlooredIntPoint(DocumentToFrame(DoublePoint(point_in_document)));
+}
+
 FloatPoint LocalFrameView::DocumentToFrame(
     const FloatPoint& point_in_document) const {
   return FloatPoint(DocumentToFrame(DoublePoint(point_in_document)));
@@ -3139,6 +3150,10 @@ LayoutRect LocalFrameView::DocumentToFrame(
                     rect_in_document.Size());
 }
 
+IntPoint LocalFrameView::FrameToDocument(const IntPoint& point_in_frame) const {
+  return FlooredIntPoint(FrameToDocument(LayoutPoint(point_in_frame)));
+}
+
 LayoutPoint LocalFrameView::FrameToDocument(
     const LayoutPoint& point_in_frame) const {
   ScrollableArea* layout_viewport = LayoutViewport();
@@ -3148,6 +3163,11 @@ LayoutPoint LocalFrameView::FrameToDocument(
   return point_in_frame + LayoutSize(layout_viewport->GetScrollOffset());
 }
 
+IntRect LocalFrameView::FrameToDocument(const IntRect& rect_in_frame) const {
+  return IntRect(FrameToDocument(rect_in_frame.Location()),
+                 rect_in_frame.Size());
+}
+
 LayoutRect LocalFrameView::FrameToDocument(
     const LayoutRect& rect_in_frame) const {
   return LayoutRect(FrameToDocument(rect_in_frame.Location()),

third_party/blink/renderer/core/frame/local_frame_view.h

@@ -547,11 +547,15 @@ class CORE_EXPORT LocalFrameView final
   IntRect RootFrameToDocument(const IntRect&);
   IntPoint RootFrameToDocument(const IntPoint&);
   FloatPoint RootFrameToDocument(const FloatPoint&);
-  DoublePoint DocumentToFrame(const DoublePoint&) const;
+  IntPoint DocumentToFrame(const IntPoint&) const;
   FloatPoint DocumentToFrame(const FloatPoint&) const;
+  DoublePoint DocumentToFrame(const DoublePoint&) const;
   LayoutPoint DocumentToFrame(const LayoutPoint&) const;
+  IntRect DocumentToFrame(const IntRect&) const;
   LayoutRect DocumentToFrame(const LayoutRect&) const;
+  IntPoint FrameToDocument(const IntPoint&) const;
   LayoutPoint FrameToDocument(const LayoutPoint&) const;
+  IntRect FrameToDocument(const IntRect&) const;
   LayoutRect FrameToDocument(const LayoutRect&) const;
 
   // Normally a LocalFrameView synchronously paints during full lifecycle

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc

@@ -868,6 +868,8 @@ void ScrollingCoordinator::ComputeShouldHandleScrollGestureOnMainThreadRegion(
     return;
   }
 
+  LocalFrameView* local_root_view = frame->LocalFrameRoot().View();
+
   if (const LocalFrameView::ScrollableAreaSet* scrollable_areas =
           frame_view->ScrollableAreas()) {
     for (const ScrollableArea* scrollable_area : *scrollable_areas) {
@@ -875,12 +877,12 @@ void ScrollingCoordinator::ComputeShouldHandleScrollGestureOnMainThreadRegion(
       if (scrollable_area->UsesCompositedScrolling())
         continue;
 
-      Region* region = ScrollsWithRootFrame(scrollable_area->GetLayoutBox())
-                           ? scrolling_region
-                           : fixed_region;
-
-      IntRect box = scrollable_area->ScrollableAreaBoundingBox();
-      region->Unite(box);
+      if (ScrollsWithRootFrame(scrollable_area->GetLayoutBox())) {
+        scrolling_region->Unite(scrollable_area->ScrollableAreaBoundingBox());
+      } else {
+        fixed_region->Unite(local_root_view->DocumentToFrame(
+            scrollable_area->ScrollableAreaBoundingBox()));
+      }
     }
   }
 
@@ -894,20 +896,22 @@ void ScrollingCoordinator::ComputeShouldHandleScrollGestureOnMainThreadRegion(
       PaintLayerScrollableArea* scrollable_area =
           box->Layer()->GetScrollableArea();
 
-      Region* region = ScrollsWithRootFrame(scrollable_area->GetLayoutBox())
-                           ? scrolling_region
-                           : fixed_region;
-
-      IntRect bounds = box->AbsoluteBoundingBoxRect();
-      // Get the corner in local coords.
-      IntRect corner =
-          scrollable_area->ResizerCornerRect(bounds, kResizerForTouch);
-      // Map corner to top-frame coords.
-      corner = scrollable_area->GetLayoutBox()
-                   ->LocalToAbsoluteQuad(FloatRect(corner),
-                                         kTraverseDocumentBoundaries)
-                   .EnclosingBoundingBox();
-      region->Unite(corner);
+      IntRect bounds_in_frame = box->AbsoluteBoundingBoxRect();
+      IntRect corner_in_frame =
+          scrollable_area->ResizerCornerRect(bounds_in_frame, kResizerForTouch);
+
+      IntRect corner_in_root_frame =
+          scrollable_area->GetLayoutBox()
+              ->LocalToAbsoluteQuad(FloatRect(corner_in_frame),
+                                    kTraverseDocumentBoundaries)
+              .EnclosingBoundingBox();
+
+      if (ScrollsWithRootFrame(scrollable_area->GetLayoutBox())) {
+        scrolling_region->Unite(
+            local_root_view->FrameToDocument(corner_in_root_frame));
+      } else {
+        fixed_region->Unite(corner_in_root_frame);
+      }
     }
   }
 
@@ -918,13 +922,13 @@ void ScrollingCoordinator::ComputeShouldHandleScrollGestureOnMainThreadRegion(
     if (!element->GetLayoutObject())
       continue;
 
-    Region* region = ScrollsWithRootFrame(element->GetLayoutObject())
-                         ? scrolling_region
-                         : fixed_region;
-
     if (plugin->WantsWheelEvents()) {
       IntRect box = frame_view->ConvertToRootFrame(plugin->FrameRect());
-      region->Unite(box);
+      if (ScrollsWithRootFrame(element->GetLayoutObject())) {
+        scrolling_region->Unite(local_root_view->FrameToDocument(box));
+      } else {
+        fixed_region->Unite(box);
+      }
     }
   }
 

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc

@@ -1499,11 +1499,7 @@ TEST_P(ScrollingCoordinatorTest, NestedFixedIFramesMainThreadScrollingRegion) {
                                       "main frame is scrolled, it should "
                                       "not be placed in the scrolling region.";
 
-  // TODO(bokan): We have a bug in calculating the fixed rects, we use document
-  // coordinates but we should be using frame coordinates since they go on the
-  // inner viewport scroll layer which doesn't scroll with the document.
-  // EXPECT_EQ(fixed.Rects().at(0), IntRect(0, 20, 75, 75));
-  EXPECT_EQ(fixed.Bounds(), IntRect(0, 1020, 75, 75))
+  EXPECT_EQ(fixed.Bounds(), IntRect(0, 20, 75, 75))
       << "Since the DIV not move when the main frame is scrolled, it should be "
          "placed in the scrolling region.";
 }

third_party/blink/renderer/core/scroll/scrollable_area.h

@@ -273,7 +273,7 @@ class CORE_EXPORT ScrollableArea : public GarbageCollectedMixin {
   virtual bool ScrollbarsCanBeActive() const = 0;
 
   // Returns the bounding box of this scrollable area, in the coordinate system
-  // of the top-level FrameView.
+  // of the top-level FrameView's Document.
   virtual IntRect ScrollableAreaBoundingBox() const = 0;
 
   virtual CompositorElementId GetCompositorElementId() const = 0;

third_party/blink/web_tests/fast/scrolling/non-composited-scroller-in-position-fixed-scrolled-document.html

@@ -103,6 +103,29 @@
                             "Scroller should be scrolled.");
       }
 
+      // Now mutate the scroller so that we force a recompute of its non-fast
+      // scrollable region.
+      {
+        scroller.style.height = "151px";
+        await waitForCompositorCommit();
+      }
+
+      // Again perform a scroll over the scroller rect. Ensure we targetted the
+      // correct scroller.
+      {
+        const delta = 100;
+        const location = elementCenter(scroller);
+        await smoothScroll(delta,
+                           location.x,
+                           location.y,
+                           GestureSourceType.TOUCH_INPUT,
+                           'up',
+                           SPEED_INSTANT);
+
+        assert_equals(scroller.scrollTop, 0,
+                      "Scroller should be scrolled to the top.");
+      }
+
     }, 'Scrolling over an uncomposited scroller inside a composited position' +
        ': fixed element.');
   }

third_party/blink/web_tests/fast/scrolling/non-composited-scroller-in-position-fixed-scroller-scrolled-document.html

@@ -106,6 +106,29 @@
                             "Scroller should be scrolled.");
       }
 
+      // Now mutate the scroller so that we force a recompute of its non-fast
+      // scrollable region.
+      {
+        scroller.style.height = "151px";
+        await waitForCompositorCommit();
+      }
+
+      // Perform another scroll to ensure the recalculated region is still
+      // correct after the scroll.
+      {
+        const delta = 100;
+        const location = elementCenter(scroller);
+        await smoothScroll(delta,
+                           location.x,
+                           location.y,
+                           GestureSourceType.TOUCH_INPUT,
+                           'up',
+                           SPEED_INSTANT);
+
+        assert_equals(scroller.scrollTop, 0,
+                      "Scroller should be scrolled to the top.");
+      }
+
     }, 'Scrolling over an uncomposited scroller inside a composited position' +
        ': fixed element.');
   }