cc: Keep PaintImage in DrawImage.

cc/paint/discardable_image_store.cc

Index: cc/paint/discardable_image_store.cc
diff --git a/cc/paint/discardable_image_store.cc b/cc/paint/discardable_image_store.cc
index 3ec50f2a742b872815d7391fbc4692c45f370d5a..e18dad7bdb521f989f86a99b12ab0a0ab19c3721 100644
--- a/cc/paint/discardable_image_store.cc
+++ b/cc/paint/discardable_image_store.cc
@@ -72,11 +72,10 @@ DiscardableImageStore::DiscardableImageStore(
     int width,
     int height,
     std::vector<std::pair<DrawImage, gfx::Rect>>* image_set,
-    base::flat_map<ImageId, gfx::Rect>* image_id_to_rect)
+    base::flat_map<PaintImage::Id, gfx::Rect>* image_id_to_rect)
     : canvas_(base::MakeUnique<PaintTrackingCanvas>(width, height)),
       image_set_(image_set),
-      image_id_to_rect_(image_id_to_rect),
-      unknown_stable_id_(PaintImage::GetNextId()) {}
+      image_id_to_rect_(image_id_to_rect) {}
 
 DiscardableImageStore::~DiscardableImageStore() = default;
 
@@ -183,24 +182,30 @@ void DiscardableImageStore::AddImageFromFlags(const SkRect& rect,
     SkShader::TileMode xy[2];
     SkImage* image = shader->isAImage(&matrix, xy);
     if (image) {
-      PaintImage paint_image(unknown_stable_id_, sk_ref_sp(image),
+      // We currently use the wrong id for images that come from shaders. We
+      // don't know what the stable id is, but since the completion and
+      // animation states are both unknown, this value doesn't matter as it
+      // won't be used in checker imaging anyway. Keep this value the same to
+      // avoid id churn.
+      // TODO(vmpstr): Remove this when we can add paint images into shaders
+      // directly.
+      PaintImage paint_image(PaintImage::kWrongStableId, sk_ref_sp(image),

[vmpstr, 2017/05/16 17:43:27]

Maybe call it kUnknownStableId? If it's wrong, then why are you using it? :)

[Khushal, 2017/05/16 21:29:12]

Done.
I thought calling it wrong will give me a push to eliminate these cases sooner.
:P

                              PaintImage::AnimationType::UNKNOWN,
                              PaintImage::CompletionState::UNKNOWN);
       // TODO(ericrk): Handle cases where we only need a sub-rect from the
       // image. crbug.com/671821
-      AddImage(paint_image, SkRect::MakeFromIRect(image->bounds()), rect,
-               &matrix, flags);
+      AddImage(std::move(paint_image), SkRect::MakeFromIRect(image->bounds()),
+               rect, &matrix, flags);
     }
   }
 }
 
-void DiscardableImageStore::AddImage(const PaintImage& paint_image,
+void DiscardableImageStore::AddImage(PaintImage paint_image,
                                      const SkRect& src_rect,
                                      const SkRect& rect,
                                      const SkMatrix* local_matrix,
                                      const PaintFlags& flags) {
-  sk_sp<const SkImage> sk_image = paint_image.sk_image();
-  if (!sk_image->isLazyGenerated())
+  if (!paint_image.sk_image()->isLazyGenerated())
     return;
 
   const SkRect& clip_rect = SkRect::Make(canvas_->getDeviceClipBounds());
@@ -244,11 +249,10 @@ void DiscardableImageStore::AddImage(const PaintImage& paint_image,
   if (local_matrix)
     matrix.postConcat(*local_matrix);
 
-  // TODO(khushalsagar): Keep PaintImage in DrawImage.
-  (*image_id_to_rect_)[sk_image->uniqueID()].Union(image_rect);
+  (*image_id_to_rect_)[paint_image.stable_id()].Union(image_rect);

[vmpstr, 2017/05/16 17:43:27]

I don't think it is, but is it possible for us to have two different uniqueIDs
in the same raster source with the same stable id?

[Khushal, 2017/05/16 21:29:12]

It shouldn't be. The uniqueID essentially represents the current frame for a
blink::Image and all elements using that image are dirtied and repainted
together if the image changes.

   image_set_->push_back(
-      std::make_pair(DrawImage(std::move(sk_image), src_irect, filter_quality,
-                               matrix, target_color_space),
+      std::make_pair(DrawImage(std::move(paint_image), src_irect,
+                               filter_quality, matrix, target_color_space),
                      image_rect));
 }