cc: Eliminate required for activation veto.

This change reverts 7875d555.

Currently we only defer images for async decoding if they are on a tile
that is required for activation, which effectively means images coming
into the viewport from a compositor scroll would always be sync decoded.
Since this can lead to excessive checkerboarding in some cases,
eliminate this veto.

This veto condition was initially added to avoid overhead of creating
pending trees for pre-paint/pre-decode checker-imaged tiles. Follow up
changes should still attempt to minimize this overhead.

Also note that comparing the UMA data for
CheckerboardedNeedRasterContentArea for the CheckerImaging experiment on
canary, async decoding on the active tree shows an overall decrease in
checkerboarding. For a 28-day aggregation:

1) Async decoding without activation veto
0-1 pixels/frame : 99.608% for checkerimaging group compared with
99.567% for control.

2) Async decoding with activation veto
0-1 pixels/frame : 99.19% for checkerimaging group compared with
99.176% for control.

R=vmpstr@chromium.org

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I79aed0a7b3c4b14b50ff54c6237839c23d1ff5c1
Reviewed-on: https://chromium-review.googlesource.com/698875
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506643}

cc/tiles/checker_image_tracker.cc

@@ -36,10 +36,9 @@ enum class CheckerImagingDecision {
   // Vetoed because checkering of images has been disabled.
   kVetoedForceDisable = 9,
 
-  // Vetoed because we only checker images on tiles required for activation.
-  kVetoedNotRequiredForActivation = 10,
+  // 10 used to be kVetoedNotRequiredForActivation.
 
-  kCheckerImagingDecisionCount,
+  kCheckerImagingDecisionCount = 11,
 };
 
 std::string ToString(PaintImage::Id paint_image_id,
@@ -261,8 +260,7 @@ void CheckerImageTracker::DidFinishImageDecode(
 }
 
 bool CheckerImageTracker::ShouldCheckerImage(const DrawImage& draw_image,
-                                             WhichTree tree,
-                                             bool required_for_activation) {
+                                             WhichTree tree) {
   const PaintImage& image = draw_image.paint_image();
   PaintImage::Id image_id = image.stable_id();
   TRACE_EVENT1("cc", "CheckerImageTracker::ShouldCheckerImage", "image_id",
@@ -306,15 +304,11 @@ bool CheckerImageTracker::ShouldCheckerImage(const DrawImage& draw_image,
     CheckerImagingDecision decision = GetCheckerImagingDecision(
         image, draw_image.src_rect(), min_image_bytes_to_checker_,
         image_controller_->image_cache_max_limit_bytes());
-    if (decision == CheckerImagingDecision::kCanChecker) {
-      if (force_disabled_) {
-        // Get the decision for all the veto reasons first, so we can UMA the
-        // images that were not checkered only because checker-imaging was force
-        // disabled.
-        decision = CheckerImagingDecision::kVetoedForceDisable;
-      } else if (!required_for_activation) {
-        decision = CheckerImagingDecision::kVetoedNotRequiredForActivation;
-      }
+    if (decision == CheckerImagingDecision::kCanChecker && force_disabled_) {
+      // Get the decision for all the veto reasons first, so we can UMA the
+      // images that were not checkered only because checker-imaging was force
+      // disabled.
+      decision = CheckerImagingDecision::kVetoedForceDisable;
     }
 
     it->second.policy = decision == CheckerImagingDecision::kCanChecker

cc/tiles/checker_image_tracker.h

@@ -59,9 +59,7 @@ class CC_EXPORT CheckerImageTracker {
 
   // Returns true if the decode for |image| will be deferred to the image decode
   // service and it should be be skipped during raster.
-  bool ShouldCheckerImage(const DrawImage& image,
-                          WhichTree tree,
-                          bool required_for_activation);
+  bool ShouldCheckerImage(const DrawImage& image, WhichTree tree);
 
   // Provides a prioritized queue of images to decode.
   using ImageDecodeQueue = std::vector<ImageDecodeRequest>;

cc/tiles/checker_image_tracker_unittest.cc

@@ -126,9 +126,7 @@ class CheckerImageTrackerTest : public testing::Test,
   }
 
   bool ShouldCheckerImage(const DrawImage& draw_image, WhichTree tree) {
-    bool required_for_activation = true;
-    return checker_image_tracker_->ShouldCheckerImage(draw_image, tree,
-                                                      required_for_activation);
+    return checker_image_tracker_->ShouldCheckerImage(draw_image, tree);
   }
 
   CheckerImageTracker::ImageDecodeQueue BuildImageDecodeQueue(
@@ -564,26 +562,5 @@ TEST_F(CheckerImageTrackerTest, DisableForSoftwareRaster) {
   EXPECT_FALSE(ShouldCheckerImage(image2, WhichTree::PENDING_TREE));
 }
 
-TEST_F(CheckerImageTrackerTest, OnlyCheckerRequiredForActivation) {
-  SetUpTracker(true);
-
-  // Should checker when required for activation.
-  DrawImage image1 = CreateImage(ImageType::CHECKERABLE);
-  bool required_for_activation = true;
-  EXPECT_TRUE(checker_image_tracker_->ShouldCheckerImage(
-      image1, WhichTree::PENDING_TREE, required_for_activation));
-
-  // Now the same image is not required for activation. We should still continue
-  // checkering it.
-  required_for_activation = false;
-  EXPECT_TRUE(checker_image_tracker_->ShouldCheckerImage(
-      image1, WhichTree::PENDING_TREE, required_for_activation));
-
-  // New image should not be checkered if it is not required for activation.
-  DrawImage image2 = CreateImage(ImageType::CHECKERABLE);
-  EXPECT_FALSE(checker_image_tracker_->ShouldCheckerImage(
-      image2, WhichTree::PENDING_TREE, required_for_activation));
-}
-
 }  // namespace
 }  // namespace cc

cc/tiles/tile_manager.cc

@@ -901,8 +901,7 @@ void TileManager::PartitionImagesForCheckering(
           frame_index;
     }
 
-    if (checker_image_tracker_.ShouldCheckerImage(
-            draw_image, tree, tile->required_for_activation()))
+    if (checker_image_tracker_.ShouldCheckerImage(draw_image, tree))
       checkered_images->push_back(draw_image.paint_image());
     else
       sync_decoded_images->push_back(std::move(draw_image));
@@ -925,8 +924,7 @@ void TileManager::AddCheckeredImagesToDecodeQueue(
         original_draw_image->paint_image(), tree);
     DrawImage draw_image(*original_draw_image, tile->raster_transform().scale(),
                          frame_index, raster_color_space);
-    if (checker_image_tracker_.ShouldCheckerImage(
-            draw_image, tree, tile->required_for_activation())) {
+    if (checker_image_tracker_.ShouldCheckerImage(draw_image, tree)) {
       image_decode_queue->emplace_back(draw_image.paint_image(), decode_type);
     }
   }

cc/trees/layer_tree_host_unittest.cc

@@ -7861,24 +7861,20 @@ class LayerTreeHostTestQueueImageDecode : public LayerTreeHostTest {
   }
 
   void ReadyToCommitOnThread(LayerTreeHostImpl* impl) override {
-    const bool required_for_activation = true;
-
     if (one_commit_done_)
       return;
     EXPECT_TRUE(
         impl->tile_manager()->checker_image_tracker().ShouldCheckerImage(
-            image_, WhichTree::PENDING_TREE, required_for_activation));
+            image_, WhichTree::PENDING_TREE));
     // Reset the tracker as if it has never seen this image.
     impl->tile_manager()->checker_image_tracker().ClearTracker(true);
   }
 
   void CommitCompleteOnThread(LayerTreeHostImpl* impl) override {
-    const bool required_for_activation = true;
-
     one_commit_done_ = true;
     EXPECT_FALSE(
         impl->tile_manager()->checker_image_tracker().ShouldCheckerImage(
-            image_, WhichTree::PENDING_TREE, required_for_activation));
+            image_, WhichTree::PENDING_TREE));
   }
 
   void ImageDecodeFinished(bool decode_succeeded) {