cc: Don't infinitely throttle large raster tasks

cc/resources/raster_worker_pool_unittest.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/raster_worker_pool.h"
 
 #include <limits>
 #include <vector>
 
 #include "base/cancelable_callback.h"
@@ skipping 89 matching lines @@
   struct RasterTaskResult {
     unsigned id;
     bool canceled;
   };
 
   typedef std::vector<scoped_refptr<RasterTask> > RasterTaskVector;
 
   RasterWorkerPoolTest()
       : context_provider_(TestContextProvider::Create()),
         timeout_seconds_(5),
+        throttle_bytes_(10000),
         timed_out_(false) {
     output_surface_ = FakeOutputSurface::Create3d(context_provider_).Pass();
     CHECK(output_surface_->BindToClient(&output_surface_client_));
 
     shared_bitmap_manager_.reset(new TestSharedBitmapManager());
     resource_provider_ =
         ResourceProvider::Create(
             output_surface_.get(), shared_bitmap_manager_.get(), 0, false, 1,
             false).Pass();
     staging_resource_pool_ = ResourcePool::Create(
         resource_provider_.get(), GL_TEXTURE_2D, RGBA_8888);
 
     switch (GetParam()) {
       case RASTER_WORKER_POOL_TYPE_PIXEL_BUFFER:
         raster_worker_pool_ = PixelBufferRasterWorkerPool::Create(
             base::MessageLoopProxy::current().get(),
             RasterWorkerPool::GetTaskGraphRunner(),
             context_provider_.get(),
             resource_provider_.get(),
-            std::numeric_limits<size_t>::max());
+            throttle_bytes_);
         break;
       case RASTER_WORKER_POOL_TYPE_IMAGE:
         raster_worker_pool_ = ImageRasterWorkerPool::Create(
             base::MessageLoopProxy::current().get(),
             RasterWorkerPool::GetTaskGraphRunner(),
             resource_provider_.get());
         break;
       case RASTER_WORKER_POOL_TYPE_IMAGE_COPY:
         raster_worker_pool_ = ImageCopyRasterWorkerPool::Create(
             base::MessageLoopProxy::current().get(),
@@ skipping 56 matching lines @@
     RasterTaskQueue queue;
 
     for (RasterTaskVector::const_iterator it = tasks_.begin();
          it != tasks_.end();
          ++it)
       queue.items.push_back(RasterTaskQueue::Item(*it, false));
 
     raster_worker_pool_->AsRasterizer()->ScheduleTasks(&queue);
   }
 
-  void AppendTask(unsigned id) {
-    const gfx::Size size(1, 1);
-
+  void AppendTask(unsigned id, gfx::Size size) {

[reveman, 2014/08/21 01:15:47]

nit: const gfx::Size& size

[enne (OOO), 2014/08/21 01:34:15]

Done.

     scoped_ptr<ScopedResource> resource(
         ScopedResource::Create(resource_provider_.get()));
     resource->Allocate(size, ResourceProvider::TextureUsageAny, RGBA_8888);
     const Resource* const_resource = resource.get();
 
     ImageDecodeTask::Vector empty;
     tasks_.push_back(new TestRasterTaskImpl(
         const_resource,
         base::Bind(&RasterWorkerPoolTest::OnTaskCompleted,
                    base::Unretained(this),
                    base::Passed(&resource),
                    id),
         &empty));
   }
 
+  void AppendTask(unsigned id) { AppendTask(id, gfx::Size(1, 1)); }
+
   void AppendBlockingTask(unsigned id, base::Lock* lock) {
     const gfx::Size size(1, 1);
 
     scoped_ptr<ScopedResource> resource(
         ScopedResource::Create(resource_provider_.get()));
     resource->Allocate(size, ResourceProvider::TextureUsageAny, RGBA_8888);
     const Resource* const_resource = resource.get();
 
     ImageDecodeTask::Vector empty;
     tasks_.push_back(new BlockingTestRasterTaskImpl(
@@ skipping 29 matching lines @@
  protected:
   scoped_refptr<TestContextProvider> context_provider_;
   FakeOutputSurfaceClient output_surface_client_;
   scoped_ptr<FakeOutputSurface> output_surface_;
   scoped_ptr<SharedBitmapManager> shared_bitmap_manager_;
   scoped_ptr<ResourceProvider> resource_provider_;
   scoped_ptr<ResourcePool> staging_resource_pool_;
   scoped_ptr<RasterWorkerPool> raster_worker_pool_;
   base::CancelableClosure timeout_;
   int timeout_seconds_;
+  size_t throttle_bytes_;

[reveman, 2014/08/21 01:15:47]

nit: max_transfer_buffer_usage_bytes_ as from this pov it's not explicit that
PBRWP is throttling tasks based on this.

Also, does this need to be a member? can it just be a constant?
kMaxTransferBufferUsageBytes..

[enne (OOO), 2014/08/21 01:34:16]

Done.

   bool timed_out_;
   RasterTaskVector tasks_;
   std::vector<RasterTaskResult> completed_tasks_;
 };
 
 TEST_P(RasterWorkerPoolTest, Basic) {
   AppendTask(0u);
   AppendTask(1u);
   ScheduleTasks();
 
@@ skipping 41 matching lines @@
   RunMessageLoopUntilAllTasksHaveCompleted();
 }
 
 INSTANTIATE_TEST_CASE_P(RasterWorkerPoolTests,
                         RasterWorkerPoolTest,
                         ::testing::Values(RASTER_WORKER_POOL_TYPE_PIXEL_BUFFER,
                                           RASTER_WORKER_POOL_TYPE_IMAGE,
                                           RASTER_WORKER_POOL_TYPE_IMAGE_COPY,
                                           RASTER_WORKER_POOL_TYPE_GPU));
 
+class PixelBufferRasterWorkerPoolTest : public RasterWorkerPoolTest {};
+
+TEST_P(PixelBufferRasterWorkerPoolTest, BigTaskThrottling) {

[reveman, 2014/08/21 01:15:47]

How about we call this LargeResources and run it for all RWP implementations? I
don't think we have to leak the detail that some implementations do throttling
to this code. Just need to make sure resources of this size will run without
problem independent of the RWP implementation..

[enne (OOO), 2014/08/21 01:34:15]

Done.

+  gfx::Size size(1000, 1000);
+
+  {
+    // Verify a resource of this size will get throttled.

[reveman, 2014/08/21 01:15:47]

are we missing a "not" here? Maybe just "Verify a resource of this size is
handled properly"

[enne (OOO), 2014/08/21 01:34:16]

No, but maybe it should be "could get throttled".  It's trying to test the test.

Rewrote this to be more clear.

+    scoped_ptr<ScopedResource> resource(
+        ScopedResource::Create(resource_provider_.get()));
+    resource->Allocate(size, ResourceProvider::TextureUsageAny, RGBA_8888);
+    EXPECT_GE(resource->bytes(), throttle_bytes_);

[reveman, 2014/08/21 01:15:47]

Could you instead compute |size| based on kMaxTransferBufferUsageBytes? maybe
change kMaxTransferBufferUsageBytes to kMaxTransferBufferResourceDimension if
that's cleaner..

[enne (OOO), 2014/08/21 01:34:15]

That's why there's the block of code right here, to make sure that the size for
this test is larger than the throttle size.  I made kLargeResourceDimension a
constant along with 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(
+    PixelBufferRasterWorkerPoolTests,
+    PixelBufferRasterWorkerPoolTest,
+    ::testing::Values(RASTER_WORKER_POOL_TYPE_PIXEL_BUFFER));
+
 }  // namespace
 }  // namespace cc