From 604e202d3548b701900c3771f9517103bb91486c Mon Sep 17 00:00:00 2001
From: "enne@chromium.org"
 <enne@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 22 Aug 2014 19:53:15 +0000
Subject: [PATCH] cc: Don't infinitely throttle large raster tasks

Previously, if a raster task had a resource size that was larger than
the throttling limit for the raster worker, that task would never get
scheduled.  This leads to starvation for that task, preventing tree
activation, and causing freezes.

The fix is to always allow such large tasks if it is the first task.

R=reveman@chromium.org
BUG=403446

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

Cr-Commit-Position: refs/heads/master@{#291484}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291484 0039d316-1c4b-4281-b951-d872f2087c98
---
 .../pixel_buffer_raster_worker_pool.cc        |  7 ++--
 cc/resources/raster_worker_pool_unittest.cc   | 34 ++++++++++++++++---
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/cc/resources/pixel_buffer_raster_worker_pool.cc b/cc/resources/pixel_buffer_raster_worker_pool.cc
index d7554736942a6..67ae593bde0a5 100644
--- a/cc/resources/pixel_buffer_raster_worker_pool.cc
+++ b/cc/resources/pixel_buffer_raster_worker_pool.cc
@@ -515,10 +515,13 @@ void PixelBufferRasterWorkerPool::ScheduleMoreTasks() {
       continue;
     }
 
-    // All raster tasks need to be throttled by bytes of pending uploads.
+    // All raster tasks need to be throttled by bytes of pending uploads,
+    // but if it's the only task allow it to complete no matter what its size,
+    // to prevent starvation of the task queue.
     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_ &&
+        bytes_pending_upload) {
       did_throttle_raster_tasks = true;
       if (item.required_for_activation)
         did_throttle_raster_tasks_required_for_activation = true;
diff --git a/cc/resources/raster_worker_pool_unittest.cc b/cc/resources/raster_worker_pool_unittest.cc
index 49912379b3003..f93e0fc714308 100644
--- a/cc/resources/raster_worker_pool_unittest.cc
+++ b/cc/resources/raster_worker_pool_unittest.cc
@@ -27,6 +27,11 @@
 namespace cc {
 namespace {
 
+const size_t kMaxTransferBufferUsageBytes = 10000U;
+// A resource of this dimension^2 * 4 must be greater than the above transfer
+// buffer constant.
+const size_t kLargeResourceDimension = 1000U;
+
 enum RasterWorkerPoolType {
   RASTER_WORKER_POOL_TYPE_PIXEL_BUFFER,
   RASTER_WORKER_POOL_TYPE_IMAGE,
@@ -126,7 +131,7 @@ class RasterWorkerPoolTest
             RasterWorkerPool::GetTaskGraphRunner(),
             context_provider_.get(),
             resource_provider_.get(),
-            std::numeric_limits<size_t>::max());
+            kMaxTransferBufferUsageBytes);
         break;
       case RASTER_WORKER_POOL_TYPE_IMAGE:
         raster_worker_pool_ = ImageRasterWorkerPool::Create(
@@ -203,9 +208,7 @@ class RasterWorkerPoolTest
     raster_worker_pool_->AsRasterizer()->ScheduleTasks(&queue);
   }
 
-  void AppendTask(unsigned id) {
-    const gfx::Size size(1, 1);
-
+  void AppendTask(unsigned id, const gfx::Size& size) {
     scoped_ptr<ScopedResource> resource(
         ScopedResource::Create(resource_provider_.get()));
     resource->Allocate(size, ResourceProvider::TextureHintImmutable, RGBA_8888);
@@ -221,6 +224,8 @@ class RasterWorkerPoolTest
         &empty));
   }
 
+  void AppendTask(unsigned id) { AppendTask(id, gfx::Size(1, 1)); }
+
   void AppendBlockingTask(unsigned id, base::Lock* lock) {
     const gfx::Size size(1, 1);
 
@@ -324,6 +329,27 @@ TEST_P(RasterWorkerPoolTest, FalseThrottling) {
   RunMessageLoopUntilAllTasksHaveCompleted();
 }
 
+TEST_P(RasterWorkerPoolTest, LargeResources) {
+  gfx::Size size(kLargeResourceDimension, kLargeResourceDimension);
+
+  {
+    // Verify a resource of this size is larger than the transfer buffer.
+    scoped_ptr<ScopedResource> resource(
+        ScopedResource::Create(resource_provider_.get()));
+    resource->Allocate(size, ResourceProvider::TextureHintImmutable, RGBA_8888);
+    EXPECT_GE(resource->bytes(), kMaxTransferBufferUsageBytes);
+  }
+
+  AppendTask(0u, size);
+  AppendTask(1u, size);
+  AppendTask(2u, size);
+  ScheduleTasks();
+
+  // This will time out if a resource that is larger than the throttle limit
+  // never gets scheduled.
+  RunMessageLoopUntilAllTasksHaveCompleted();
+}
+
 INSTANTIATE_TEST_CASE_P(RasterWorkerPoolTests,
                         RasterWorkerPoolTest,
                         ::testing::Values(RASTER_WORKER_POOL_TYPE_PIXEL_BUFFER,