cc: Add new RasterWorkerPool interface.

cc/resources/tile_manager.cc

Index: cc/resources/tile_manager.cc
diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
index acbe865deba9700d79509fa23639e82bc348ec19..a52706bf21ec7ce2a5f0789bb6009e9f88cf4ae0 100644
--- a/cc/resources/tile_manager.cc
+++ b/cc/resources/tile_manager.cc
@@ -13,8 +13,8 @@
 #include "base/metrics/histogram.h"
 #include "cc/debug/devtools_instrumentation.h"
 #include "cc/debug/traced_value.h"
-#include "cc/resources/raster_worker_pool.h"
-#include "cc/resources/resource_pool.h"
+#include "cc/resources/image_raster_worker_pool.h"
+#include "cc/resources/pixel_buffer_raster_worker_pool.h"
 #include "cc/resources/tile.h"
 #include "third_party/skia/include/core/SkDevice.h"
 #include "ui/gfx/rect_conversions.h"
@@ -23,28 +23,6 @@ namespace cc {
 
 namespace {
 
-// If we raster too fast we become upload bound, and pending
-// uploads consume memory. For maximum upload throughput, we would
-// want to allow for upload_throughput * pipeline_time of pending
-// uploads, after which we are just wasting memory. Since we don't
-// know our upload throughput yet, this just caps our memory usage.
-#if defined(OS_ANDROID)
-// For reference, the Nexus10 can upload 1MB in about 2.5ms.
-// Assuming a three frame deep pipeline this implies ~20MB.
-const size_t kMaxPendingUploadBytes = 20 * 1024 * 1024;
-// TODO(epenner): We should remove this upload limit (crbug.com/176197)
-const size_t kMaxPendingUploads = 72;
-#else
-const size_t kMaxPendingUploadBytes = 100 * 1024 * 1024;
-const size_t kMaxPendingUploads = 1000;
-#endif
-
-#if defined(OS_ANDROID)
-const int kMaxNumPendingTasksPerThread = 8;
-#else
-const int kMaxNumPendingTasksPerThread = 40;
-#endif
-
 // Determine bin based on three categories of tiles: things we need now,
 // things we need soon, and eventually.
 inline TileManagerBin BinFromTilePriority(const TilePriority& prio) {
@@ -112,15 +90,18 @@ scoped_ptr<TileManager> TileManager::Create(
     bool use_color_estimator,
     RenderingStatsInstrumentation* rendering_stats_instrumentation,
     bool use_map_image) {
-  scoped_ptr<RasterWorkerPool> raster_worker_pool =
-      RasterWorkerPool::Create(num_raster_threads);
-  return make_scoped_ptr(new TileManager(client,
-                                         resource_provider,
-                                         raster_worker_pool.Pass(),
-                                         num_raster_threads,
-                                         use_color_estimator,
-                                         rendering_stats_instrumentation,
-                                         use_map_image));
+  return make_scoped_ptr(
+      new TileManager(client,
+                      resource_provider,
+                      use_map_image ?
+                      ImageRasterWorkerPool::Create(
+                          resource_provider, num_raster_threads) :
+                      PixelBufferRasterWorkerPool::Create(
+                          resource_provider, num_raster_threads),
+                      num_raster_threads,
+                      use_color_estimator,
+                      rendering_stats_instrumentation,
+                      use_map_image));
 }
 
 TileManager::TileManager(
@@ -135,13 +116,10 @@ TileManager::TileManager(
       resource_pool_(ResourcePool::Create(resource_provider)),
       raster_worker_pool_(raster_worker_pool.Pass()),
       manage_tiles_pending_(false),
-      bytes_pending_upload_(0),
-      has_performed_uploads_since_last_flush_(false),
       ever_exceeded_memory_budget_(false),
       rendering_stats_instrumentation_(rendering_stats_instrumentation),
       use_color_estimator_(use_color_estimator),
-      did_initialize_visible_tile_(false),
-      max_pending_tasks_(kMaxNumPendingTasksPerThread * num_raster_threads) {
+      did_initialize_visible_tile_(false) {
   raster_worker_pool_->SetClient(this);
 }
 
@@ -153,8 +131,6 @@ TileManager::~TileManager() {
   // This should finish all pending tasks and release any uninitialized
   // resources.
   raster_worker_pool_->Shutdown();
-  AbortPendingTileUploads();
-  DCHECK_EQ(0u, tiles_with_pending_upload_.size());
   DCHECK_EQ(0u, tiles_.size());
 }
 
@@ -321,65 +297,19 @@ void TileManager::ManageTiles() {
 }
 
 void TileManager::CheckForCompletedTileUploads() {
-  while (!tiles_with_pending_upload_.empty()) {
-    Tile* tile = tiles_with_pending_upload_.front();
-    DCHECK(tile->tile_version().resource_);
-
-    // Set pixel tasks complete in the order they are posted.
-    if (!resource_pool_->resource_provider()->DidSetPixelsComplete(
-          tile->tile_version().resource_->id())) {
-      break;
-    }
-
-    // It's now safe to release the pixel buffer.
-    resource_pool_->resource_provider()->ReleasePixelBuffer(
-        tile->tile_version().resource_->id());
-
-    bytes_pending_upload_ -= tile->bytes_consumed_if_allocated();
-    bool was_forced = tile->tile_version().forced_upload_;
-    // Reset forced_upload_ since we now got the upload completed notification.
-    tile->tile_version().forced_upload_ = false;
-    tile->tile_version().memory_state_ = USING_RELEASABLE_MEMORY;
-    if (!was_forced)
-      DidFinishTileInitialization(tile);
-
-    tiles_with_pending_upload_.pop();
-  }
-
-  ScheduleTasks();
-}
-
-void TileManager::AbortPendingTileUploads() {
-  while (!tiles_with_pending_upload_.empty()) {
-    Tile* tile = tiles_with_pending_upload_.front();
-    DCHECK(tile->tile_version().resource_);
-
-    resource_pool_->resource_provider()->AbortSetPixels(
-        tile->tile_version().resource_->id());
-    resource_pool_->resource_provider()->ReleasePixelBuffer(
-        tile->tile_version().resource_->id());
-    tile->tile_version().memory_state_ = USING_RELEASABLE_MEMORY;
-
-    FreeResourcesForTile(tile);
-
-    bytes_pending_upload_ -= tile->bytes_consumed_if_allocated();
-    tiles_with_pending_upload_.pop();
-  }
+  raster_worker_pool_->CheckForCompletedTasks();
 }
 
 void TileManager::ForceTileUploadToComplete(Tile* tile) {
   DCHECK(tile);
-  if (tile->tile_version().resource_ &&
+  if (tile->tile_version().resource_id_ &&
       tile->tile_version().memory_state_ == USING_UNRELEASABLE_MEMORY &&
       !tile->tile_version().forced_upload_) {
-    resource_pool_->resource_provider()->
-        ForceSetPixelsToComplete(tile->tile_version().resource_->id());
-
-    // We have to set the memory state to be unreleasable, to ensure
-    // that the tile will not be freed until we get the upload finished
-    // notification. However, setting |forced_upload_| to true makes
-    // this tile ready to draw.
-    tile->tile_version().memory_state_ = USING_UNRELEASABLE_MEMORY;
+    if (!raster_worker_pool_->ForceUploadToComplete(
+            tile->tile_version().resource_id_))
+      return;
+
+    // Setting |forced_upload_| to true makes this tile ready to draw.
     tile->tile_version().forced_upload_ = true;
     DidFinishTileInitialization(tile);
     DCHECK(tile->tile_version().IsReadyToDraw());
@@ -459,14 +389,11 @@ void TileManager::AddRequiredTileForActivation(Tile* tile) {
   tiles_that_need_to_be_initialized_for_activation_.insert(tile);
 }
 
-void TileManager::DidFinishDispatchingWorkerPoolCompletionCallbacks() {
-  // If a flush is needed, do it now before starting to dispatch more tasks.
-  if (has_performed_uploads_since_last_flush_) {
-    resource_pool_->resource_provider()->ShallowFlushIfSupported();
-    has_performed_uploads_since_last_flush_ = false;
+void TileManager::DidFinishDispatchingRasterWorkerPoolCompletionCallbacks() {
+  if (did_initialize_visible_tile_) {

[vmpstr, 2013/05/29 16:55:03]

This block of code repeats in at least one other spot. Maybe it's worth making
something like "NotifyClientOfVisibleTileInitialization" and call that here and
wherever the code appears

[reveman, 2013/05/29 19:39:12]

I'm planning to remove this function and the RasterWorkerPoolClient interface
completely before this patch is ready to land. I think that will clean it up
nicely.

[vmpstr, 2013/05/29 20:45:06]

Sounds good.

[reveman, 2013/05/30 01:46:10]

Please take a look at latest patch.

+    did_initialize_visible_tile_ = false;
+    client_->DidInitializeVisibleTile();
   }
-
-  ScheduleTasks();
 }
 
 void TileManager::AssignGpuMemoryToTiles() {
@@ -638,10 +565,7 @@ void TileManager::FreeResourcesForTile(Tile* tile) {
 
 void TileManager::ScheduleTasks() {
   TRACE_EVENT0("cc", "TileManager::ScheduleTasks");
-  RasterWorkerPool::Task::Queue tasks;
-
-  size_t bytes_pending_upload = bytes_pending_upload_;
-  unsigned pending_tasks = 0;
+  RasterWorkerPool::RasterTask::Queue tasks;
 
   // Build a new task queue containing all task currently needed. Tasks
   // are added in order of priority, highest priority task first.
@@ -651,29 +575,8 @@ void TileManager::ScheduleTasks() {
     Tile* tile = *it;
     ManagedTileState& mts = tile->managed_state();
 
-    // Skip tile if determined to not require resource.
-    if (!tile->tile_version().requires_resource())
-      continue;
-
-    // Skip tile if already rasterized.
-    if (tile->tile_version().resource_)
-      continue;
-
-    // TODO(reveman): Remove throttling based on max pending tasks.
-    if (pending_tasks >= max_pending_tasks_)
-      break;
-
-    // TODO(reveman): Remove throttling based on max pending uploads.
-    if (tiles_with_pending_upload_.size() >= kMaxPendingUploads)
-      break;
-
-    // TODO(reveman): Throttle based on shared memory usage rather
-    // than bytes pending upload.
-    size_t new_bytes_pending = bytes_pending_upload;
-    new_bytes_pending += tile->bytes_consumed_if_allocated();
-    if (new_bytes_pending > kMaxPendingUploadBytes)
-      break;
-    bytes_pending_upload = new_bytes_pending;
+    DCHECK(tile->tile_version().requires_resource());
+    DCHECK(!tile->tile_version().resource_);
 
     // Create raster task for this tile if necessary.
     if (mts.raster_task.is_null())
@@ -681,24 +584,12 @@ void TileManager::ScheduleTasks() {
 
     // Finally append raster task.
     tasks.Append(mts.raster_task);
-    pending_tasks++;
-  }
-
-  if (!tasks.empty()) {
-    RasterWorkerPool::Task root(&tasks);
-
-    // Schedule running of |tasks|. This replaces any previously
-    // scheduled tasks and effectively cancels all tasks not present
-    // in |tasks|.
-    raster_worker_pool_->ScheduleTasks(&root);
-  } else {
-    raster_worker_pool_->ScheduleTasks(NULL);
   }
 
-  if (did_initialize_visible_tile_) {
-    did_initialize_visible_tile_ = false;
-    client_->DidInitializeVisibleTile();
-  }
+  // Schedule running of |tasks|. This replaces any previously
+  // scheduled tasks and effectively cancels all tasks not present
+  // in |tasks|.
+  raster_worker_pool_->ScheduleTasks(&tasks);
 }
 
 RasterWorkerPool::Task TileManager::CreateImageDecodeTask(
@@ -717,8 +608,7 @@ RasterWorkerPool::Task TileManager::CreateImageDecodeTask(
 }
 
 void TileManager::OnImageDecodeTaskCompleted(scoped_refptr<Tile> tile,
-                                             uint32_t pixel_ref_id,
-                                             bool was_canceled) {
+                                             uint32_t pixel_ref_id) {
   TRACE_EVENT0("cc", "TileManager::OnImageDecodeTaskCompleted");
   DCHECK(pending_decode_tasks_.find(pixel_ref_id) !=
          pending_decode_tasks_.end());
@@ -738,28 +628,22 @@ TileManager::RasterTaskMetadata TileManager::GetRasterTaskMetadata(
   return metadata;
 }
 
-RasterWorkerPool::Task TileManager::CreateRasterTask(Tile* tile) {
+RasterWorkerPool::RasterTask TileManager::CreateRasterTask(Tile* tile) {
   TRACE_EVENT0("cc", "TileManager::CreateRasterTask");
 
   scoped_ptr<ResourcePool::Resource> resource =
       resource_pool_->AcquireResource(
           tile->tile_size_.size(),
           tile->tile_version().resource_format_);
-  resource_pool_->resource_provider()->AcquirePixelBuffer(resource->id());
+  const Resource* const_resource = resource.get();
 
   DCHECK_EQ(CAN_USE_MEMORY, tile->tile_version().memory_state_);
   tile->tile_version().memory_state_ = USING_UNRELEASABLE_MEMORY;
 
   PicturePileImpl::Analysis* analysis = new PicturePileImpl::Analysis;
 
-  // MapPixelBuffer() returns NULL if context was lost at the time
-  // AcquirePixelBuffer() was called. For simplicity we still create
-  // a raster task that is essentially a noop in these situations.
-  uint8* buffer = resource_pool_->resource_provider()->MapPixelBuffer(
-      resource->id());
-
   // Create and queue all image decode tasks that this tile depends on.
-  RasterWorkerPool::Task::Queue decode_tasks;
+  RasterWorkerPool::Task::Set decode_tasks;
   for (PicturePileImpl::PixelRefIterator iter(tile->content_rect(),
                                               tile->contents_scale(),
                                               tile->picture_pile());
@@ -770,7 +654,7 @@ RasterWorkerPool::Task TileManager::CreateRasterTask(Tile* tile) {
     // Append existing image decode task if available.
     PixelRefMap::iterator decode_task_it = pending_decode_tasks_.find(id);
     if (decode_task_it != pending_decode_tasks_.end()) {
-      decode_tasks.Append(decode_task_it->second);
+      decode_tasks.Insert(decode_task_it->second);
       continue;
     }
 
@@ -783,12 +667,13 @@ RasterWorkerPool::Task TileManager::CreateRasterTask(Tile* tile) {
     // Create and append new image decode task for this pixel ref.
     RasterWorkerPool::Task decode_task = CreateImageDecodeTask(
         tile, pixel_ref);
-    decode_tasks.Append(decode_task);
+    decode_tasks.Insert(decode_task);
     pending_decode_tasks_[id] = decode_task;
   }
 
-  return RasterWorkerPool::PictureTask(
+  return RasterWorkerPool::RasterTask(
       tile->picture_pile(),
+      const_resource,
       base::Bind(&TileManager::RunAnalyzeAndRasterTask,
                  base::Bind(&TileManager::RunAnalyzeTask,
                             analysis,
@@ -798,7 +683,6 @@ RasterWorkerPool::Task TileManager::CreateRasterTask(Tile* tile) {
                             GetRasterTaskMetadata(*tile),
                             rendering_stats_instrumentation_),
                  base::Bind(&TileManager::RunRasterTask,
-                            buffer,
                             analysis,
                             tile->content_rect(),
                             tile->contents_scale(),
@@ -826,12 +710,8 @@ void TileManager::OnRasterTaskCompleted(
   // Tile resources can't be freed until upload has completed.
   DCHECK_EQ(USING_UNRELEASABLE_MEMORY, tile->tile_version().memory_state_);
 
-  // Release raster resources.
-  resource_pool_->resource_provider()->UnmapPixelBuffer(resource->id());
-
   if (was_canceled) {
     tile->tile_version().memory_state_ = CAN_USE_MEMORY;
-    resource_pool_->resource_provider()->ReleasePixelBuffer(resource->id());
     resource_pool_->ReleaseResource(resource.Pass());
     return;
   }
@@ -841,23 +721,14 @@ void TileManager::OnRasterTaskCompleted(
 
   if (analysis->is_solid_color) {
     tile->tile_version().set_solid_color(analysis->solid_color);
-    resource_pool_->resource_provider()->ReleasePixelBuffer(resource->id());
+    tile->tile_version().memory_state_ = CAN_USE_MEMORY;

[vmpstr, 2013/05/29 16:55:03]

Why CAN_USE_MEMORY? set_solid_color sets it to NOT_ALLOWED_TO_USE_MEMORY. 

[reveman, 2013/05/29 19:39:12]

Oh, this is not needed in that case. I still have a hard time reasoning with
this memory state thing. We should be able to remove it completely once this
patch lands as there's no longer such a thing as a TileVersion with unreleasable
memory. It either owns a resource that can be released or it doesn't.

[vmpstr, 2013/05/29 20:45:06]

Agreed. CAN_USE_MEMORY right now means "this version has memory allocated to
it", which means that solid color tiles shouldn't be in that state

     resource_pool_->ReleaseResource(resource.Pass());
-    DidFinishTileInitialization(tile);
-    return;
+  } else {
+    tile->tile_version().memory_state_ = USING_RELEASABLE_MEMORY;
+    tile->tile_version().resource_ = resource.Pass();

[vmpstr, 2013/05/29 16:55:03]

Do you have to set the resource_id_ as well? I can't really find where that's
set. Also, what's the reasoning for separating the two? It seems that you now
have to maintain two items that should(?) be related

[reveman, 2013/05/29 19:39:12]

resource_id_ is needed for ForceUpload. Maybe we should change that API a bit.
How about RasterWorkerPool::ForceUploadToComplete take a raster task as
identifier instead and return a temporary ResourceId that can be used for
drawing until we have the real resource. And we store that temporary id in
something like TileVersion::forced_upload_resource_id_. Wdyt?

[vmpstr, 2013/05/29 20:45:06]

I like ForceUpload taking a task, since it makes it a bit more clear (to me)
what it's operating on. I don't know about forced_upload_resource_id_. It kinda
makes managed tile state cluttered with different resources and/or resource ids.
This temporary id would be the same id as the final one, right? Except the
ownership of the resource is still in the upload queue. 

Ideally we can hand off the ownership of the resource (ie eliminate the concept
of forced or not forced from the managed tile state's perspective), and just
deal with the fact that we might get an upload completed notifications for
resources that are no longer owned by the upload queue. However, we have to be
careful not to free the resource until we get that notification.

[reveman, 2013/05/30 01:46:10]

ForceUpload takes a task in latest patch. It's out of the scope for this patch
but we should move towards being able to always pass the resource ownership back
at completion. This requires detaching the pixel buffer and the shared memory
from the resource somehow and I'm not sure yet how to best do that.

I'd like to eliminate the idea of a forced upload completely. Doing this
requires adding information about what tiles are required for activation to the
RasterWorkerPool::ScheduleTasks API. ie. ScheduleTasks(const RasterTaskQueue&
tasks_required_for_activation, const RasterTaskQueue& other_tasks).

   }
 
-  resource_pool_->resource_provider()->BeginSetPixels(resource->id());
-  has_performed_uploads_since_last_flush_ = true;
-
-  tile->tile_version().resource_ = resource.Pass();
-
-  bytes_pending_upload_ += tile->bytes_consumed_if_allocated();
-  tiles_with_pending_upload_.push(tile);
-
-  if (tile->required_for_activation() &&
-      client_->ShouldForceTileUploadsRequiredForActivationToComplete())
-    ForceTileUploadToComplete(tile);
+  DidFinishTileInitialization(tile);
 }
 
 void TileManager::DidFinishTileInitialization(Tile* tile) {
@@ -893,12 +764,13 @@ void TileManager::RunImageDecodeTask(
 }
 
 // static
-void TileManager::RunAnalyzeAndRasterTask(
-    const RasterWorkerPool::PictureTask::Callback& analyze_task,
-    const RasterWorkerPool::PictureTask::Callback& raster_task,
+bool TileManager::RunAnalyzeAndRasterTask(
+    const base::Callback<void(PicturePileImpl*)>& analyze_task,
+    const RasterWorkerPool::RasterTask::Callback& raster_task,
+    uint8* buffer,
     PicturePileImpl* picture_pile) {
   analyze_task.Run(picture_pile);
-  raster_task.Run(picture_pile);
+  return raster_task.Run(buffer, picture_pile);
 }
 
 // static
@@ -941,13 +813,13 @@ scoped_ptr<base::Value> TileManager::RasterTaskMetadata::AsValue() const {
 }
 
 // static
-void TileManager::RunRasterTask(
-    uint8* buffer,
+bool TileManager::RunRasterTask(
     PicturePileImpl::Analysis* analysis,
     gfx::Rect rect,
     float contents_scale,
     const RasterTaskMetadata& metadata,
     RenderingStatsInstrumentation* stats_instrumentation,
+    uint8* buffer,
     PicturePileImpl* picture_pile) {
   TRACE_EVENT1(
       "cc", "TileManager::RunRasterTask",
@@ -960,10 +832,10 @@ void TileManager::RunRasterTask(
 
   // |buffer| can be NULL in lost context situations.
   if (!buffer)
-    return;
+    return false;
 
   if (analysis->is_solid_color)
-    return;
+    return false;
 
   SkBitmap bitmap;
   bitmap.setConfig(SkBitmap::kARGB_8888_Config, rect.width(), rect.height());
@@ -989,6 +861,8 @@ void TileManager::RunRasterTask(
   } else {
     picture_pile->RasterToBitmap(&canvas, rect, contents_scale, NULL);
   }
+
+  return true;
 }
 
 }  // namespace cc