cc: Add new RasterWorkerPool interface.

cc/resources/raster_worker_pool.cc

Index: cc/resources/raster_worker_pool.cc
diff --git a/cc/resources/raster_worker_pool.cc b/cc/resources/raster_worker_pool.cc
index a078fb00bb7e9600e7408a10bd37c92b23a5d9fb..97489582d5a30919cd8da6c13395852214e91ae0 100644
--- a/cc/resources/raster_worker_pool.cc
+++ b/cc/resources/raster_worker_pool.cc
@@ -1,4 +1,4 @@
-// Copyright 2012 The Chromium Authors. All rights reserved.
+// Copyright 2013 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.
 
@@ -10,72 +10,70 @@ namespace cc {
 
 namespace {
 
-class RasterWorkerPoolContainerTaskImpl : public internal::WorkerPoolTask {
- public:
-  RasterWorkerPoolContainerTaskImpl(
-      internal::WorkerPoolTask::TaskVector* dependencies)
-      : internal::WorkerPoolTask(dependencies) {
-  }
-
-  // Overridden from internal::WorkerPoolTask:
-  virtual void RunOnThread(unsigned thread_index) OVERRIDE {}
-  virtual void DispatchCompletionCallback() OVERRIDE {}
-
- private:
-  virtual ~RasterWorkerPoolContainerTaskImpl() {}
-};
+void Noop() {}
 
-class RasterWorkerPoolTaskImpl : public internal::WorkerPoolTask {
+class WorkerPoolTaskImpl : public internal::WorkerPoolTask {
  public:
-  RasterWorkerPoolTaskImpl(const base::Closure& callback,
-                           const RasterWorkerPool::Task::Reply& reply)
+  WorkerPoolTaskImpl(const base::Closure& callback,
+                     const base::Closure& reply)
       : callback_(callback),
         reply_(reply) {
   }
+  explicit WorkerPoolTaskImpl(
+      internal::WorkerPoolTask::TaskVector* dependencies)
+      : internal::WorkerPoolTask(dependencies),
+        callback_(base::Bind(&Noop)),
+        reply_(base::Bind(&Noop)) {
+  }
 
   // Overridden from internal::WorkerPoolTask:
   virtual void RunOnThread(unsigned thread_index) OVERRIDE {
     callback_.Run();
   }
   virtual void DispatchCompletionCallback() OVERRIDE {
-    reply_.Run(!HasFinishedRunning());
+    reply_.Run();
   }
 
  private:
-  virtual ~RasterWorkerPoolTaskImpl() {}
+  virtual ~WorkerPoolTaskImpl() {}
 
   const base::Closure callback_;
-  const RasterWorkerPool::Task::Reply reply_;
+  const base::Closure reply_;
+
+  DISALLOW_COPY_AND_ASSIGN(WorkerPoolTaskImpl);
 };
 
-class RasterWorkerPoolPictureTaskImpl : public internal::WorkerPoolTask {
+class RasterWorkerPoolTaskImpl : public internal::RasterWorkerPoolTask {
  public:
-  RasterWorkerPoolPictureTaskImpl(
+  RasterWorkerPoolTaskImpl(
       PicturePileImpl* picture_pile,
-      const RasterWorkerPool::PictureTask::Callback& callback,
-      const RasterWorkerPool::Task::Reply& reply,
+      const Resource* resource,
+      const RasterWorkerPool::RasterTask::Callback& callback,
+      const RasterWorkerPool::RasterTask::Reply& reply,
       internal::WorkerPoolTask::TaskVector* dependencies)
-      : internal::WorkerPoolTask(dependencies),
+      : internal::RasterWorkerPoolTask(resource, dependencies),
         picture_pile_(picture_pile),
         callback_(callback),
         reply_(reply) {
-    DCHECK(picture_pile_);
   }
 
-  // Overridden from internal::WorkerPoolTask:
-  virtual void RunOnThread(unsigned thread_index) OVERRIDE {
-    callback_.Run(picture_pile_->GetCloneForDrawingOnThread(thread_index));
+  // Overridden from internal::RasterWorkerPoolTask:
+  virtual bool RunOnThread(SkDevice* device, unsigned thread_index) OVERRIDE {
+    return callback_.Run(
+        device, picture_pile_->GetCloneForDrawingOnThread(thread_index));
   }
   virtual void DispatchCompletionCallback() OVERRIDE {
     reply_.Run(!HasFinishedRunning());
   }
 
  private:
-  virtual ~RasterWorkerPoolPictureTaskImpl() {}
+  virtual ~RasterWorkerPoolTaskImpl() {}
 
   scoped_refptr<PicturePileImpl> picture_pile_;

[reveman, 2013/05/30 23:25:18]

A tile's PicturePileImpl might change during activation. Maybe we can limit the
tiles that we can change piles for to only tiles that doesn't have pending
raster tasks but it seems like a slippery slope. Keeping a ref here is a simple
and reliable way to make sure we always have a valid pile for raster.

[vmpstr, 2013/05/31 15:55:10]

I think that makes the most sense.

[enne (OOO), 2013/05/31 17:05:38]

The reason we change the pile during activation is to keep memory under control
by keeping the number of piles in use capped at 2.  If we temporarily go above
that for tiles in flight, that doesn't seem like a big deal.

Correctness isn't affected here, as if using the new pile would have created a
different rasterization we would have created an entirely new tile for it anyway
and it wouldn't be using the old pile.

-  const RasterWorkerPool::PictureTask::Callback callback_;
-  const RasterWorkerPool::Task::Reply reply_;
+  const RasterWorkerPool::RasterTask::Callback callback_;
+  const RasterWorkerPool::RasterTask::Reply reply_;
+
+  DISALLOW_COPY_AND_ASSIGN(RasterWorkerPoolTaskImpl);
 };
 
 const char* kWorkerThreadNamePrefix = "CompositorRaster";
@@ -84,13 +82,47 @@ const int kCheckForCompletedTasksDelayMs = 6;
 
 }  // namespace
 
-RasterWorkerPool::Task::Queue::Queue() {
+namespace internal {
+
+RasterWorkerPoolTask::RasterWorkerPoolTask(
+    const Resource* resource,
+    WorkerPoolTask::TaskVector* dependencies)
+    : did_run_(false),
+      did_complete_(false),
+      resource_(resource) {
+  dependencies_.swap(*dependencies);
+}
+
+RasterWorkerPoolTask::~RasterWorkerPoolTask() {
+}
+
+void RasterWorkerPoolTask::DidRun() {
+  DCHECK(!did_run_);
+  did_run_ = true;
+}
+
+bool RasterWorkerPoolTask::HasFinishedRunning() const {
+  return did_run_;
+}
+
+void RasterWorkerPoolTask::DidComplete() {
+  DCHECK(!did_complete_);
+  did_complete_ = true;
 }
 
-RasterWorkerPool::Task::Queue::~Queue() {
+bool RasterWorkerPoolTask::HasCompleted() const {
+  return did_complete_;
 }
 
-void RasterWorkerPool::Task::Queue::Append(const Task& task) {
+}  // namespace internal
+
+RasterWorkerPool::Task::Set::Set() {
+}
+
+RasterWorkerPool::Task::Set::~Set() {
+}
+
+void RasterWorkerPool::Task::Set::Insert(const Task& task) {
   DCHECK(!task.is_null());
   tasks_.push_back(task.internal_);
 }
@@ -99,54 +131,84 @@ RasterWorkerPool::Task::Task() {
 }
 
 RasterWorkerPool::Task::Task(const base::Closure& callback,
-                             const Reply& reply)
-    : internal_(new RasterWorkerPoolTaskImpl(callback, reply)) {
+                             const base::Closure& reply)
+    : internal_(new WorkerPoolTaskImpl(callback, reply)) {
+}
+
+RasterWorkerPool::Task::~Task() {
 }
 
-RasterWorkerPool::Task::Task(Queue* dependencies)
-    : internal_(new RasterWorkerPoolContainerTaskImpl(&dependencies->tasks_)) {
+void RasterWorkerPool::Task::Reset() {
+  internal_ = NULL;
 }
 
-RasterWorkerPool::Task::Task(scoped_refptr<internal::WorkerPoolTask> internal)
-    : internal_(internal) {
+RasterWorkerPool::RasterTask::Queue::Queue() {
 }
 
-RasterWorkerPool::Task::~Task() {
+RasterWorkerPool::RasterTask::Queue::~Queue() {
 }
 
-void RasterWorkerPool::Task::Reset() {
+void RasterWorkerPool::RasterTask::Queue::Append(const RasterTask& task) {
+  DCHECK(!task.is_null());
+  tasks_.push_back(task.internal_);
+}
+
+RasterWorkerPool::RasterTask::RasterTask() {
+}
+
+RasterWorkerPool::RasterTask::RasterTask(PicturePileImpl* picture_pile,
+                                         const Resource* resource,
+                                         const Callback& callback,
+                                         const Reply& reply,
+                                         Task::Set* dependencies)
+    : internal_(new RasterWorkerPoolTaskImpl(picture_pile,
+                                             resource,
+                                             callback,
+                                             reply,
+                                             &dependencies->tasks_)) {
+}
+
+void RasterWorkerPool::RasterTask::Reset() {
   internal_ = NULL;
 }
 
-RasterWorkerPool::PictureTask::PictureTask(PicturePileImpl* picture_pile,
-                                           const Callback& callback,
-                                           const Reply& reply,
-                                           Task::Queue* dependencies)
-    : RasterWorkerPool::Task(
-        new RasterWorkerPoolPictureTaskImpl(picture_pile,
-                                            callback,
-                                            reply,
-                                            &dependencies->tasks_)) {
+RasterWorkerPool::RasterTask::~RasterTask() {
 }
 
-RasterWorkerPool::RasterWorkerPool(size_t num_threads) : WorkerPool(
-    num_threads,
-    base::TimeDelta::FromMilliseconds(kCheckForCompletedTasksDelayMs),
-    kWorkerThreadNamePrefix) {
+RasterWorkerPool::RasterWorkerPool(ResourceProvider* resource_provider,
+                                   size_t num_threads)
+    : WorkerPool(
+        num_threads,
+        base::TimeDelta::FromMilliseconds(kCheckForCompletedTasksDelayMs),
+        kWorkerThreadNamePrefix),
+      resource_provider_(resource_provider) {
 }
 
 RasterWorkerPool::~RasterWorkerPool() {
 }
 
 void RasterWorkerPool::Shutdown() {
-  // Cancel all previously scheduled tasks.
-  WorkerPool::ScheduleTasks(NULL);
-
+  raster_tasks_.clear();
   WorkerPool::Shutdown();
 }
 
-void RasterWorkerPool::ScheduleTasks(Task* task) {
-  WorkerPool::ScheduleTasks(task ? task->internal_ : NULL);
+bool RasterWorkerPool::ForceUploadToComplete(const RasterTask& raster_task) {
+  return false;
+}
+
+void RasterWorkerPool::SetRasterTasks(RasterTask::Queue* queue) {
+  raster_tasks_.swap(queue->tasks_);
+}
+
+void RasterWorkerPool::ScheduleRasterTasks(
+    internal::WorkerPoolTask::TaskVector* raster_tasks) {
+  if (raster_tasks->empty()) {
+    WorkerPool::ScheduleTasks(NULL);
+    return;
+  }
+
+  scoped_refptr<WorkerPoolTaskImpl> root(new WorkerPoolTaskImpl(raster_tasks));
+  WorkerPool::ScheduleTasks(root);
 }
 
 }  // namespace cc