[root-scroller] Don't promote when ancestor is scrollable

If we have a candidate implicit root scroller, we should only promote it
if it doesn't have a scrollable ancestor. The reason for this is that we
end scroll chaining at the root scroller. This is necessary as the root
scroller consumes all scroll in the form of overscroll effects. A (user)
scrollable ancestor would be broken in this case, as shown in the
example on the attached bug.

Note: overflow in an ancestor is still allowed, as long as its overflow
style is hidden or visible.

Bug: 837007
Change-Id: Ied7a9f02eab76c4ea59fced94d0804ba47ef4e4a
Reviewed-on: https://chromium-review.googlesource.com/c/1351590Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611245}

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

@@ -88,21 +88,6 @@ PaintLayerScrollableArea* GetScrollableArea(const Element& element) {
   return ToLayoutBox(element.GetLayoutObject())->GetScrollableArea();
 }
 
-bool ScrollsVerticalOverflow(LayoutView& layout_view) {
-  DCHECK(layout_view.GetScrollableArea());
-
-  if (layout_view.Size().IsZero() ||
-      !layout_view.GetScrollableArea()->HasVerticalOverflow() ||
-      !layout_view.ScrollsOverflowY())
-    return false;
-
-  ScrollbarMode h_mode;
-  ScrollbarMode v_mode;
-  layout_view.CalculateScrollbarModes(h_mode, v_mode);
-
-  return v_mode != kScrollbarAlwaysOff;
-}
-
 }  // namespace
 
 // static
