[PE] Don't allow raster tasks for pre-paint tiles with at-raster images.

In GPU raster mode, only one raster task can be active at a time. This means
that these rasters have to happen quickly, or they will block other work.
For this reason, we should not allow pre-paint GPU raster tasks which might
have very slow performance to execute, because otherwise they will block
future high-priority on-screen raster tasks.

Raster tasks can have at-raster image decoding if the memory for storing
pre-decoded images has been exhausted. This is the case in the website
referenced in the bug. In these cases, rather than allowing the raster
to proceed and potentially jank display due to long image decodes, we
abort the raster, but still proceed with the image decodes (which don't
block other raster, and may be useful to decode in any case).

Bug: 768799
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I2ec78361739b7b8f455f26211f21a5f8e995a8b6
Reviewed-on: https://chromium-review.googlesource.com/806855
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521975}

cc/tiles/tile.h

@@ -67,6 +67,10 @@ class CC_EXPORT Tile {
     required_for_draw_ = is_required;
   }
 
+  bool is_prepaint() const {
+    return !required_for_activation() && !required_for_draw();
+  }
+
   bool use_picture_analysis() const {
     return !!(flags_ & USE_PICTURE_ANALYSIS);
   }

cc/tiles/tile_manager.cc

@@ -705,8 +705,7 @@ TileManager::PrioritizedWorkToSchedule TileManager::AssignGpuMemoryToTiles() {
     }
 
     // Prepaint tiles that are far away are only processed for images.
-    if (!tile->required_for_activation() && !tile->required_for_draw() &&
-        prioritized_tile.is_process_for_images_only()) {
+    if (tile->is_prepaint() && prioritized_tile.is_process_for_images_only()) {
       work_to_schedule.tiles_to_process_for_images.push_back(prioritized_tile);
       continue;
     }
@@ -734,8 +733,6 @@ TileManager::PrioritizedWorkToSchedule TileManager::AssignGpuMemoryToTiles() {
       break;
     }
 
-    tile->scheduled_priority_ = schedule_priority++;
-
     DCHECK(tile->draw_info().mode() == TileDrawInfo::OOM_MODE ||
            !tile->draw_info().IsReadyToDraw());
 
@@ -787,11 +784,16 @@ TileManager::PrioritizedWorkToSchedule TileManager::AssignGpuMemoryToTiles() {
     } else {
       // Creating the raster task here will acquire resources, but
       // this resource usage has already been accounted for above.
-      tile->raster_task_ =
-          CreateRasterTask(prioritized_tile, client_->GetRasterColorSpace(),
-                           &work_to_schedule.checker_image_decode_queue);
+      auto raster_task = CreateRasterTask(
+          prioritized_tile, client_->GetRasterColorSpace(), &work_to_schedule);
+      if (!raster_task) {
+        continue;
+      }
+
+      tile->raster_task_ = std::move(raster_task);
     }
 
+    tile->scheduled_priority_ = schedule_priority++;
     memory_usage += memory_required_by_tile_to_be_scheduled;
     work_to_schedule.tiles_to_raster.push_back(prioritized_tile);
   }
@@ -1040,6 +1042,10 @@ void TileManager::ScheduleTasks(PrioritizedWorkToSchedule work_to_schedule) {
     }
   }
 
+  new_locked_images.insert(new_locked_images.end(),
+                           work_to_schedule.extra_prepaint_images.begin(),
+                           work_to_schedule.extra_prepaint_images.end());
+
   // TODO(vmpstr): SOON is misleading here, but these images can come from
   // several diffent tiles. Rethink what we actually want to trace here. Note
   // that I'm using SOON, since it can't be NOW (these are prepaint).
