cc: Fix incorrect completion of tiles and missing "ready to activate" signal.

cc/resources/pixel_buffer_raster_worker_pool.cc

 // 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.
 
 #include "cc/resources/pixel_buffer_raster_worker_pool.h"
 
 #include "base/containers/stack_container.h"
 #include "base/debug/trace_event.h"
 #include "base/values.h"
 #include "cc/debug/traced_value.h"
@@ skipping 79 matching lines @@
     const NodeVector::ContainerType& dependencies) {
   for (NodeVector::ContainerType::const_iterator it = dependencies.begin();
        it != dependencies.end(); ++it) {
     internal::GraphNode* dependency = *it;
 
     node->add_dependency();
     dependency->add_dependent(node);
   }
 }
 
+// Only used as std::find_if predicate for DCHECKs.
+bool WasCanceled(const internal::RasterWorkerPoolTask* task) {
+  return task->WasCanceled();
+}
+
 }  // namespace
 
 PixelBufferRasterWorkerPool::PixelBufferRasterWorkerPool(
     ResourceProvider* resource_provider,
     size_t num_threads)
     : RasterWorkerPool(resource_provider, num_threads),
       shutdown_(false),
       scheduled_raster_task_count_(0),
       bytes_pending_upload_(0),
       has_performed_uploads_since_last_flush_(false),
@@ skipping 83 matching lines @@
   }
 
   tasks_required_for_activation_.clear();
   for (TaskMap::iterator it = new_pixel_buffer_tasks.begin();
        it != new_pixel_buffer_tasks.end(); ++it) {
     internal::RasterWorkerPoolTask* task = it->first;
     if (IsRasterTaskRequiredForActivation(task))
       tasks_required_for_activation_.insert(task);
   }
 
+  // |tasks_required_for_activation_| contains all tasks that need to
+  // complete before we can send a "ready to activate" signal. Tasks
+  // that have already completed should not be part of this set.
+  for (TaskDeque::const_iterator it = completed_tasks_.begin();
+       it != completed_tasks_.end(); ++it) {
+    tasks_required_for_activation_.erase(*it);
+  }
+
   pixel_buffer_tasks_.swap(new_pixel_buffer_tasks);
 
   // Check for completed tasks when ScheduleTasks() is called as
   // priorities might have changed and this maximizes the number
   // of top priority tasks that are scheduled.
   RasterWorkerPool::CheckForCompletedTasks();
   CheckForCompletedUploads();
   FlushUploads();
 
   // Schedule new tasks.
@@ skipping 189 matching lines @@
   TRACE_EVENT_ASYNC_STEP1(
       "cc", "ScheduledTasks", this, StateName(),
       "state", TracedValue::FromValue(StateAsValue().release()));
 
   // Schedule another check for completed raster tasks while there are
   // pending raster tasks or pending uploads.
   if (HasPendingTasks())
     ScheduleCheckForCompletedRasterTasks();
 
   // Generate client notifications.
-  if (will_notify_client_that_no_tasks_required_for_activation_are_pending)
+  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()->DidFinishedRunningTasksRequiredForActivation();
+  }
   if (will_notify_client_that_no_tasks_are_pending) {
     TRACE_EVENT_ASYNC_END0("cc", "ScheduledTasks", this);
+    DCHECK(!HasPendingTasksRequiredForActivation());
     client()->DidFinishedRunningTasks();
   }
 }
 
 void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
   TRACE_EVENT0("cc", "PixelBufferRasterWorkerPool::ScheduleMoreTasks");
 
   enum RasterTaskType {
     PREPAINT_TYPE = 0,
     REQUIRED_FOR_ACTIVATION_TYPE = 1,
@@ skipping 149 matching lines @@
                "was_canceled", was_canceled,
                "needs_upload", needs_upload);
 
   DCHECK(pixel_buffer_tasks_.find(task.get()) != pixel_buffer_tasks_.end());
 
   // Balanced with MapPixelBuffer() call in ScheduleMoreTasks().
   resource_provider()->UnmapPixelBuffer(task->resource()->id());
 
   if (!needs_upload) {
     resource_provider()->ReleasePixelBuffer(task->resource()->id());
+
+    if (was_canceled) {
+      // When priorites change, a raster task can be canceled as a result of
+      // no longer being of high enough priority to fit in our throttled
+      // raster task budget. The task has not yet completed in this case.
+      RasterTaskVector::const_iterator it = std::find(raster_tasks().begin(),
+                                                      raster_tasks().end(),
+                                                      task);
+      if (it != raster_tasks().end()) {
+        pixel_buffer_tasks_[task.get()] = NULL;
+        return;
+      }
+    }
+
     task->DidRun(was_canceled);
     DCHECK(std::find(completed_tasks_.begin(),
                      completed_tasks_.end(),
                      task) == completed_tasks_.end());
     completed_tasks_.push_back(task);
     tasks_required_for_activation_.erase(task);
     return;
   }
 
+  DCHECK(!was_canceled);

[vmpstr, 2013/07/11 17:55:12]

nit: I would also prefer DCHECK(!was_canceled || !needs_upload) (was_canceled
implies !needs_upload) above the if (!needs_upload) check.

[vmpstr, 2013/07/11 18:12:55]

Nevermind this, I missed the fact that the if returns at the end.

+
   resource_provider()->BeginSetPixels(task->resource()->id());
   has_performed_uploads_since_last_flush_ = true;
 
   bytes_pending_upload_ += task->resource()->bytes();
   tasks_with_pending_upload_.push_back(task);
 }
 
 unsigned PixelBufferRasterWorkerPool::PendingRasterTaskCount() const {
   unsigned num_completed_raster_tasks =
       tasks_with_pending_upload_.size() + completed_tasks_.size();
@@ skipping 39 matching lines @@
 
   throttle_state->SetInteger("bytes_available_for_upload",
                              kMaxPendingUploadBytes - bytes_pending_upload_);
   throttle_state->SetInteger("bytes_pending_upload", bytes_pending_upload_);
   throttle_state->SetInteger("scheduled_raster_task_count",
                              scheduled_raster_task_count_);
   return throttle_state.PassAs<base::Value>();
 }
 
 }  // namespace cc