cc: Fix logic for detecting when raster tasks were throttled

The PixelBufferRasterWorkerPool checks whether raster tasks were
throttled to avoid notifying the client that all pending tasks were
completed too early. The current logic for this fails if tasks from the
previous graph were still active when we schedule a new graph. In these
situations we end up thinking some tasks were throttled and will not
schedule the final sentinel task to indicate work completion. This can
lead to delayed tree activation.

The fix is to only consider tasks from the new work queue when seeing if
any got throttled.

BUG=331534

Review URL: https://codereview.chromium.org/131763003

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

cc/resources/pixel_buffer_raster_worker_pool.cc

@@ -447,6 +447,7 @@ void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
   TaskGraph graph;
 
   size_t bytes_pending_upload = bytes_pending_upload_;
+  bool did_throttle_raster_tasks = false;
 
   for (RasterTaskVector::const_iterator it = raster_tasks().begin();
        it != raster_tasks().end(); ++it) {
@@ -468,8 +469,10 @@ void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
     // All raster tasks need to be throttled by bytes of pending uploads.
     size_t new_bytes_pending_upload = bytes_pending_upload;
     new_bytes_pending_upload += task->resource()->bytes();
-    if (new_bytes_pending_upload > max_bytes_pending_upload_)
+    if (new_bytes_pending_upload > max_bytes_pending_upload_) {
+      did_throttle_raster_tasks = true;
       break;
+    }
 
     internal::WorkerPoolTask* pixel_buffer_task = pixel_buffer_it->second.get();
 
@@ -483,8 +486,10 @@ void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
     size_t scheduled_raster_task_count =
         tasks[PREPAINT_TYPE].container().size() +
         tasks[REQUIRED_FOR_ACTIVATION_TYPE].container().size();
-    if (scheduled_raster_task_count >= kMaxScheduledRasterTasks)
+    if (scheduled_raster_task_count >= kMaxScheduledRasterTasks) {
+      did_throttle_raster_tasks = true;
       break;
+    }
 
     // Update |bytes_pending_upload| now that task has cleared all
     // throttling limits.
@@ -561,7 +566,7 @@ void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
   DCHECK_LE(scheduled_raster_task_count, PendingRasterTaskCount());
   // Schedule OnRasterTasksFinished call only when notification is pending
   // and throttling is not preventing all pending tasks from being scheduled.
-  if (scheduled_raster_task_count == PendingRasterTaskCount() &&
+  if (!did_throttle_raster_tasks &&
       should_notify_client_if_no_tasks_are_pending_) {
     new_raster_finished_task = CreateRasterFinishedTask();
     internal::GraphNode* raster_finished_node =

cc/resources/raster_worker_pool_unittest.cc

@@ -70,6 +70,39 @@ class TestRasterWorkerPoolTaskImpl : public internal::RasterWorkerPoolTask {
   DISALLOW_COPY_AND_ASSIGN(TestRasterWorkerPoolTaskImpl);
 };
 
+class BlockingRasterWorkerPoolTaskImpl : public TestRasterWorkerPoolTaskImpl {
+ public:
+  BlockingRasterWorkerPoolTaskImpl(const Resource* resource,
+                                   const Reply& reply,
+                                   base::Lock* lock,
+                                   TaskVector* dependencies,
+                                   bool use_gpu_rasterization)
+      : TestRasterWorkerPoolTaskImpl(resource,
+                                     reply,
+                                     dependencies,
+                                     use_gpu_rasterization),
+        lock_(lock) {}
+
+  // Overridden from TestRasterWorkerPoolTaskImpl:
+  virtual bool RunOnWorkerThread(unsigned thread_index,
+                                 void* buffer,
+                                 gfx::Size size,
+                                 int stride) OVERRIDE {
+    base::AutoLock lock(*lock_);
+    return TestRasterWorkerPoolTaskImpl::RunOnWorkerThread(
+        thread_index, buffer, size, stride);
+  }
+  virtual void CompleteOnOriginThread() OVERRIDE {}
+
+ protected:
+  virtual ~BlockingRasterWorkerPoolTaskImpl() {}
+
+ private:
+  base::Lock* lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlockingRasterWorkerPoolTaskImpl);
+};
+
 class RasterWorkerPoolTest : public testing::Test,
                              public RasterWorkerPoolClient  {
  public:
@@ -190,6 +223,27 @@ class RasterWorkerPoolTest : public testing::Test,
             use_gpu_rasterization)));
   }
 
+  void AppendBlockingTask(unsigned id, base::Lock* lock) {
+    const gfx::Size size(1, 1);
+
+    scoped_ptr<ScopedResource> resource(
+        ScopedResource::Create(resource_provider()));
+    resource->Allocate(size, ResourceProvider::TextureUsageAny, RGBA_8888);
+    const Resource* const_resource = resource.get();
+
+    RasterWorkerPool::Task::Set empty;
+    tasks_.push_back(
+        RasterWorkerPool::RasterTask(new BlockingRasterWorkerPoolTaskImpl(
+            const_resource,
+            base::Bind(&RasterWorkerPoolTest::OnTaskCompleted,
+                       base::Unretained(this),
+                       base::Passed(&resource),
+                       id),
+            lock,
+            &empty.tasks_,
+            false)));
+  }
+
   virtual void OnTaskCompleted(
       scoped_ptr<ScopedResource> resource,
       unsigned id,
@@ -379,6 +433,40 @@ class RasterWorkerPoolTestFailedMapResource : public RasterWorkerPoolTest {
 
 PIXEL_BUFFER_AND_IMAGE_TEST_F(RasterWorkerPoolTestFailedMapResource);
 
+class RasterWorkerPoolTestFalseThrottling : public RasterWorkerPoolTest {
+ public:
+  // Overridden from RasterWorkerPoolTest:
+  virtual void BeginTest() OVERRIDE {
+    // This test checks that replacing a pending raster task with another does
+    // not prevent the DidFinishRunningTasks notification from being sent.
+
+    // Schedule a task that is prevented from completing with a lock.
+    lock_.Acquire();
+    AppendBlockingTask(0u, &lock_);
+    ScheduleTasks();
+
+    // Schedule another task to replace the still-pending task. Because the old
+    // task is not a throttled task in the new task set, it should not prevent
+    // DidFinishRunningTasks from getting signaled.
+    tasks_.clear();
+    AppendTask(1u, false);
+    ScheduleTasks();
+
+    // Unblock the first task to allow the second task to complete.
+    lock_.Release();
+  }
+
+  virtual void AfterTest() OVERRIDE {}
+
+  virtual void DidFinishRunningTasks() OVERRIDE {
+    EndTest();
+  }
+
+  base::Lock lock_;
+};
+
+PIXEL_BUFFER_AND_IMAGE_TEST_F(RasterWorkerPoolTestFalseThrottling);
+
 }  // namespace
 
 }  // namespace cc