cc: Move pre-decodes to background worker.

cc/tiles/tile_manager.cc

Index: cc/tiles/tile_manager.cc
diff --git a/cc/tiles/tile_manager.cc b/cc/tiles/tile_manager.cc
index e2d11552268c562607b140c1b0349d73670a49a1..971197285a90a41092d1ce6e8bab291386747341 100644
--- a/cc/tiles/tile_manager.cc
+++ b/cc/tiles/tile_manager.cc
@@ -509,7 +509,7 @@ bool TileManager::PrepareTiles(
       prioritized_work.tiles_to_raster.front().tile()->required_for_draw());
 
   // Schedule tile tasks.
-  ScheduleTasks(prioritized_work);
+  ScheduleTasks(std::move(prioritized_work));
 
   TRACE_EVENT_INSTANT1("cc", "DidPrepareTiles", TRACE_EVENT_SCOPE_THREAD,
                        "state", BasicStateAsValue());
@@ -705,6 +705,7 @@ TileManager::PrioritizedWorkToSchedule TileManager::AssignGpuMemoryToTiles() {
 
       AddCheckeredImagesToDecodeQueue(
           prioritized_tile, raster_color_space,
+          CheckerImageTracker::DecodeType::kRaster,
           &work_to_schedule.checker_image_decode_queue);
       continue;
     }
@@ -763,6 +764,7 @@ TileManager::PrioritizedWorkToSchedule TileManager::AssignGpuMemoryToTiles() {
           prioritized_tile.should_decode_checkered_images_for_tile()) {
         AddCheckeredImagesToDecodeQueue(
             prioritized_tile, raster_color_space,
+            CheckerImageTracker::DecodeType::kRaster,
             &work_to_schedule.checker_image_decode_queue);
       }
     } else {
@@ -805,6 +807,7 @@ TileManager::PrioritizedWorkToSchedule TileManager::AssignGpuMemoryToTiles() {
           tile->raster_task_scheduled_with_checker_images()) {
         AddCheckeredImagesToDecodeQueue(
             prioritized_tile, raster_color_space,
+            CheckerImageTracker::DecodeType::kRaster,
             &work_to_schedule.checker_image_decode_queue);
       }
     }
