cc: Decouple tile manager from raster worker pool

In order to unittest tile manager, it would be nice if we
can fake raster worker pool (and possibly other components)
that tile manager currently creates in its constructor.

This should be a small step towards making tile manager more
testable.

Review URL: https://chromiumcodereview.appspot.com/14803016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@201118 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 73 insertions, 40 deletions

diff --git a/cc/base/worker_pool.cc b/cc/base/worker_pool.cc
index be9abf9..27e6440 100644
--- a/cc/base/worker_pool.cc
+++ b/cc/base/worker_pool.cc
@@ -293,11 +293,10 @@ void WorkerPool::Inner::Run() {
   has_pending_tasks_cv_.Signal();
 }
 
-WorkerPool::WorkerPool(WorkerPoolClient* client,
-                       size_t num_threads,
+WorkerPool::WorkerPool(size_t num_threads,
                        base::TimeDelta check_for_completed_tasks_delay,
                        const std::string& thread_name_prefix)
-    : client_(client),
+    : client_(NULL),
       origin_loop_(base::MessageLoopProxy::current()),
       weak_ptr_factory_(this),
       check_for_completed_tasks_delay_(check_for_completed_tasks_delay),
@@ -367,6 +366,7 @@ void WorkerPool::DispatchCompletionCallbacks() {
     task->DidComplete();
   }
 
+  DCHECK(client_);
   client_->DidFinishDispatchingWorkerPoolCompletionCallbacks();
 }
 
