Fix main thread viewport pan from outside root scroller

This CL fixes a bug when scrolling from outside the root scroller
hierarchy. The root scroller is the "main" scroller on a page. On
Android, this can be designated on an element other than the document.
However, it's still possible to scroll a chain of elements outside of
the root scroller subtree. For example, from a sibling of the root
scroller or from a |position: fixed| element. In these cases, when the
scroll chains to the root, we still want to scroll the visual viewport
so that we support panning around if the user is zoomed in. We don't do
this on the main thread prior to this CL.

Typically, the VisualViewport is scrolled by using the RootFrameViewport
class. However, in this case we can't use it since that would cause
scrolling in the root scroller (and thus change behavior based on
whether the page is viewed using a UA that supports root scroller). This
CL adds a special case to Node::NativeApplyScroll. When we construct a
chain that doesn't end at the root scroller, we'll always add the root
LayoutView to the scroll chain. When scrolling it, we'll attempt to
scroll the visual viewport if the document itself can no longer scroll.

Note: we have an equivalent special case in
LayerTreeHostImpl::ApplyScroll where we can pass a bool to
viewport()->ScrollBy() to specify that we should scroll only the visual
viewport. This behavior is somewhat more natural in CC since the scroll
chain walks the ScrollTree which contains the visual viewport. Blink's
scrolling code assumes all scrollable nodes are DOM nodes which isn't
true of the visual viewport; hence the need to make this a special case
of the root document node.

This was implemented in CC in https://crrev.com/c/1752866. At the time,
it was assumed this case would be exceedingly rare on the main thread so
it was left unimplemented. However, fixing bug 1011866 will result in
this case becoming more common.

Bug: 977954, 1011866
Change-Id: I7d2d9cb70b66b54a8c972af4bdc6d69978fe690b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1859940Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707222}

third_party/blink/renderer/core/dom/node.cc

@@ -88,6 +88,7 @@
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/frame/local_frame_client.h"
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
+#include "third_party/blink/renderer/core/frame/visual_viewport.h"
 #include "third_party/blink/renderer/core/fullscreen/fullscreen.h"
 #include "third_party/blink/renderer/core/html/custom/custom_element.h"
 #include "third_party/blink/renderer/core/html/html_dialog_element.h"
@@ -544,13 +545,39 @@ void Node::NativeApplyScroll(ScrollState& scroll_state) {
   ScrollableArea* scrollable_area =
       box_to_scroll->EnclosingBox()->GetScrollableArea();
 
-  if (!scrollable_area)
+  // TODO(bokan): This is a hack to fix https://crbug.com/977954. If we have a
+  // non-default root scroller, scrolling from one of its siblings or a fixed
+  // element will chain up to the root node without passing through the root
+  // scroller. This should scroll the visual viewport (so we can still pan
+  // while zoomed) but not by using the RootFrameViewport, which would cause
+  // scrolling in the root scroller element. Implementing this on the main
+  // thread is awkward since we assume only Nodes are scrollable but the
+  // VisualViewport isn't a Node. See LTHI::ApplyScroll for the equivalent
+  // behavior in CC.
+  bool also_scroll_visual_viewport = GetDocument().GetFrame() &&
+                                     GetDocument().GetFrame()->IsMainFrame() &&
+                                     box_to_scroll->IsLayoutView();
+  DCHECK(!also_scroll_visual_viewport ||
+         !box_to_scroll->IsGlobalRootScroller());
+
+  if (!scrollable_area) {
+    // The LayoutView should always create a ScrollableArea.
+    DCHECK(!also_scroll_visual_viewport);
     return;
+  }
 
   ScrollResult result = scrollable_area->UserScroll(
       ScrollGranularity(static_cast<int>(scroll_state.deltaGranularity())),
       delta, ScrollableArea::ScrollCallback());
 
+  // Also try scrolling the visual viewport if we're at the end of the scroll
+  // chain.
+  if (!result.DidScroll() && also_scroll_visual_viewport) {
+    result = GetDocument().GetPage()->GetVisualViewport().UserScroll(
+        ScrollGranularity(static_cast<int>(scroll_state.deltaGranularity())),
+        delta, ScrollableArea::ScrollCallback());
+  }
+
   if (!result.DidScroll())
     return;
 

third_party/blink/renderer/core/input/scroll_manager.cc

@@ -209,6 +209,17 @@ bool ScrollManager::CanScroll(const ScrollState& scroll_state,
   if (current_node.GetLayoutBox()->IsGlobalRootScroller())
     return true;
 
+  // If this is the main LayoutView, and it's not the root scroller, that means
+  // we have a non-default root scroller on the page. In this case, attempts to
+  // scroll the LayoutView should cause panning of the visual viewport as well
+  // so ensure it gets added to the scroll chain. See LTHI::ApplyScroll for the
+  // equivalent behavior in CC. Node::NativeApplyScroll contains a special
+  // handler for this case.
+  if (current_node.GetLayoutBox()->IsLayoutView() &&
+      current_node.GetDocument().GetFrame()->IsMainFrame()) {
+    return true;
+  }
+
   ScrollableArea* scrollable_area =
       current_node.GetLayoutBox()->GetScrollableArea();
 

third_party/blink/web_tests/TestExpectations

@@ -2218,11 +2218,6 @@ crbug.com/249112 external/wpt/css/css-flexbox/flex-minimum-width-flex-items-007.
 crbug.com/336604 external/wpt/css/css-flexbox/flexbox_visibility-collapse-line-wrapping.html [ Failure ]
 crbug.com/336604 external/wpt/css/css-flexbox/flexbox_visibility-collapse.html [ Failure ]
 
-# Broken for main thread scrolling only. Not worth fixing in the short term but
-# should be addressed by scroll unification.
-crbug.com/977954 rootscroller/fixed-chaining-with-implicit.html [ Failure ]
-crbug.com/977954 rootscroller/fixed-scroller-chaining-with-implicit.html [ Failure ]
-
 # Requires support for hit-testing based on flex order.
 crbug.com/844505 external/wpt/css/css-flexbox/hittest-overlapping-order.html [ Failure ]