cc: Add LayerTreeHost::InitParams for LayerTreeHost creation.

cc/test/layer_tree_test.cc

Index: cc/test/layer_tree_test.cc
diff --git a/cc/test/layer_tree_test.cc b/cc/test/layer_tree_test.cc
index 055d0817d30466ff13be525e2b5edc9dedb5bd5a..8aee21c5b76d3e9c245a725a8f8db32dca96aa18 100644
--- a/cc/test/layer_tree_test.cc
+++ b/cc/test/layer_tree_test.cc
@@ -460,10 +460,15 @@ class LayerTreeHostForTesting : public LayerTreeHost {
       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,
@@ -500,17 +505,12 @@ class LayerTreeHostForTesting : public LayerTreeHost {
   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) {}