diff --git a/cc/base/worker_pool.h b/cc/base/worker_pool.h
index 7675567..aa49ec9 100644
--- a/cc/base/worker_pool.h
+++ b/cc/base/worker_pool.h
@@ -51,12 +51,10 @@ class CC_EXPORT WorkerPool {
   virtual ~WorkerPool();
 
   static scoped_ptr<WorkerPool> Create(
-      WorkerPoolClient* client,
       size_t num_threads,
       base::TimeDelta check_for_completed_tasks_delay,
       const std::string& thread_name_prefix) {
-    return make_scoped_ptr(new WorkerPool(client,
-                                          num_threads,
+    return make_scoped_ptr(new WorkerPool(num_threads,
                                           check_for_completed_tasks_delay,
                                           thread_name_prefix));
   }
@@ -69,9 +67,13 @@ class CC_EXPORT WorkerPool {
   // is posted to the thread that called PostTaskAndReply().
   void PostTaskAndReply(const Callback& task, const base::Closure& reply);
 
+  // Set a new client.
+  void SetClient(WorkerPoolClient* client) {
+    client_ = client;
+  }
+
  protected:
-  WorkerPool(WorkerPoolClient* client,
-             size_t num_threads,
+  WorkerPool(size_t num_threads,
              base::TimeDelta check_for_completed_tasks_delay,
              const std::string& thread_name_prefix);
 
diff --git a/cc/base/worker_pool_unittest.cc b/cc/base/worker_pool_unittest.cc
index 28bef7f..c032e2c 100644
--- a/cc/base/worker_pool_unittest.cc
+++ b/cc/base/worker_pool_unittest.cc
@@ -35,10 +35,10 @@ class WorkerPoolTest : public testing::Test,
   }
 
   void Reset() {
-    worker_pool_ = WorkerPool::Create(this,
-                                      1,
+    worker_pool_ = WorkerPool::Create(1,
                                       base::TimeDelta::FromDays(1024),
                                       "test");
+    worker_pool_->SetClient(this);
   }
 
   void RunAllTasksAndReset() {
diff --git a/cc/resources/raster_worker_pool.cc b/cc/resources/raster_worker_pool.cc
index 0cb913d..dba3fc2 100644
--- a/cc/resources/raster_worker_pool.cc
+++ b/cc/resources/raster_worker_pool.cc
@@ -36,12 +36,11 @@ const int kCheckForCompletedTasksDelayMs = 6;
 
 }  // namespace
 
-RasterWorkerPool::RasterWorkerPool(
-    WorkerPoolClient* client, size_t num_threads) : WorkerPool(
-        client,
-        num_threads,
-        base::TimeDelta::FromMilliseconds(kCheckForCompletedTasksDelayMs),
-        kWorkerThreadNamePrefix) {
+RasterWorkerPool::RasterWorkerPool(size_t num_threads)
+    : WorkerPool(
+          num_threads,
+          base::TimeDelta::FromMilliseconds(kCheckForCompletedTasksDelayMs),
+          kWorkerThreadNamePrefix) {
 }
 
 RasterWorkerPool::~RasterWorkerPool() {
diff --git a/cc/resources/raster_worker_pool.h b/cc/resources/raster_worker_pool.h
index 8d3691d..76d0497 100644
--- a/cc/resources/raster_worker_pool.h
+++ b/cc/resources/raster_worker_pool.h
@@ -13,15 +13,14 @@ namespace cc {
 class PicturePileImpl;
 
 // A worker thread pool that runs raster tasks.
-class RasterWorkerPool : public WorkerPool {
+class CC_EXPORT RasterWorkerPool : public WorkerPool {
  public:
   typedef base::Callback<void(PicturePileImpl* picture_pile)> RasterCallback;
 
   virtual ~RasterWorkerPool();
 
-  static scoped_ptr<RasterWorkerPool> Create(
-      WorkerPoolClient* client, size_t num_threads) {
-    return make_scoped_ptr(new RasterWorkerPool(client, num_threads));
+  static scoped_ptr<RasterWorkerPool> Create(size_t num_threads) {
+    return make_scoped_ptr(new RasterWorkerPool(num_threads));
   }
 
   void PostRasterTaskAndReply(PicturePileImpl* picture_pile,
@@ -29,7 +28,7 @@ class RasterWorkerPool : public WorkerPool {
                               const base::Closure& reply);
 
  private:
-  RasterWorkerPool(WorkerPoolClient* client, size_t num_threads);
+  explicit RasterWorkerPool(size_t num_threads);
 
   DISALLOW_COPY_AND_ASSIGN(RasterWorkerPool);
 };
diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
index 0474e83..31a66b6 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -105,15 +105,33 @@ scoped_ptr<base::Value> TileManagerBinPriorityAsValue(
   }
 }
 
+// static
+scoped_ptr<TileManager> TileManager::Create(
+    TileManagerClient* client,
+    ResourceProvider* resource_provider,
+    size_t num_raster_threads,
+    bool use_color_estimator,
+    RenderingStatsInstrumentation* rendering_stats_instrumentation) {
+  scoped_ptr<RasterWorkerPool> raster_worker_pool =
+      RasterWorkerPool::Create(num_raster_threads);
+  return make_scoped_ptr(new TileManager(client,
+                                         resource_provider,
+                                         raster_worker_pool.Pass(),
+                                         num_raster_threads,
+                                         use_color_estimator,
+                                         rendering_stats_instrumentation));
+}
+
 TileManager::TileManager(
     TileManagerClient* client,
     ResourceProvider* resource_provider,
+    scoped_ptr<RasterWorkerPool> raster_worker_pool,
     size_t num_raster_threads,
     bool use_color_estimator,
     RenderingStatsInstrumentation* rendering_stats_instrumentation)
     : client_(client),
       resource_pool_(ResourcePool::Create(resource_provider)),
-      raster_worker_pool_(RasterWorkerPool::Create(this, num_raster_threads)),
+      raster_worker_pool_(raster_worker_pool.Pass()),
       manage_tiles_pending_(false),
       manage_tiles_call_count_(0),
       bytes_pending_upload_(0),
@@ -124,6 +142,7 @@ TileManager::TileManager(
       did_initialize_visible_tile_(false),
       pending_tasks_(0),
       max_pending_tasks_(kMaxNumPendingTasksPerThread * num_raster_threads) {
+  raster_worker_pool_->SetClient(this);
 }
 
 TileManager::~TileManager() {
diff --git a/cc/resources/tile_manager.h b/cc/resources/tile_manager.h
index 6079aa7..cdf1f3d 100644
--- a/cc/resources/tile_manager.h
+++ b/cc/resources/tile_manager.h
@@ -61,11 +61,13 @@ scoped_ptr<base::Value> TileManagerBinPriorityAsValue(
 // created, and unregister from the manager when they are deleted.
 class CC_EXPORT TileManager : public WorkerPoolClient {
  public:
-  TileManager(TileManagerClient* client,
-              ResourceProvider *resource_provider,
-              size_t num_raster_threads,
-              bool use_color_estimator,
-              RenderingStatsInstrumentation* rendering_stats_instrumentation);
+  static scoped_ptr<TileManager> Create(
+      TileManagerClient* client,
+      ResourceProvider* resource_provider,
+      size_t num_raster_threads,
+      bool use_color_estimator,
+      RenderingStatsInstrumentation* rendering_stats_instrumentation);
+
   virtual ~TileManager();
 
   const GlobalStateThatImpactsTilePriority& GlobalState() const {
@@ -92,6 +94,13 @@ class CC_EXPORT TileManager : public WorkerPoolClient {
   virtual void DidFinishDispatchingWorkerPoolCompletionCallbacks() OVERRIDE;
 
  protected:
+  TileManager(TileManagerClient* client,
+              ResourceProvider* resource_provider,
+              scoped_ptr<RasterWorkerPool> raster_worker_pool,
+              size_t num_raster_threads,
+              bool use_color_estimator,
+              RenderingStatsInstrumentation* rendering_stats_instrumentation);
+
   // Methods called by Tile
   friend class Tile;
   void RegisterTile(Tile* tile);
diff --git a/cc/test/fake_picture_layer_tiling_client.cc b/cc/test/fake_picture_layer_tiling_client.cc
index 7c18d35..3a838dd 100644
--- a/cc/test/fake_picture_layer_tiling_client.cc
+++ b/cc/test/fake_picture_layer_tiling_client.cc
@@ -23,15 +23,14 @@ class FakeInfinitePicturePileImpl : public PicturePileImpl {
 };
 
 FakePictureLayerTilingClient::FakePictureLayerTilingClient()
-    : tile_manager_(&tile_manager_client_,
-                    NULL,
-                    1,
-                    false,
-                    &stats_instrumentation_),
+    : tile_manager_(TileManager::Create(&tile_manager_client_,
+                                        NULL,
+                                        1,
+                                        false,
+                                        &stats_instrumentation_)),
       pile_(new FakeInfinitePicturePileImpl()),
       twin_tiling_(NULL),
-      allow_create_tile_(true) {
-}
+      allow_create_tile_(true) {}
 
 FakePictureLayerTilingClient::~FakePictureLayerTilingClient() {
 }
@@ -41,7 +40,7 @@ scoped_refptr<Tile> FakePictureLayerTilingClient::CreateTile(
     gfx::Rect rect) {
   if (!allow_create_tile_)
     return NULL;
-  return new Tile(&tile_manager_,
+  return new Tile(tile_manager_.get(),
                   pile_.get(),
                   tile_size_,
                   rect,
diff --git a/cc/test/fake_picture_layer_tiling_client.h b/cc/test/fake_picture_layer_tiling_client.h
index 32a39bf..1050705 100644
--- a/cc/test/fake_picture_layer_tiling_client.h
+++ b/cc/test/fake_picture_layer_tiling_client.h
@@ -44,7 +44,7 @@ class FakePictureLayerTilingClient : public PictureLayerTilingClient {
  protected:
   FakeTileManagerClient tile_manager_client_;
   FakeRenderingStatsInstrumentation stats_instrumentation_;
-  TileManager tile_manager_;
+  scoped_ptr<TileManager> tile_manager_;
   scoped_refptr<PicturePileImpl> pile_;
   gfx::Size tile_size_;
   PictureLayerTiling* twin_tiling_;
diff --git a/cc/test/fake_tile_manager.cc b/cc/test/fake_tile_manager.cc
index 06c6c86..c3d1349 100644
--- a/cc/test/fake_tile_manager.cc
+++ b/cc/test/fake_tile_manager.cc
@@ -4,9 +4,15 @@
 
 #include "cc/test/fake_tile_manager.h"
 
+#include "cc/resources/raster_worker_pool.h"
+
 namespace cc {
 
 FakeTileManager::FakeTileManager(TileManagerClient* client)
-    : TileManager(client, NULL, 1, false, NULL) {
-}
+    : TileManager(client,
+                  NULL,
+                  RasterWorkerPool::Create(1),
+                  1,
+                  false,
+                  NULL) {}
 }
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index fa6664a..4dcbe76 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -1410,11 +1410,11 @@ bool LayerTreeHostImpl::InitializeRenderer(
     return false;
 
   if (settings_.impl_side_painting) {
-    tile_manager_.reset(new TileManager(this,
+    tile_manager_ = TileManager::Create(this,
                                         resource_provider.get(),
                                         settings_.num_raster_threads,
                                         settings_.use_color_estimator,
-                                        rendering_stats_instrumentation_));
+                                        rendering_stats_instrumentation_);
     UpdateTileManagerMemoryPolicy(ActualManagedMemoryPolicy());
   }