cc: Remove task completion check interval from WorkerPool.

This moves the task completion check interval into the
PixelBufferRasterWorkerPool and allows ImageRasterWorkerPool
to run at full speed.

BUG=246185
TEST=

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

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

4 files changed, 21 insertions, 118 deletions

diff --git a/cc/base/worker_pool.cc b/cc/base/worker_pool.cc
index 46eeb31..024372e 100644
--- a/cc/base/worker_pool.cc
+++ b/cc/base/worker_pool.cc
@@ -100,9 +100,7 @@ bool WorkerPoolTask::HasCompleted() const {
 // by |lock_|.
 class WorkerPool::Inner : public base::DelegateSimpleThread::Delegate {
  public:
-  Inner(WorkerPool* worker_pool,
-        size_t num_threads,
-        const std::string& thread_name_prefix);
+  Inner(size_t num_threads, const std::string& thread_name_prefix);
   virtual ~Inner();
 
   void Shutdown();
@@ -116,8 +114,8 @@ class WorkerPool::Inner : public base::DelegateSimpleThread::Delegate {
   // ScheduleTasks().
   void ScheduleTasks(internal::WorkerPoolTask* root);
 
-  // Collect all completed tasks in |completed_tasks|. Returns true if idle.
-  bool CollectCompletedTasks(TaskDeque* completed_tasks);
+  // Collect all completed tasks in |completed_tasks|.
+  void CollectCompletedTasks(TaskDeque* completed_tasks);
 
  private:
   class ScheduledTask {
@@ -148,11 +146,6 @@ class WorkerPool::Inner : public base::DelegateSimpleThread::Delegate {
   static void BuildScheduledTaskMap(
       internal::WorkerPoolTask* root, ScheduledTaskMap* scheduled_tasks);
 
-  // Collect all completed tasks by swapping the contents of
-  // |completed_tasks| and |completed_tasks_|. Lock must be acquired
-  // before calling this function. Returns true if idle.
-  bool CollectCompletedTasksWithLockAcquired(TaskDeque* completed_tasks);
-
   // Overridden from base::DelegateSimpleThread:
   virtual void Run() OVERRIDE;
 
@@ -195,9 +188,8 @@ class WorkerPool::Inner : public base::DelegateSimpleThread::Delegate {
   DISALLOW_COPY_AND_ASSIGN(Inner);
 };
 
-WorkerPool::Inner::Inner(WorkerPool* worker_pool,
-                         size_t num_threads,
-                         const std::string& thread_name_prefix)
+WorkerPool::Inner::Inner(
+    size_t num_threads, const std::string& thread_name_prefix)
     : lock_(),
       has_ready_to_run_tasks_cv_(&lock_),
       next_thread_index_(0),
@@ -324,20 +316,11 @@ void WorkerPool::Inner::ScheduleTasks(internal::WorkerPoolTask* root) {
   }
 }
 
-bool WorkerPool::Inner::CollectCompletedTasks(TaskDeque* completed_tasks) {
+void WorkerPool::Inner::CollectCompletedTasks(TaskDeque* completed_tasks) {
   base::AutoLock lock(lock_);
 
-  return CollectCompletedTasksWithLockAcquired(completed_tasks);
-}
-
-bool WorkerPool::Inner::CollectCompletedTasksWithLockAcquired(
-    TaskDeque* completed_tasks) {
-  lock_.AssertAcquired();
-
   DCHECK_EQ(0u, completed_tasks->size());
   completed_tasks->swap(completed_tasks_);
-
-  return running_tasks_.empty() && pending_tasks_.empty();
 }
 
 void WorkerPool::Inner::Run() {
@@ -477,16 +460,9 @@ void WorkerPool::Inner::BuildScheduledTaskMap(
 }
 
 WorkerPool::WorkerPool(size_t num_threads,
-                       base::TimeDelta check_for_completed_tasks_delay,
                        const std::string& thread_name_prefix)
-    : client_(NULL),
-      origin_loop_(base::MessageLoopProxy::current()),
-      check_for_completed_tasks_delay_(check_for_completed_tasks_delay),
-      check_for_completed_tasks_pending_(false),
-      in_dispatch_completion_callbacks_(false),
-      inner_(make_scoped_ptr(new Inner(this,
-                                       num_threads,
-                                       thread_name_prefix))) {
+    : in_dispatch_completion_callbacks_(false),
+      inner_(make_scoped_ptr(new Inner(num_threads, thread_name_prefix))) {
 }
 
 WorkerPool::~WorkerPool() {
@@ -498,23 +474,6 @@ void WorkerPool::Shutdown() {
   DCHECK(!in_dispatch_completion_callbacks_);
 
   inner_->Shutdown();
-
-  TaskDeque completed_tasks;
-  inner_->CollectCompletedTasks(&completed_tasks);
-  DispatchCompletionCallbacks(&completed_tasks);
-}
-
-void WorkerPool::ScheduleCheckForCompletedTasks() {
-  if (check_for_completed_tasks_pending_)
-    return;
-  check_for_completed_tasks_callback_.Reset(
-      base::Bind(&WorkerPool::CheckForCompletedTasks,
-                 base::Unretained(this)));
-  origin_loop_->PostDelayedTask(
-      FROM_HERE,
-      check_for_completed_tasks_callback_.callback(),
-      check_for_completed_tasks_delay_);
-  check_for_completed_tasks_pending_ = true;
 }
 
 void WorkerPool::CheckForCompletedTasks() {
@@ -522,40 +481,27 @@ void WorkerPool::CheckForCompletedTasks() {
 
   DCHECK(!in_dispatch_completion_callbacks_);
 
-  check_for_completed_tasks_callback_.Cancel();
-  check_for_completed_tasks_pending_ = false;
-
   TaskDeque completed_tasks;
-
-  // Schedule another check for completed tasks if not idle.
-  if (!inner_->CollectCompletedTasks(&completed_tasks))
-    ScheduleCheckForCompletedTasks();
-
+  inner_->CollectCompletedTasks(&completed_tasks);
   DispatchCompletionCallbacks(&completed_tasks);
 }
 
 void WorkerPool::DispatchCompletionCallbacks(TaskDeque* completed_tasks) {
   TRACE_EVENT0("cc", "WorkerPool::DispatchCompletionCallbacks");
 
-  // Early out when |completed_tasks| is empty to prevent unnecessary
-  // call to DidFinishDispatchingWorkerPoolCompletionCallbacks().
-  if (completed_tasks->empty())
-    return;
-
   // Worker pool instance is not reentrant while processing completed tasks.
   in_dispatch_completion_callbacks_ = true;
 
   while (!completed_tasks->empty()) {
-    scoped_refptr<internal::WorkerPoolTask> task = completed_tasks->front();
-    completed_tasks->pop_front();
+    internal::WorkerPoolTask* task = completed_tasks->front();
+
     task->DidComplete();
     task->DispatchCompletionCallback();
+
+    completed_tasks->pop_front();
   }
 
   in_dispatch_completion_callbacks_ = false;
-
-  DCHECK(client_);
-  client_->DidFinishDispatchingWorkerPoolCompletionCallbacks();
 }
 
 void WorkerPool::ScheduleTasks(internal::WorkerPoolTask* root) {
@@ -563,10 +509,6 @@ void WorkerPool::ScheduleTasks(internal::WorkerPoolTask* root) {
 
   DCHECK(!in_dispatch_completion_callbacks_);
 
-  // Schedule check for completed tasks.
-  if (root)
-    ScheduleCheckForCompletedTasks();
-
   inner_->ScheduleTasks(root);
 }
 
diff --git a/cc/base/worker_pool.h b/cc/base/worker_pool.h
index 758302c..047023f 100644
--- a/cc/base/worker_pool.h
+++ b/cc/base/worker_pool.h
@@ -55,14 +55,6 @@ class CC_EXPORT WorkerPoolTask
 
 }  // namespace internal
 
-class CC_EXPORT WorkerPoolClient {
- public:
-  virtual void DidFinishDispatchingWorkerPoolCompletionCallbacks() = 0;
-
- protected:
-  virtual ~WorkerPoolClient() {}
-};
-
 // A worker thread pool that runs tasks provided by task graph and
 // guarantees completion of all pending tasks at shutdown.
 class CC_EXPORT WorkerPool {
@@ -73,18 +65,11 @@ class CC_EXPORT WorkerPool {
   // completed.
   virtual void Shutdown();
 
-  // Set a new client.
-  void SetClient(WorkerPoolClient* client) {
-    client_ = client;
-  }
-
   // Force a check for completed tasks.
   virtual void CheckForCompletedTasks();
 
  protected:
-  WorkerPool(size_t num_threads,
-             base::TimeDelta check_for_completed_tasks_delay,
-             const std::string& thread_name_prefix);
+  WorkerPool(size_t num_threads, const std::string& thread_name_prefix);
 
   void ScheduleTasks(internal::WorkerPoolTask* root);
 
@@ -94,14 +79,8 @@ class CC_EXPORT WorkerPool {
 
   typedef std::deque<scoped_refptr<internal::WorkerPoolTask> > TaskDeque;
 
-  void ScheduleCheckForCompletedTasks();
   void DispatchCompletionCallbacks(TaskDeque* completed_tasks);
 
-  WorkerPoolClient* client_;
-  scoped_refptr<base::MessageLoopProxy> origin_loop_;
-  base::CancelableClosure check_for_completed_tasks_callback_;
-  base::TimeDelta check_for_completed_tasks_delay_;
-  bool check_for_completed_tasks_pending_;
   bool in_dispatch_completion_callbacks_;
 
   // Hide the gory details of the worker pool in |inner_|.
diff --git a/cc/base/worker_pool_perftest.cc b/cc/base/worker_pool_perftest.cc
index c203526..2fd07d9 100644
--- a/cc/base/worker_pool_perftest.cc
+++ b/cc/base/worker_pool_perftest.cc
@@ -61,7 +61,7 @@ class PerfControlTaskImpl : public internal::WorkerPoolTask {
 
 class PerfWorkerPool : public WorkerPool {
  public:
-  PerfWorkerPool() : WorkerPool(1, base::TimeDelta::FromDays(1024), "test") {}
+  PerfWorkerPool() : WorkerPool(1, "test") {}
   virtual ~PerfWorkerPool() {}
 
   static scoped_ptr<PerfWorkerPool> Create() {
@@ -73,23 +73,19 @@ class PerfWorkerPool : public WorkerPool {
   }
 };
 
-class WorkerPoolPerfTest : public testing::Test,
-                           public WorkerPoolClient {
+class WorkerPoolPerfTest : public testing::Test {
  public:
   WorkerPoolPerfTest() : num_runs_(0) {}
 
   // Overridden from testing::Test:
   virtual void SetUp() OVERRIDE {
     worker_pool_ = PerfWorkerPool::Create();
-    worker_pool_->SetClient(this);
   }
   virtual void TearDown() OVERRIDE {
     worker_pool_->Shutdown();
+    worker_pool_->CheckForCompletedTasks();
   }
 
-  // Overridden from WorkerPoolClient:
-  virtual void DidFinishDispatchingWorkerPoolCompletionCallbacks() OVERRIDE {}
-
   void EndTest() {
     elapsed_ = base::TimeTicks::HighResNow() - start_time_;
   }
diff --git a/cc/base/worker_pool_unittest.cc b/cc/base/worker_pool_unittest.cc
index 9dfe565..fd50ae7 100644
--- a/cc/base/worker_pool_unittest.cc
+++ b/cc/base/worker_pool_unittest.cc
@@ -46,7 +46,7 @@ class FakeTaskImpl : public internal::WorkerPoolTask {
 
 class FakeWorkerPool : public WorkerPool {
  public:
-  FakeWorkerPool() : WorkerPool(1, base::TimeDelta::FromDays(1024), "test") {}
+  FakeWorkerPool() : WorkerPool(1, "test") {}
   virtual ~FakeWorkerPool() {}
 
   static scoped_ptr<FakeWorkerPool> Create() {
@@ -89,10 +89,9 @@ class FakeWorkerPool : public WorkerPool {
   scoped_ptr<CompletionEvent> scheduled_tasks_completion_;
 };
 
-class WorkerPoolTest : public testing::Test,
-                       public WorkerPoolClient {
+class WorkerPoolTest : public testing::Test {
  public:
-  WorkerPoolTest() : finish_dispatching_completion_callbacks_count_(0) {}
+  WorkerPoolTest() {}
   virtual ~WorkerPoolTest() {}
 
   // Overridden from testing::Test:
@@ -103,19 +102,14 @@ class WorkerPoolTest : public testing::Test,
     worker_pool_->Shutdown();
   }
 
-  // Overridden from WorkerPoolClient:
-  virtual void DidFinishDispatchingWorkerPoolCompletionCallbacks() OVERRIDE {
-    ++finish_dispatching_completion_callbacks_count_;
-  }
-
   void Reset() {
     worker_pool_ = FakeWorkerPool::Create();
-    worker_pool_->SetClient(this);
   }
 
   void RunAllTasksAndReset() {
     worker_pool_->WaitForTasksToComplete();
     worker_pool_->Shutdown();
+    worker_pool_->CheckForCompletedTasks();
     Reset();
   }
 
@@ -139,21 +133,15 @@ class WorkerPoolTest : public testing::Test,
     return on_task_completed_ids_;
   }
 
-  unsigned finish_dispatching_completion_callbacks_count() {
-    return finish_dispatching_completion_callbacks_count_;
-  }
-
  private:
   scoped_ptr<FakeWorkerPool> worker_pool_;
   std::vector<unsigned> run_task_ids_;
   std::vector<unsigned> on_task_completed_ids_;
-  unsigned finish_dispatching_completion_callbacks_count_;
 };
 
 TEST_F(WorkerPoolTest, Basic) {
   EXPECT_EQ(0u, run_task_ids().size());
   EXPECT_EQ(0u, on_task_completed_ids().size());
-  EXPECT_EQ(0u, finish_dispatching_completion_callbacks_count());
 
   worker_pool()->ScheduleTasks(
       base::Bind(&WorkerPoolTest::RunTask, base::Unretained(this), 0u),
@@ -164,7 +152,6 @@ TEST_F(WorkerPoolTest, Basic) {
 
   EXPECT_EQ(1u, run_task_ids().size());
   EXPECT_EQ(1u, on_task_completed_ids().size());
-  EXPECT_EQ(1u, finish_dispatching_completion_callbacks_count());
 
   worker_pool()->ScheduleTasks(
       base::Bind(&WorkerPoolTest::RunTask, base::Unretained(this), 0u),
@@ -175,7 +162,6 @@ TEST_F(WorkerPoolTest, Basic) {
 
   EXPECT_EQ(3u, run_task_ids().size());
   EXPECT_EQ(3u, on_task_completed_ids().size());
-  EXPECT_EQ(2u, finish_dispatching_completion_callbacks_count());
 }
 
 TEST_F(WorkerPoolTest, Dependencies) {