@@ -307,7 +292,29 @@ bool RootScrollerController::IsValidImplicit(const Element& element) const {
   if (!scrollable_area)
     return false;
 
-  return scrollable_area->ScrollsOverflow();
+  if (!scrollable_area->ScrollsOverflow())
+    return false;
+
+  // If any of the ancestors are user scrollable (i.e. overflow == scroll|auto)
+  // then don't promote as we'd likely break intended scrolling by stopping
+  // chaining at this scroller.
+  for (LayoutBox* ancestor = element.GetLayoutObject()->ContainingBlock();
+       ancestor; ancestor = ancestor->ContainingBlock()) {
+    const ComputedStyle* style = ancestor->Style();
+    if (!style)
+      continue;
+
+    PaintLayerScrollableArea* area = ancestor->GetScrollableArea();
+    if (!area)
+      continue;
+
+    if ((style->ScrollsOverflowX() && area->HasHorizontalOverflow()) ||
+        (style->ScrollsOverflowY() && area->HasVerticalOverflow())) {
+      return false;
+    }
+  }
+
+  return true;
 }
 
 void RootScrollerController::ApplyRootScrollerProperties(Node& node) {
@@ -377,11 +384,6 @@ void RootScrollerController::ProcessImplicitCandidates() {
   if (!document_->GetFrame()->IsMainFrame())
     return;
 
-  // If the main document has vertical scrolling, that's a good sign we
-  // shouldn't implicitly promote anything.
-  if (ScrollsVerticalOverflow(*document_->GetLayoutView()))
-    return;
-
   bool multiple_matches = false;
 
   HeapHashSet<WeakMember<Element>> copy(implicit_candidates_);

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

@@ -2621,11 +2621,149 @@ TEST_F(ImplicitRootScrollerSimTest, OverflowInMainDocumentRestrictsImplicit) {
                                ASSERT_NO_EXCEPTION);
   Compositor().BeginFrame();
 
+  EXPECT_EQ(GetDocument(),
+            GetDocument().GetRootScrollerController().EffectiveRootScroller())
+      << "iframe still shouldn't be promoted due to horizontal overflow in "
+      << "the main document.";
+
+  spacer->style()->setProperty(&GetDocument(), "width", "100%", String(),
+                               ASSERT_NO_EXCEPTION);
+  Compositor().BeginFrame();
+
   EXPECT_EQ(GetDocument().getElementById("container"),
             GetDocument().GetRootScrollerController().EffectiveRootScroller())
       << "Once vertical overflow is removed, the iframe should be promoted.";
 }
 
+// Test that we overflow in an ancestor is ok as long as it is overflow:hidden.
+TEST_F(ImplicitRootScrollerSimTest, OverflowHiddenDoesntRestrictImplicit) {
+  WebView().ResizeWithBrowserControls(IntSize(800, 600), 50, 0, true);
+  SimRequest main_request("https://example.com/test.html", "text/html");
+  SimRequest child_request("https://example.com/child.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  main_request.Complete(R"HTML(
+          <!DOCTYPE html>
+          <style>
+            ::-webkit-scrollbar {
+              width: 0px;
+              height: 0px;
+            }
+            html {
+              overflow: hidden;
+            }
+            body, html {
+              width: 100%;
+              height: 100%;
+              margin: 0px;
+            }
+            iframe {
+              width: 100%;
+              height: 100%;
+              border: 0;
+            }
+            #spacer {
+              position: absolute;
+              left: 0;
+              top: 0;
+              height: 150%;
+              width: 150%;
+            }
+          </style>
+          <iframe id="container" src="child.html">
+          </iframe>
+          <div id="spacer"></div>
+      )HTML");
+  child_request.Complete(R"HTML(
+        <!DOCTYPE html>
+        <style>
+          body {
+            height: 1000px;
+          }
+        </style>
+  )HTML");
+
+  Compositor().BeginFrame();
+  EXPECT_EQ(GetDocument().getElementById("container"),
+            GetDocument().GetRootScrollerController().EffectiveRootScroller())
+      << "iframe should be promoted since document's overflow is hidden.";
+
+  Element* html = GetDocument().documentElement();
+  html->style()->setProperty(&GetDocument(), "overflow", "auto", String(),
+                             ASSERT_NO_EXCEPTION);
+  Compositor().BeginFrame();
+
+  EXPECT_EQ(GetDocument(),
+            GetDocument().GetRootScrollerController().EffectiveRootScroller())
+      << "iframe should now be demoted since main document scrolls overflow.";
+}
+
+// Test that we user-scrollable overflow in an ancestor prevents implicit
+// promotion.
+TEST_F(ImplicitRootScrollerSimTest, UserScrollableAncestorPreventsPromotion) {
+  WebView().ResizeWithBrowserControls(IntSize(800, 600), 50, 0, true);
+  SimRequest main_request("https://example.com/test.html", "text/html");
+  SimRequest child_request("https://example.com/child.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  main_request.Complete(R"HTML(
+          <!DOCTYPE html>
+          <style>
+            ::-webkit-scrollbar {
+              width: 0px;
+              height: 0px;
+            }
+            html {
+              overflow: hidden;
+            }
+            body, html {
+              width: 100%;
+              height: 100%;
+              margin: 0px;
+            }
+            iframe {
+              width: 100%;
+              height: 100%;
+              border: 0;
+            }
+            #ancestor {
+              width: 100%;
+              height: 100%;
+              overflow: scroll;
+              opacity: 0.5;
+            }
+            #spacer {
+              height: 150%;
+              width: 150%;
+            }
+          </style>
+          <div id="ancestor">
+            <iframe id="container" src="child.html"></iframe>
+            <div id="spacer"></div>
+          </div>
+      )HTML");
+  child_request.Complete(R"HTML(
+        <!DOCTYPE html>
+        <style>
+          body {
+            height: 1000px;
+          }
+        </style>
+  )HTML");
+
+  Compositor().BeginFrame();
+  EXPECT_EQ(GetDocument(),
+            GetDocument().GetRootScrollerController().EffectiveRootScroller())
+      << "iframe should not promote since ancestor div scrolls overflow.";
+
+  Element* ancestor = GetDocument().getElementById("ancestor");
+  ancestor->style()->setProperty(&GetDocument(), "overflow", "hidden", String(),
+                                 ASSERT_NO_EXCEPTION);
+  Compositor().BeginFrame();
+
+  EXPECT_EQ(GetDocument().getElementById("container"),
+            GetDocument().GetRootScrollerController().EffectiveRootScroller())
+      << "iframe should now be promoted since ancestor's overflow is hidden.";
+}
+
 TEST_F(ImplicitRootScrollerSimTest, AppliedAtFractionalZoom) {
   // Matches Pixel 2XL screen size of 412x671 at 3.5 DevicePixelRatio.
   WebView().SetZoomFactorForDeviceScaleFactor(3.5f);