cc: Simplify raster task completion notification logic

cc/resources/pixel_buffer_raster_worker_pool.cc

Index: cc/resources/pixel_buffer_raster_worker_pool.cc
diff --git a/cc/resources/pixel_buffer_raster_worker_pool.cc b/cc/resources/pixel_buffer_raster_worker_pool.cc
index 011e5147e066e4c3e294aceb14ae22bc0b931f31..b6c7358e0eb7ee6741338a0e491bc89f084ea437 100644
--- a/cc/resources/pixel_buffer_raster_worker_pool.cc
+++ b/cc/resources/pixel_buffer_raster_worker_pool.cc
@@ -250,29 +250,25 @@ void PixelBufferRasterWorkerPool::CheckForCompletedTasks() {
 }
 
 void PixelBufferRasterWorkerPool::OnRasterTasksFinished() {
-  // |should_notify_client_if_no_tasks_are_pending_| can be set to false as
-  // a result of a scheduled CheckForCompletedRasterTasks() call. No need to
-  // perform another check in that case as we've already notified the client.
-  if (!should_notify_client_if_no_tasks_are_pending_)
+  if (CheckForCompletedRasterTasks() != ALL_TASKS_COMPLETED)
     return;
 
-  // Call CheckForCompletedRasterTasks() when we've finished running all
-  // raster tasks needed since last time ScheduleTasks() was called.
-  // This reduces latency between the time when all tasks have finished
-  // running and the time when the client is notified.
-  CheckForCompletedRasterTasks();
+  TRACE_EVENT_ASYNC_END0("cc", "ScheduledTasks", this);
+  DCHECK(!HasPendingTasksRequiredForActivation());
+  client()->DidFinishRunningTasks();

[reveman, 2013/12/14 04:31:01]

Hm, what happens if CheckForCompletedRasterTasks() returns TASKS_PENDING because
some uploads are pending? Seems like this client callback is never going to
happen in that case or am I missing something?

What happens in ToT right now is that a periodic check can sometimes result in
us detecting completion and making the client callback before the signal has
been received, right?  I think we can keep the existing code but just make sure
will_notify_client_that_no_tasks_are_pending is not true unless we've received
the ::OnRasterTasksFinished() signal. Wdyt?

[Sami, 2013/12/17 16:27:38]

This shouldn't be any different than before, i.e., DidFinishRunningTasks() will
never get signaled while HasPendingTasks() is true. AFAICT pending uploads don't
factor into that, since they don't get their own worker pool tasks.
Yes, that's exactly the problem I was trying to fix, but perhaps tried to change
things around a little too much. I've now gone for the simpler approach you
suggested.

[reveman, 2013/12/17 20:46:25]

The problem was that you removed the client callback in
CheckForCompletedRasterTasks() which will happend when the uploads complete. Not
a problem in latest patch though.

[Sami, 2013/12/18 12:05:06]

Ah, now I see. I didn't realize pending uploads were also keeping the
CheckForCompletedRasterTasks() tick going.

 }
 
 void PixelBufferRasterWorkerPool::OnRasterTasksRequiredForActivationFinished() {
-  // Analogous to OnRasterTasksFinished(), there's no need to call
-  // CheckForCompletedRasterTasks() if the client has already been notified.
-  if (!should_notify_client_if_no_tasks_required_for_activation_are_pending_)
+  TaskQueueCompletionStatus completion_status = CheckForCompletedRasterTasks();
+  if (completion_status != TASKS_REQUIRED_FOR_ACTIVATION_COMPLETED &&
+      completion_status != ALL_TASKS_COMPLETED)
     return;
 
-  // This reduces latency between the time when all tasks required for
-  // activation have finished running and the time when the client is
-  // notified.
-  CheckForCompletedRasterTasks();
+  DCHECK(std::find_if(raster_tasks_required_for_activation().begin(),
+                      raster_tasks_required_for_activation().end(),
+                      WasCanceled) ==
+        raster_tasks_required_for_activation().end());
+  client()->DidFinishRunningTasksRequiredForActivation();
 }
 
 void PixelBufferRasterWorkerPool::FlushUploads() {
@@ -361,7 +357,8 @@ void PixelBufferRasterWorkerPool::ScheduleCheckForCompletedRasterTasks() {
     return;
 
   check_for_completed_raster_tasks_callback_.Reset(
-      base::Bind(&PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks,
+      base::Bind(base::IgnoreResult(
+                    &PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks),
                  base::Unretained(this)));
   base::MessageLoopProxy::current()->PostDelayedTask(
       FROM_HERE,
@@ -370,7 +367,8 @@ void PixelBufferRasterWorkerPool::ScheduleCheckForCompletedRasterTasks() {
   check_for_completed_raster_tasks_pending_ = true;
 }
 
-void PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() {
+PixelBufferRasterWorkerPool::TaskQueueCompletionStatus
+    PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() {
   TRACE_EVENT0(
       "cc", "PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks");
 
@@ -383,19 +381,11 @@ void PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() {
   CheckForCompletedUploads();
   FlushUploads();
 
-  // Determine what client notifications to generate.
-  bool will_notify_client_that_no_tasks_required_for_activation_are_pending =
-      (should_notify_client_if_no_tasks_required_for_activation_are_pending_ &&
-       !HasPendingTasksRequiredForActivation());
-  bool will_notify_client_that_no_tasks_are_pending =
-      (should_notify_client_if_no_tasks_are_pending_ &&
-       !HasPendingTasks());
-
-  // Adjust the need to generate notifications before scheduling more tasks.
-  should_notify_client_if_no_tasks_required_for_activation_are_pending_ &=
-      !will_notify_client_that_no_tasks_required_for_activation_are_pending;
-  should_notify_client_if_no_tasks_are_pending_ &=
-      !will_notify_client_that_no_tasks_are_pending;
+  TaskQueueCompletionStatus completion_status = TASKS_PENDING;
+  if (!HasPendingTasks())
+    completion_status = ALL_TASKS_COMPLETED;
+  else if (!HasPendingTasksRequiredForActivation())
+    completion_status = TASKS_REQUIRED_FOR_ACTIVATION_COMPLETED;
 
   scheduled_raster_task_count_ = 0;
   if (PendingRasterTaskCount())
@@ -409,20 +399,7 @@ void PixelBufferRasterWorkerPool::CheckForCompletedRasterTasks() {
   // pending raster tasks or pending uploads.
   if (HasPendingTasks())
     ScheduleCheckForCompletedRasterTasks();
-
-  // Generate client notifications.
-  if (will_notify_client_that_no_tasks_required_for_activation_are_pending) {
-    DCHECK(std::find_if(raster_tasks_required_for_activation().begin(),
-                        raster_tasks_required_for_activation().end(),
-                        WasCanceled) ==
-          raster_tasks_required_for_activation().end());
-    client()->DidFinishRunningTasksRequiredForActivation();
-  }
-  if (will_notify_client_that_no_tasks_are_pending) {
-    TRACE_EVENT_ASYNC_END0("cc", "ScheduledTasks", this);
-    DCHECK(!HasPendingTasksRequiredForActivation());
-    client()->DidFinishRunningTasks();
-  }
+  return completion_status;
 }
 
 void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {