cc: Add LayerTreeHost::InitParams for LayerTreeHost creation.

cc/test/layer_tree_test.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/test/layer_tree_test.h"
 
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
@@ skipping 442 matching lines @@
   static scoped_ptr<LayerTreeHostForTesting> Create(
       TestHooks* test_hooks,
       LayerTreeHostClientForTesting* client,
       SharedBitmapManager* shared_bitmap_manager,
       gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
       TaskGraphRunner* task_graph_runner,
       const LayerTreeSettings& settings,
       scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
       scoped_refptr<base::SingleThreadTaskRunner> impl_task_runner,
       scoped_ptr<BeginFrameSource> external_begin_frame_source) {
+    LayerTreeHost::InitParams params;
+    params.client = client;
+    params.shared_bitmap_manager = shared_bitmap_manager;
+    params.gpu_memory_buffer_manager = gpu_memory_buffer_manager;
+    params.task_graph_runner = task_graph_runner;
+    params.settings = &settings;
+    params.main_task_runner = main_task_runner;

[danakj, 2015/05/07 18:28:32]

this is similar, we used to not pass it to the constructor, and only pass it to
the Initialize, maybe we should keep that

[sadrul, 2015/05/07 22:47:43]

Done (not setting main_task_runner anymore).

     scoped_ptr<LayerTreeHostForTesting> layer_tree_host(
-        new LayerTreeHostForTesting(test_hooks, client, shared_bitmap_manager,
-                                    gpu_memory_buffer_manager,
-                                    task_graph_runner, settings));
+        new LayerTreeHostForTesting(test_hooks, &params));
     if (impl_task_runner.get()) {
       layer_tree_host->InitializeForTesting(
           ThreadProxyForTest::Create(test_hooks,
                                      layer_tree_host.get(),
                                      main_task_runner,
                                      impl_task_runner,
                                      external_begin_frame_source.Pass()));
     } else {
       layer_tree_host->InitializeForTesting(
           SingleThreadProxyForTest::Create(
@@ skipping 16 matching lines @@
 
   void SetNeedsCommit() override {
     if (!test_started_)
       return;
     LayerTreeHost::SetNeedsCommit();
   }
 
   void set_test_started(bool started) { test_started_ = started; }
 
  private:
-  LayerTreeHostForTesting(
-      TestHooks* test_hooks,
-      LayerTreeHostClient* client,
-      SharedBitmapManager* shared_bitmap_manager,
-      gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
-      TaskGraphRunner* task_graph_runner,
-      const LayerTreeSettings& settings)
-      : LayerTreeHost(client, NULL, NULL, NULL, settings),
-        shared_bitmap_manager_(shared_bitmap_manager),
-        gpu_memory_buffer_manager_(gpu_memory_buffer_manager),
-        task_graph_runner_(task_graph_runner),
+  LayerTreeHostForTesting(TestHooks* test_hooks,
+                          LayerTreeHost::InitParams* params)
+      : LayerTreeHost(params),
+        shared_bitmap_manager_(params->shared_bitmap_manager),
+        gpu_memory_buffer_manager_(params->gpu_memory_buffer_manager),
+        task_graph_runner_(params->task_graph_runner),

[sadrul, 2015/05/07 04:39:26]

Can you double check this a bit? The old code sends nullptr for
shared_bitmap_manager, gpu_memory_buffer_manager, and task_graph_runner to the
LayerTreeHost ctor, but sets the duplicate-member variables here. With this
change, both the LTH ctor and the local memebers are set. The tests don't seem
to mind, so it's a bit unclear if the old behaviour was deliberate or not.

[danakj, 2015/05/07 18:28:32]

Right, passing them to the LTH is pointless, cuz they are only stored there to
pass to the LTHI when it's created.
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...

But we override that method and pass them from the test harness on L491.

We could stop storing them in the harness and have the above method pull them
off the LTH and pass them to the LTHI like the real method does, then they
aren't stored in 2 places.

Or we could explain all of this in some comments here, which would be nice.

[sadrul, 2015/05/07 22:47:43]

Thanks for explaining! I have made this change.

         test_hooks_(test_hooks),
         test_started_(false) {}
 
   SharedBitmapManager* shared_bitmap_manager_;
   gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager_;
   TaskGraphRunner* task_graph_runner_;
   TestHooks* test_hooks_;
   bool test_started_;
 };
 
@@ skipping 365 matching lines @@
 LayerTreeHost* LayerTreeTest::layer_tree_host() {
   // We check for a null proxy here as we sometimes ask for the layer tree host
   // when the proxy does not exist, often for checking settings after a test has
   // completed. For example, LTHPixelResourceTest::RunPixelResourceTest. See
   // elsewhere in this file for other examples.
   DCHECK(!proxy() || proxy()->IsMainThread() || proxy()->IsMainThreadBlocked());
   return layer_tree_host_.get();
 }
 
 }  // namespace cc