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..f0c2d8443bf0eed150beb336bdb1a059c0210201 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,11 @@ void TileManager::ScheduleTasks(
     TRACE_EVENT_ASYNC_BEGIN0("cc", "ScheduledTasks", this);
   }
 
+  // Ensure that we don't schedule any decode work for checkered images until
+  // the raster work for visible tiles is done.
+  checker_image_tracker_.SetCanScheduleDecodeType(
+      CheckerImageTracker::DecodeType::kNone);

[vmpstr, 2017/06/20 18:41:01]

Maybe just renaming this to AllowedDecodeType or something? then kNone makes
more sense.

[Khushal, 2017/06/21 03:35:57]

This is also used when specifying the DecodeType in the ImageDecodeRequest.
AllowedDecodeType there looks awkward.

+
   // Cancel existing OnTaskSetFinished callbacks.
   task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
 
@@ -969,16 +979,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 +1121,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())

[vmpstr, 2017/06/20 18:41:01]

nit: braces

[Khushal, 2017/06/21 03:35:57]

Done.

-        checker_image_decode_queue->push_back(image);
+        checker_image_decode_queue->push_back(
+            CheckerImageTracker::ImageDecodeRequest(
+                image, CheckerImageTracker::DecodeType::kRaster));
     }
   }
 
@@ -1303,6 +1322,18 @@ void TileManager::CheckAndIssueSignals() {
       client_->NotifyAllTileTasksCompleted();
     }
   }
+
+  // Allow decodes for rasterized tiles if all required for draw/activate tiles
+  // are done. And pre-decode tiles once all tile tasks are done.
+  CheckerImageTracker::DecodeType decode_type =

[vmpstr, 2017/06/20 18:41:01]

Do we need this to be strictly in CheckAndIssueSignals? Can we do this from the
tile task completion callbacks instead? Like NotifyRasterFinished
NotifyReadyTo... functions? This feels like a common place, but I think it makes
the function do more things that it was designed to do (ie it's just just
checking and issuing signals anymore). 

[Khushal, 2017/06/21 03:35:57]

There can be a race between the DidFinish* task running for the previous graph
and the compositor thread scheduling another PrepareTiles. I think we try to
guard against this by actually iterating through the queue to check if the
required tiles are ready to draw. That's why I wanted to key off the notify_*
signals to be consistent with the logic that is used to issue these signals to
the rest of the system. And CheckAndIssueSignals is the place where that
happens.

+      CheckerImageTracker::DecodeType::kNone;
+  if (signals_.did_notify_all_tile_tasks_completed) {
+    decode_type = CheckerImageTracker::DecodeType::kPreDecode;
+  } else if (signals_.did_notify_ready_to_activate &&
+             signals_.did_notify_ready_to_draw) {
+    decode_type = CheckerImageTracker::DecodeType::kRaster;
+  }
+  checker_image_tracker_.SetCanScheduleDecodeType(decode_type);
 }
 
 void TileManager::CheckIfMoreTilesNeedToBePrepared() {
@@ -1322,7 +1353,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;
   }