[ScrollUnification] Fix LayerTreeHostTest

This CL updates expectations for tests in
layer_tree_host_unittest_scroll to pass when the scroll unification flag
is turned on. We also run each LayerTreeHostTest with both states of the
flag. Due to the use of macros, using a parameterized test is difficult
so we do this manually inside the macro definition.

These changes are mostly straightforward updates to expectations
returned from ScrollBegin. Additionally, we replace instances of
TryScroll with ScrollBegin as the former will go away after unification
and we shouldn't be directly calling this internal method anyway. Its
only external callers were these tests so this method is also made
private.

LayerTreeHostScrollTestScrollZeroMaxScrollOffset gets some significant
changes to the test structure. This multi-step test was posting a
SetNeedsCommit to the main thread which causes a main frame; the test
step was implicitly tracked in the frame number. By updating to use
ScrollBegin/ScrollEnd, the test became racy (under TSan) because
ScrollEnd calls SetNeedsCommit as well and can in some cases
synchronously post a task to the main thread. This meant that in some
cases we might post two MBF which can commit before the first is drawn.
The issue is solved by manually tracking the step and requesting the
commit from the main frame.

NonScrollingNonFastScrollableRegion also required a fix; it mixed up the
container and scrollable bounds on the scrolling node so that the node
wasn't actually scrollable. The test passed without unification because
we fallback to the viewport node when we can't scroll any other node;
the test wasn't checking which node is scrolling, only that one was.
The fallback doesn't work in unification because we (correctly) expect
that all scrolls will chain up through the viewport node. In this case,
the test also didn't correctly parent the scroll node so this is fixed
as well by making the layers parent from the outer viewport scroll
layer.

Bug: 1086625
Change-Id: I3c0c6b5f9e976c038f537fe50e1861b12d51acf5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2223088
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774604}

cc/test/layer_tree_test.h

@@ -10,6 +10,7 @@
 #include "base/threading/thread.h"
 #include "build/build_config.h"
 #include "cc/animation/animation_delegate.h"
+#include "cc/base/features.h"
 #include "cc/test/property_tree_test_utils.h"
 #include "cc/test/test_hooks.h"
 #include "cc/test/test_task_graph_runner.h"
@@ -313,21 +314,31 @@ class LayerTreeTest : public testing::Test, public TestHooks {
 // a specific test name. eg.
 // // TODO(crbug.com/abcd): Disabled for some reasons stated here.
 // // SINGLE_AND_MULTI_THREAD_TEST_F(SomeRandomTest)
-#define SINGLE_THREAD_TEST_F(TEST_FIXTURE_NAME)                   \
-  TEST_F(TEST_FIXTURE_NAME, RunSingleThread_DelegatingRenderer) { \
-    RunTest(CompositorMode::SINGLE_THREADED);                     \
-  }                                                               \
+#define SINGLE_THREAD_TEST_F(TEST_FIXTURE_NAME)                                \
+  TEST_F(TEST_FIXTURE_NAME, RunSingleThread_DelegatingRenderer) {              \
+    RunTest(CompositorMode::SINGLE_THREADED);                                  \
+  }                                                                            \
+  TEST_F(TEST_FIXTURE_NAME, RunSingleThread_DelegatingRendererUnifiedScroll) { \
+    base::test::ScopedFeatureList scoped_feature_list;                         \
+    scoped_feature_list.InitAndEnableFeature(features::kScrollUnification);    \
+    RunTest(CompositorMode::SINGLE_THREADED);                                  \
+  }                                                                            \
   class SingleThreadDelegatingImplNeedsSemicolon##TEST_FIXTURE_NAME {}
 
 // Do not change this macro to disable a test, it will disable half of
 // the unit test suite. Instead, comment out the usage of this macro for
 // a specific test name. eg.
 // // TODO(crbug.com/abcd): Disabled for some reasons stated here.
 // // SINGLE_AND_MULTI_THREAD_TEST_F(SomeRandomTest)
-#define MULTI_THREAD_TEST_F(TEST_FIXTURE_NAME)                   \
-  TEST_F(TEST_FIXTURE_NAME, RunMultiThread_DelegatingRenderer) { \
-    RunTest(CompositorMode::THREADED);                           \
-  }                                                              \
+#define MULTI_THREAD_TEST_F(TEST_FIXTURE_NAME)                                \
+  TEST_F(TEST_FIXTURE_NAME, RunMultiThread_DelegatingRenderer) {              \
+    RunTest(CompositorMode::THREADED);                                        \
+  }                                                                           \
+  TEST_F(TEST_FIXTURE_NAME, RunMultiThread_DelegatingRendererUnifiedScroll) { \
+    base::test::ScopedFeatureList scoped_feature_list;                        \
+    scoped_feature_list.InitAndEnableFeature(features::kScrollUnification);   \
+    RunTest(CompositorMode::THREADED);                                        \
+  }                                                                           \
   class MultiThreadDelegatingImplNeedsSemicolon##TEST_FIXTURE_NAME {}
 
 // Do not change this macro to disable a test, it will disable half of

cc/trees/layer_tree_host_impl.h

@@ -797,16 +797,6 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler,
                : task_runner_provider_->MainThreadTaskRunner();
   }
 
-  // Determines whether the given scroll node can scroll on the compositor
-  // thread or if there are any reasons it must be scrolled on the main thread
-  // or not at all. Note: in general, this is not sufficient to determine if a
-  // scroll can occur on the compositor thread. If hit testing to a scroll
-  // node, the caller must also check whether the hit point intersects a
-  // non-fast-scrolling-region of any ancestor scrolling layers. Can be removed
-  // after scroll unification https://crbug.com/476553.
-  InputHandler::ScrollStatus TryScroll(const ScrollTree& scroll_tree,
-                                       ScrollNode* scroll_node) const;
-
   // Return all ScrollNode indices that have an associated layer with a non-fast
   // region that intersects the point.
   base::flat_set<int> NonFastScrollableNodes(
@@ -915,6 +905,16 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler,
   // scroll chaining rules.
   ScrollNode* GetNodeToScroll(ScrollNode* node) const;
 
+  // Determines whether the given scroll node can scroll on the compositor
+  // thread or if there are any reasons it must be scrolled on the main thread
+  // or not at all. Note: in general, this is not sufficient to determine if a
+  // scroll can occur on the compositor thread. If hit testing to a scroll
+  // node, the caller must also check whether the hit point intersects a
+  // non-fast-scrolling-region of any ancestor scrolling layers. Can be removed
+  // after scroll unification https://crbug.com/476553.
+  InputHandler::ScrollStatus TryScroll(const ScrollTree& scroll_tree,
+                                       ScrollNode* scroll_node) const;
+
   // Transforms viewport start point and scroll delta to local start point and
   // local delta, respectively. If the transformation of either the start or end
   // point of a scroll is clipped, the function returns false.