Cleanup FakeLayerTreeHost's initialization of Impl

As part of splitting out input from LayerTreeHostImpl I found some
issues in tests that use FakeLayerTreeHost. This class also initializes
a FakeLayerTreeHostImpl which is used in some tests.

However, some tests also call InitializeSingleThreaded to use the
FakeLayerTreeHost with a Proxy. Initializing a Proxy will call
LayerTreeHost::CreateLayerTreeHostImpl which currently creates a (real)
LayerTreeHostImpl that's owned by the Proxy. These tests are incorrectly
running with multiple LTHI objects.

This CL overrides CreateLayerTreeHostImpl in the fake so that this
method creates a fake Impl object. We also avoid manually creating a
fake Impl inside the FakeLayerTreeHost and instead rely on the call
to CreateLayerTreeHostImpl to store a pointer to the fake impl.

For tests that don't use a Proxy, we now manually call a new
CreateFakeLayerTreeHostImpl method that does the same thing and the
FakeLayerTreeHost takes ownership of the Impl instead.

Bug: 915926
Change-Id: I63202e1717d2eab026688d69c8d0ef75939c7c47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363247Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799390}

cc/layers/solid_color_layer_impl_unittest.cc

@@ -177,6 +177,7 @@ TEST_F(SolidColorLayerImplTest, VerifyNeedsBlending) {
   auto animation_host = AnimationHost::CreateForTesting(ThreadInstance::MAIN);
   std::unique_ptr<FakeLayerTreeHost> host = FakeLayerTreeHost::Create(
       &client, &task_graph_runner, animation_host.get());
+  host->CreateFakeLayerTreeHostImpl();
   host->SetRootLayer(root);
 
   UpdateDrawProperties(host.get());

cc/test/fake_layer_tree_host.cc

@@ -4,6 +4,9 @@
 
 #include "cc/test/fake_layer_tree_host.h"
 
+#include <memory>
+#include <utility>
+
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -19,7 +22,6 @@ FakeLayerTreeHost::FakeLayerTreeHost(FakeLayerTreeHostClient* client,
                                      CompositorMode mode)
     : LayerTreeHost(std::move(params), mode),
       client_(client),
-      host_impl_(GetSettings(), &task_runner_provider_, task_graph_runner()),
       needs_commit_(false) {
   scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner =
       mode == CompositorMode::THREADED ? base::ThreadTaskRunnerHandle::Get()
@@ -67,6 +69,22 @@ FakeLayerTreeHost::~FakeLayerTreeHost() {
 
 void FakeLayerTreeHost::SetNeedsCommit() { needs_commit_ = true; }
 
+std::unique_ptr<LayerTreeHostImpl> FakeLayerTreeHost::CreateLayerTreeHostImpl(
+    LayerTreeHostImplClient* client) {
+  DCHECK(!host_impl_);
+  auto host_impl = std::make_unique<FakeLayerTreeHostImpl>(
+      GetSettings(), &task_runner_provider(), task_graph_runner());
+  host_impl_ = host_impl.get();
+  return host_impl;
+}
+
+void FakeLayerTreeHost::CreateFakeLayerTreeHostImpl() {
+  DCHECK(!host_impl_);
+  owned_host_impl_ = std::make_unique<FakeLayerTreeHostImpl>(
+      GetSettings(), &task_runner_provider(), task_graph_runner());
+  host_impl_ = owned_host_impl_.get();
+}
+
 LayerImpl* FakeLayerTreeHost::CommitAndCreateLayerImplTree() {
   // TODO(pdr): Update the LayerTreeImpl lifecycle states here so lifecycle
   // violations can be caught.
@@ -74,7 +92,7 @@ LayerImpl* FakeLayerTreeHost::CommitAndCreateLayerImplTree() {
   active_tree()->SetPropertyTrees(property_trees());
   TreeSynchronizer::PushLayerProperties(root_layer()->layer_tree_host(),
                                         active_tree());
-  mutator_host()->PushPropertiesTo(host_impl_.mutator_host());
+  mutator_host()->PushPropertiesTo(host_impl_->mutator_host());
 
   active_tree()->property_trees()->scroll_tree.PushScrollUpdatesFromMainThread(
       property_trees(), active_tree());
@@ -87,7 +105,7 @@ LayerImpl* FakeLayerTreeHost::CommitAndCreatePendingTree() {
   pending_tree()->SetPropertyTrees(property_trees());
   TreeSynchronizer::PushLayerProperties(root_layer()->layer_tree_host(),
                                         pending_tree());
-  mutator_host()->PushPropertiesTo(host_impl_.mutator_host());
+  mutator_host()->PushPropertiesTo(host_impl_->mutator_host());
 
   pending_tree()->property_trees()->scroll_tree.PushScrollUpdatesFromMainThread(
       property_trees(), pending_tree());

cc/test/fake_layer_tree_host.h

@@ -5,6 +5,8 @@
 #ifndef CC_TEST_FAKE_LAYER_TREE_HOST_H_
 #define CC_TEST_FAKE_LAYER_TREE_HOST_H_
 
+#include <memory>
+
 #include "cc/benchmarks/micro_benchmark_controller.h"
 #include "cc/test/fake_impl_task_runner_provider.h"
 #include "cc/test/fake_layer_tree_host_client.h"
@@ -18,7 +20,23 @@ namespace cc {
 class MutatorHost;
 class TestTaskGraphRunner;
 
-class FakeLayerTreeHost : public LayerTreeHost {
+// FakeImplTaskRunnerProvider uses DebugScopedSetImplThread to set its task
+// runner to look like the impl thread. This means it needs to outlive the
+// LayerTreeHost so that the task runner remains impl-thread bound. This helper
+// class is inherited before LayerTreeHost to ensure it's constructed first and
+// destructed last.
+class TaskRunnerProviderHolder {
+ public:
+  FakeImplTaskRunnerProvider& task_runner_provider() {
+    return task_runner_provider_;
+  }
+
+ private:
+  FakeImplTaskRunnerProvider task_runner_provider_;
+};
+
+class FakeLayerTreeHost : private TaskRunnerProviderHolder,
+                          public LayerTreeHost {
  public:
   static std::unique_ptr<FakeLayerTreeHost> Create(
       FakeLayerTreeHostClient* client,
@@ -47,12 +65,29 @@ class FakeLayerTreeHost : public LayerTreeHost {
   void SetNeedsCommit() override;
   void SetNeedsUpdateLayers() override {}
 
+  std::unique_ptr<LayerTreeHostImpl> CreateLayerTreeHostImpl(
+      LayerTreeHostImplClient* client) override;
+
+  // This method is exposed for tests that don't use a Proxy (the
+  // initialization of which would call the overridden CreateLayerTreeHostImpl
+  // above). This method will create a FakeLayerTreeHostImpl which is owned by
+  // this object and can be accessed via host_impl(). This classs ensures that
+  // the FakeLayerTreeHost doesn't then try to create a second
+  // FakeLayerTreeHostImpl via a proxy.
+  void CreateFakeLayerTreeHostImpl();
+
   LayerImpl* CommitAndCreateLayerImplTree();
   LayerImpl* CommitAndCreatePendingTree();
 
-  FakeLayerTreeHostImpl* host_impl() { return &host_impl_; }
-  LayerTreeImpl* active_tree() { return host_impl_.active_tree(); }
-  LayerTreeImpl* pending_tree() { return host_impl_.pending_tree(); }
+  FakeLayerTreeHostImpl* host_impl() { return host_impl_; }
+  LayerTreeImpl* active_tree() {
+    DCHECK(host_impl_);
+    return host_impl_->active_tree();
+  }
+  LayerTreeImpl* pending_tree() {
+    DCHECK(host_impl_);
+    return host_impl_->pending_tree();
+  }
 
   using LayerTreeHost::ScheduleMicroBenchmark;
   using LayerTreeHost::SendMessageToMicroBenchmark;
@@ -74,11 +109,21 @@ class FakeLayerTreeHost : public LayerTreeHost {
                     LayerTreeHost::InitParams params,
                     CompositorMode mode);
 
+ protected:
+  FakeLayerTreeHostClient* client_ = nullptr;
+  FakeLayerTreeHostImpl* host_impl_ = nullptr;
+
+  bool needs_commit_ = false;
+
  private:
-  FakeImplTaskRunnerProvider task_runner_provider_;
-  FakeLayerTreeHostClient* client_;
-  FakeLayerTreeHostImpl host_impl_;
-  bool needs_commit_;
+  // Used only if created via CreateFakeLayerTreeHostImpl to provide ownership
+  // and lifetime management. Normally the Proxy object owns the LTHI which
+  // will be the case if the test initializes a Proxy and the Impl is created
+  // via the overriden CreateLayerTreeHostImpl. Tests without a Proxy that
+  // manually create an Impl will call CreateFakeLayerTreeHostImpl and this
+  // class will own that object. Calls should be made on the |host_impl_|
+  // pointer as that'll always be set to the correct object.
+  std::unique_ptr<FakeLayerTreeHostImpl> owned_host_impl_;
 };
 
 }  // namespace cc

cc/test/fake_layer_tree_host_impl.h

@@ -15,6 +15,11 @@ namespace cc {
 
 class AnimationHost;
 
+// Note: If you're creating this as a pair with a FakeLayerTreeHost, consider
+// creating it via FakeLayerTreeHost::InitializeSingleThreaded if your test
+// will use a Proxy or FakeLayerTreeHost::CreateFakeLayerTreeHostImpl if it
+// doesn't use a Proxy. These will ensure we're not accidentally creating
+// multiple HostImpls.
 class FakeLayerTreeHostImpl : public LayerTreeHostImpl {
  public:
   FakeLayerTreeHostImpl(TaskRunnerProvider* task_runner_provider,

cc/test/layer_tree_impl_test_base.cc

@@ -44,6 +44,7 @@ LayerTreeImplTestBase::LayerTreeImplTestBase(
                                       settings)),
       render_pass_(viz::RenderPass::Create()),
       layer_impl_id_(2) {
+  host_->CreateFakeLayerTreeHostImpl();
   std::unique_ptr<LayerImpl> root =
       LayerImpl::Create(host_impl()->active_tree(), 1);
   root->SetBounds(gfx::Size(1, 1));

cc/trees/layer_tree_host_unittest_record_gpu_histogram.cc

@@ -20,6 +20,7 @@ TEST(LayerTreeHostRecordGpuHistogramTest, SingleThreaded) {
   std::unique_ptr<FakeLayerTreeHost> host = FakeLayerTreeHost::Create(
       &host_client, &task_graph_runner, animation_host.get(), settings,
       CompositorMode::SINGLE_THREADED);
+  host->CreateFakeLayerTreeHostImpl();
   host->RecordGpuRasterizationHistogram(host->host_impl());
   EXPECT_FALSE(host->gpu_rasterization_histogram_recorded());
 }
@@ -32,6 +33,7 @@ TEST(LayerTreeHostRecordGpuHistogramTest, Threaded) {
   std::unique_ptr<FakeLayerTreeHost> host = FakeLayerTreeHost::Create(
       &host_client, &task_graph_runner, animation_host.get(), settings,
       CompositorMode::THREADED);
+  host->CreateFakeLayerTreeHostImpl();
   host->RecordGpuRasterizationHistogram(host->host_impl());
   EXPECT_TRUE(host->gpu_rasterization_histogram_recorded());
 }

cc/trees/occlusion_tracker_unittest.cc

@@ -6,6 +6,9 @@
 
 #include <stddef.h>
 
+#include <memory>
+#include <utility>
+
 #include "cc/animation/animation_host.h"
 #include "cc/base/math_util.h"
 #include "cc/layers/layer.h"
@@ -99,6 +102,7 @@ class OcclusionTrackerTest : public testing::Test {
                                         animation_host_.get(),
                                         LayerListSettings())),
         next_layer_impl_id_(1) {
+    host_->CreateFakeLayerTreeHostImpl();
     host_->host_impl()->InitializeFrameSink(layer_tree_frame_sink_.get());
   }
 

cc/trees/tree_synchronizer_unittest.cc

@@ -117,6 +117,8 @@ class TreeSynchronizerTest : public testing::Test {
   void ResetLayerTreeHost(const LayerTreeSettings& settings) {
     host_ = FakeLayerTreeHost::Create(&client_, &task_graph_runner_,
                                       animation_host_.get(), settings);
+    host_->InitializeSingleThreaded(&single_thread_client_,
+                                    base::ThreadTaskRunnerHandle::Get());
     host_->host_impl()->CreatePendingTree();
   }
 
@@ -591,8 +593,6 @@ TEST_F(TreeSynchronizerTest, SynchronizeCurrentlyScrollingNode) {
 }
 
 TEST_F(TreeSynchronizerTest, SynchronizeScrollTreeScrollOffsetMap) {
-  host_->InitializeSingleThreaded(&single_thread_client_,
-                                  base::ThreadTaskRunnerHandle::Get());
   LayerTreeSettings settings;
   FakeLayerTreeHostImplClient client;
   FakeImplTaskRunnerProvider task_runner_provider;
@@ -704,8 +704,6 @@ TEST_F(TreeSynchronizerTest, SynchronizeScrollTreeScrollOffsetMap) {
 }
 
 TEST_F(TreeSynchronizerTest, RefreshPropertyTreesCachedData) {
-  host_->InitializeSingleThreaded(&single_thread_client_,
-                                  base::ThreadTaskRunnerHandle::Get());
   LayerTreeSettings settings;
   FakeLayerTreeHostImplClient client;
   FakeImplTaskRunnerProvider task_runner_provider;
@@ -752,9 +750,6 @@ TEST_F(TreeSynchronizerTest, RoundedScrollDeltasOnCommit) {
   settings.commit_fractional_scroll_deltas = false;
   ResetLayerTreeHost(settings);
 
-  host_->InitializeSingleThreaded(&single_thread_client_,
-                                  base::ThreadTaskRunnerHandle::Get());
-
   scoped_refptr<Layer> scroll_layer = SetupScrollLayer();
 
   // Scroll the layer by a fractional amount.
@@ -776,9 +771,6 @@ TEST_F(TreeSynchronizerTest, PreserveFractionalScrollDeltasOnCommit) {
   settings.commit_fractional_scroll_deltas = true;
   ResetLayerTreeHost(settings);
 
-  host_->InitializeSingleThreaded(&single_thread_client_,
-                                  base::ThreadTaskRunnerHandle::Get());
-
   scoped_refptr<Layer> scroll_layer = SetupScrollLayer();
 
   // Scroll the layer by a fractional amount.