@@ -874,6 +877,7 @@ void TileManager::PartitionImagesForCheckering(
 void TileManager::AddCheckeredImagesToDecodeQueue(
     const PrioritizedTile& prioritized_tile,
     const gfx::ColorSpace& raster_color_space,
+    CheckerImageTracker::DecodeType decode_type,
     CheckerImageTracker::ImageDecodeQueue* image_decode_queue) {
   Tile* tile = prioritized_tile.tile();
   std::vector<DrawImage> images_in_tile;
@@ -883,13 +887,14 @@ void TileManager::AddCheckeredImagesToDecodeQueue(
   WhichTree tree = tile->tiling()->tree();
 
   for (auto& draw_image : images_in_tile) {
-    if (checker_image_tracker_.ShouldCheckerImage(draw_image, tree))
-      image_decode_queue->push_back(draw_image.paint_image());
+    if (checker_image_tracker_.ShouldCheckerImage(draw_image, tree)) {
+      image_decode_queue->push_back(CheckerImageTracker::ImageDecodeRequest(
+          draw_image.paint_image(), decode_type));
+    }
   }
 }
 
-void TileManager::ScheduleTasks(
-    const PrioritizedWorkToSchedule& work_to_schedule) {
+void TileManager::ScheduleTasks(PrioritizedWorkToSchedule work_to_schedule) {
   const std::vector<PrioritizedTile>& tiles_that_need_to_be_rasterized =
       work_to_schedule.tiles_to_raster;
   TRACE_EVENT1("cc", "TileManager::ScheduleTasks", "count",
@@ -901,6 +906,10 @@ void TileManager::ScheduleTasks(
     TRACE_EVENT_ASYNC_BEGIN0("cc", "ScheduledTasks", this);
   }
 
+  // Ensure that we don't schedule any pre-decode work till all tile tasks have

[enne (OOO), 2017/06/08 20:08:10]

I'm mostly going to defer to vmpstr on this review, but I just had some higher
level questions:

(1) Is this the right priority for checker image decodes (i.e. last)? Sure,
visible tiles probably should have highest priority, but should we fix checker
images before we do pre-raster work of offscreen tiles?  Maybe this is correct,
but I just wanted clarification that this is intended.

(2) Similarly, checker image scheduling seems kind of bolted onto the side and
not prioritized against other work.  I suspect maybe it's not desirable to mix
tile prioritization with decode prioritization, but mixing the two systems seems
a little awkward from an outside perspective.

[Khushal, 2017/06/08 20:34:15]

This is only going to defer the decoding for pre-decode tiles. The decodes for
checkered images from any tiles that are being rasterized will still continue in
parallel with the raster work.
The fact that checker image scheduling is completely independent of work done by
the raster worker pool creates interesting scenarios on devices that have a
limited number of cores. Currently the raster worker pool first runs visible
tile tasks on the foreground workers, followed by pre-paint tiles on the
background worker, and lastly decodes for pre-decoded tiles. With
checker-imaging on the side, we could have cases where the visible tile work is
scheduled in parallel with the decode for an offscreen tile on the image worker
and it starts pre-empting the raster work. Should we try to avoid that? Listen
to when all NOW bin tiles are done before starting checkered decodes for
pre-paint tiles for instance? At the least I wanted to keep decodes for
pre-decode tiles from interrupting raster work we know is higher priority.

+  // finished.
+  checker_image_tracker_.SetCanSchedulePredecodeImages(false);
+
   // Cancel existing OnTaskSetFinished callbacks.
   task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
 
@@ -969,16 +978,23 @@ void TileManager::ScheduleTasks(
       work_to_schedule.tiles_to_process_for_images;
   std::vector<DrawImage> new_locked_images;
   for (const PrioritizedTile& prioritized_tile : tiles_to_process_for_images) {
-    Tile* tile = prioritized_tile.tile();
+    std::vector<DrawImage> sync_decoded_images;
+    std::vector<PaintImage> checkered_images;
+    PartitionImagesForCheckering(prioritized_tile, raster_color_space,
+                                 &sync_decoded_images, &checkered_images);
 
-    // TODO(khushalsagar): Send these images to the ImageDecodeService, through
-    // the CheckerImageTracker as well. See crbug.com/691087.
-    std::vector<DrawImage> images;
-    prioritized_tile.raster_source()->GetDiscardableImagesInRect(
-        tile->enclosing_layer_rect(), tile->raster_transform().scale(),
-        raster_color_space, &images);
-    new_locked_images.insert(new_locked_images.end(), images.begin(),
-                             images.end());
+    // Add the sync decoded images to |new_locked_images| so they can be added
+    // to the task graph.
+    new_locked_images.insert(new_locked_images.end(),
+                             sync_decoded_images.begin(),
+                             sync_decoded_images.end());
+
+    // For checkered-images, send them to the decode service.
+    for (auto& image : checkered_images) {
+      work_to_schedule.checker_image_decode_queue.push_back(
+          CheckerImageTracker::ImageDecodeRequest(
+              std::move(image), CheckerImageTracker::DecodeType::kPreDecode));
+    }
   }
 
   // TODO(vmpstr): SOON is misleading here, but these images can come from
@@ -1104,7 +1120,9 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
       // skipping images for these tiles, we don't need to decode them since
       // they will not be required on the next active tree.
       if (prioritized_tile.should_decode_checkered_images_for_tile())
-        checker_image_decode_queue->push_back(image);
+        checker_image_decode_queue->push_back(
+            CheckerImageTracker::ImageDecodeRequest(
+                image, CheckerImageTracker::DecodeType::kRaster));
     }
   }
 
@@ -1300,6 +1318,7 @@ void TileManager::CheckAndIssueSignals() {
           TRACE_DISABLED_BY_DEFAULT("cc.debug"),
           "TileManager::CheckAndIssueSignals - all tile tasks completed");
       signals_.did_notify_all_tile_tasks_completed = true;
+      checker_image_tracker_.SetCanSchedulePredecodeImages(true);

[vmpstr, 2017/06/12 21:45:30]

Can you document the order of execution somewhere? Maybe on TileManager class to
explain the total order of things

[Khushal, 2017/06/14 00:29:20]

I added a comment on TileManager. Does that look sufficient?

       client_->NotifyAllTileTasksCompleted();
     }
   }
@@ -1322,7 +1341,7 @@ void TileManager::CheckIfMoreTilesNeedToBePrepared() {
   // |tiles_that_need_to_be_rasterized| will be empty when we reach a
   // steady memory state. Keep scheduling tasks until we reach this state.
   if (!work_to_schedule.tiles_to_raster.empty()) {
-    ScheduleTasks(work_to_schedule);
+    ScheduleTasks(std::move(work_to_schedule));
     return;
   }