@@ -1049,6 +1055,8 @@ void TileManager::ScheduleTasks(PrioritizedWorkToSchedule work_to_schedule) {
   std::vector<scoped_refptr<TileTask>> new_locked_image_tasks =
       image_controller_.SetPredecodeImages(std::move(new_locked_images),
                                            tracing_info);
+  work_to_schedule.extra_prepaint_images.clear();
+
   for (auto& task : new_locked_image_tasks) {
     auto decode_it = std::find_if(graph_.nodes.begin(), graph_.nodes.end(),
                                   [&task](const TaskGraph::Node& node) {
@@ -1112,7 +1120,7 @@ void TileManager::ScheduleTasks(PrioritizedWorkToSchedule work_to_schedule) {
 scoped_refptr<TileTask> TileManager::CreateRasterTask(
     const PrioritizedTile& prioritized_tile,
     const gfx::ColorSpace& color_space,
-    CheckerImageTracker::ImageDecodeQueue* checker_image_decode_queue) {
+    PrioritizedWorkToSchedule* work_to_schedule) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "TileManager::CreateRasterTask");
   Tile* tile = prioritized_tile.tile();
@@ -1154,35 +1162,15 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
   std::vector<DrawImage>& sync_decoded_images =
       scheduled_draw_images_[tile->id()];
   sync_decoded_images.clear();
-  PaintImageIdFlatSet images_to_skip;
+  std::vector<PaintImage> checkered_images;
   base::flat_map<PaintImage::Id, size_t> image_id_to_current_frame_index;
   if (!skip_images) {
-    std::vector<PaintImage> checkered_images;
     PartitionImagesForCheckering(
         prioritized_tile, color_space, &sync_decoded_images, &checkered_images,
         partial_tile_decode ? &invalidated_rect : nullptr,
         &image_id_to_current_frame_index);
-    for (const auto& image : checkered_images) {
-      DCHECK(!image.ShouldAnimate());
-
-      images_to_skip.insert(image.stable_id());
-
-      // This can be the case for tiles on the active tree that will be replaced
-      // or are occluded on the pending tree. While we still need to continue
-      // 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->emplace_back(
-            image, CheckerImageTracker::DecodeType::kRaster);
-      }
-    }
   }
 
-  const bool has_checker_images = !images_to_skip.empty();
-  tile->set_raster_task_scheduled_with_checker_images(has_checker_images);
-  if (has_checker_images)
-    num_of_tiles_with_checker_images_++;
-
   // Get the tasks for the required images.
   ImageDecodeCache::TracingInfo tracing_info(
       prepare_tiles_count_, prioritized_tile.priority().priority_bin,
@@ -1191,6 +1179,39 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
   image_controller_.GetTasksForImagesAndRef(
       &sync_decoded_images, &at_raster_images, &decode_tasks, tracing_info);
 
+  const bool has_checker_images = !checkered_images.empty();
+  tile->set_raster_task_scheduled_with_checker_images(has_checker_images);
+  if (has_checker_images)
+    num_of_tiles_with_checker_images_++;
+
+  // Don't allow at-raster prepaint tiles, because they could be very slow
+  // and block high-priority tasks.
+  if (!at_raster_images.empty() && tile->is_prepaint()) {
+    work_to_schedule->extra_prepaint_images.insert(
+        work_to_schedule->extra_prepaint_images.end(),
+        sync_decoded_images.begin(), sync_decoded_images.end());
+    // This will unref the images, but ScheduleTasks will schedule them
+    // right away anyway.
+    OnRasterTaskCompleted(tile->id(), resource, true /* was_canceled */);
+    return nullptr;
+  }
+
+  PaintImageIdFlatSet images_to_skip;
+  for (const auto& image : checkered_images) {
+    DCHECK(!image.ShouldAnimate());
+
+    images_to_skip.insert(image.stable_id());
+
+    // This can be the case for tiles on the active tree that will be replaced
+    // or are occluded on the pending tree. While we still need to continue
+    // 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()) {
+      work_to_schedule->checker_image_decode_queue.emplace_back(
+          image, CheckerImageTracker::DecodeType::kRaster);
+    }
+  }
+
   std::unique_ptr<RasterBuffer> raster_buffer =
       raster_buffer_provider_->AcquireBufferForRaster(
           resource, resource_content_id, tile->invalidated_id());
@@ -1224,16 +1245,14 @@ void TileManager::ResetSignalsForTesting() {
   signals_.reset();
 }
 
-void TileManager::OnRasterTaskCompleted(
-    Tile::Id tile_id,
-    Resource* resource,
-    bool was_canceled) {
+void TileManager::OnRasterTaskCompleted(Tile::Id tile_id,
+                                        Resource* resource,
+                                        bool was_canceled) {
   auto found = tiles_.find(tile_id);
   Tile* tile = nullptr;
   bool raster_task_was_scheduled_with_checker_images = false;
   if (found != tiles_.end()) {
     tile = found->second;
-    DCHECK(tile->raster_task_.get());
     tile->raster_task_ = nullptr;
     raster_task_was_scheduled_with_checker_images =
         tile->set_raster_task_scheduled_with_checker_images(false);

cc/tiles/tile_manager.h

@@ -323,6 +323,9 @@ class CC_EXPORT TileManager : CheckerImageTrackerClient {
 
     std::vector<PrioritizedTile> tiles_to_raster;
     std::vector<PrioritizedTile> tiles_to_process_for_images;
+    // A vector of additional images to be decoded for prepaint, but that
+    // are not necessarily associated with any tile.
+    std::vector<DrawImage> extra_prepaint_images;
     CheckerImageTracker::ImageDecodeQueue checker_image_decode_queue;
   };
 
@@ -331,7 +334,7 @@ class CC_EXPORT TileManager : CheckerImageTrackerClient {
   scoped_refptr<TileTask> CreateRasterTask(
       const PrioritizedTile& prioritized_tile,
       const gfx::ColorSpace& color_space,
-      CheckerImageTracker::ImageDecodeQueue* checker_image_decode_queue);
+      PrioritizedWorkToSchedule* work_to_schedule);
 
   std::unique_ptr<EvictionTilePriorityQueue>
   FreeTileResourcesUntilUsageIsWithinLimit(

cc/tiles/tile_manager_unittest.cc

@@ -2510,6 +2510,89 @@ TEST_F(CheckerImagingTileManagerTest,
   EXPECT_FALSE(host_impl()->tile_manager()->HasScheduledTileTasksForTesting());
 }
 
+class EmptyCacheTileManagerTest : public TileManagerTest {
+ public:
+  LayerTreeSettings CreateSettings() override {
+    LayerTreeSettings settings;
+    settings.decoded_image_working_set_budget_bytes = 0;
+    return settings;
+  }
+};
+
+TEST_F(EmptyCacheTileManagerTest, AtRasterOnScreenTileRasterTasks) {
+  const gfx::Size layer_bounds(500, 500);
+
+  std::unique_ptr<FakeRecordingSource> recording_source =
+      FakeRecordingSource::CreateFilledRecordingSource(layer_bounds);
+  recording_source->set_fill_with_nonsolid_color(true);
+
+  int dimension = 500;
+  PaintImage image =
+      CreateDiscardablePaintImage(gfx::Size(dimension, dimension));
+  recording_source->add_draw_image(image, gfx::Point(0, 0));
+
+  recording_source->Rerecord();
+  scoped_refptr<RasterSource> raster_source =
+      recording_source->CreateRasterSource();
+
+  gfx::Size tile_size(500, 500);
+  Region invalidation((gfx::Rect(layer_bounds)));
+  SetupPendingTree(raster_source, tile_size, invalidation);
+
+  PictureLayerTilingSet* tiling_set =
+      pending_layer()->picture_layer_tiling_set();
+  PictureLayerTiling* pending_tiling = tiling_set->tiling_at(0);
+  pending_tiling->set_resolution(HIGH_RESOLUTION);
+  pending_tiling->CreateAllTilesForTesting();
+  pending_tiling->SetTilePriorityRectsForTesting(
+      gfx::Rect(layer_bounds),   // Visible rect.
+      gfx::Rect(layer_bounds),   // Skewport rect.
+      gfx::Rect(layer_bounds),   // Soon rect.
+      gfx::Rect(layer_bounds));  // Eventually rect.
+
+  host_impl()->tile_manager()->PrepareTiles(host_impl()->global_tile_state());
+  // There will be a tile raster task and an image decode task.
+  EXPECT_TRUE(pending_tiling->TileAt(0, 0)->HasRasterTask());
+  EXPECT_TRUE(host_impl()->tile_manager()->HasScheduledTileTasksForTesting());
+}
+
+TEST_F(EmptyCacheTileManagerTest, AtRasterPrepaintTileRasterTasksSkipped) {
+  const gfx::Size layer_bounds(500, 500);
+
+  std::unique_ptr<FakeRecordingSource> recording_source =
+      FakeRecordingSource::CreateFilledRecordingSource(layer_bounds);
+  recording_source->set_fill_with_nonsolid_color(true);
+
+  int dimension = 500;
+  PaintImage image =
+      CreateDiscardablePaintImage(gfx::Size(dimension, dimension));
+  recording_source->add_draw_image(image, gfx::Point(0, 0));
+
+  recording_source->Rerecord();
+  scoped_refptr<RasterSource> raster_source =
+      recording_source->CreateRasterSource();
+
+  gfx::Size tile_size(500, 500);
+  Region invalidation((gfx::Rect(layer_bounds)));
+  SetupPendingTree(raster_source, tile_size, invalidation);
+
+  PictureLayerTilingSet* tiling_set =
+      pending_layer()->picture_layer_tiling_set();
+  PictureLayerTiling* pending_tiling = tiling_set->tiling_at(0);
+  pending_tiling->set_resolution(HIGH_RESOLUTION);
+  pending_tiling->CreateAllTilesForTesting();
+  pending_tiling->SetTilePriorityRectsForTesting(
+      gfx::Rect(),  // An empty visual rect leads to the tile being pre-paint.
+      gfx::Rect(layer_bounds),   // Skewport rect.
+      gfx::Rect(layer_bounds),   // Soon rect.
+      gfx::Rect(layer_bounds));  // Eventually rect.
+
+  host_impl()->tile_manager()->PrepareTiles(host_impl()->global_tile_state());
+  // There will be no tile raster task, but there will be an image decode task.
+  EXPECT_FALSE(pending_tiling->TileAt(0, 0)->HasRasterTask());
+  EXPECT_TRUE(host_impl()->tile_manager()->HasScheduledTileTasksForTesting());
+}
+
 TEST_F(CheckerImagingTileManagerTest, BuildsImageDecodeQueueAsExpected) {
   const gfx::Size layer_bounds(900, 900);