cc: Keep PaintImage in DrawImage.

cc/tiles/image_controller.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/tiles/image_controller.h"
 
 #include "base/bind.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/task_scheduler/task_traits.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/base/completion_event.h"
 #include "cc/tiles/tile_task_manager.h"
 
 namespace cc {
 
 ImageController::ImageDecodeRequestId
     ImageController::s_next_image_decode_queue_id_ = 1;
 
 ImageController::ImageController(
     base::SequencedTaskRunner* origin_task_runner,
     scoped_refptr<base::SequencedTaskRunner> worker_task_runner)
     : worker_task_runner_(std::move(worker_task_runner)),
       origin_task_runner_(origin_task_runner),
+      paint_image_id_(PaintImage::GetNextId()),

[vmpstr, 2017/05/12 22:15:42]

This is something that we need to talk about I think. We need to be able to
plumb ... something.

Like, I think the cache should know the stable id for us to properly clean up
partially loaded images. Also, the checker tracker has to also never checker a
skia id which was decoded by the decode() call.

[Khushal, 2017/05/13 02:45:50]

We'll definitely need PaintImage plumbed to cc, instead of SkImage, for the
decode API. That shouldn't be difficult. And that should handle the
checker-imaging.

For clearing the caches, I think we should address that in the doc that
discusses unifying blink and cc's caching.

[vmpstr, 2017/05/15 15:02:55]

My concern is that it doesn't feel that we're cleanly defining the boundary
where PaintImage has data generated by blink and where it's being used as a
placeholder in draw images. I think we need to tune this to ensure that we
always get the right paint image (the id specifically). I think this should
naturally fall out from removing image hijack canvas, since we'll be passing the
right id there. 

the problem is that if I want to add any code that depends on the id into the
cache (such as clearing old entries), I need to know that each call has the
right id. If we sometimes pass the right id, and sometimes we just generate a
dummy one, then we'll end up in a situation where we might be clearing things we
didn't mean to, or not clearing things that we should.

[Khushal, 2017/05/15 21:22:14]

ImageHijackCanvas is one use-case. The other one is the decode API.
crbug.com/722559 has info about all these use-cases right now. The plumbing we
have so far is enough for the checker-imaging system, but the hijack canvas and
decode API will definitely need to be handled before the cache can make use of
this id.

       weak_ptr_factory_(this) {}
 
 ImageController::~ImageController() {
   StopWorkerTasks();
   for (auto& request : orphaned_decode_requests_)
     request.callback.Run(request.id, ImageDecodeResult::FAILURE);
 }
 
 void ImageController::StopWorkerTasks() {
   // We can't have worker threads without a cache_ or a worker_task_runner_, so
@@ skipping 153 matching lines @@
 
   // TODO(ccameron): The target color space specified here should match the
   // target color space that will be used at rasterization time. Leave this
   // unspecified now, since that will match the rasterization-time color
   // space while color correct rendering is disabled.
   gfx::ColorSpace target_color_space;
 
   DCHECK(image);
   bool is_image_lazy = image->isLazyGenerated();
   auto image_bounds = image->bounds();
-  DrawImage draw_image(std::move(image), image_bounds, kNone_SkFilterQuality,
-                       SkMatrix::I(), target_color_space);
+  DrawImage draw_image(
+      PaintImage(paint_image_id_,
+                 sk_ref_sp(const_cast<SkImage*>(image.release()))),
+      image_bounds, kNone_SkFilterQuality, SkMatrix::I(), target_color_space);
 
   // Get the tasks for this decode.
   scoped_refptr<TileTask> task;
   bool need_unref = false;
   if (is_image_lazy) {
     need_unref =
         cache_->GetOutOfRasterDecodeTaskForImageAndRef(draw_image, &task);
   }
   // If we don't need to unref this, we don't actually have a task.
   DCHECK(need_unref || !task);
@@ skipping 164 matching lines @@
 ImageController::ImageDecodeRequest::ImageDecodeRequest(
     const ImageDecodeRequest& other) = default;
 ImageController::ImageDecodeRequest::~ImageDecodeRequest() = default;
 
 ImageController::ImageDecodeRequest& ImageController::ImageDecodeRequest::
 operator=(ImageDecodeRequest&& other) = default;
 ImageController::ImageDecodeRequest& ImageController::ImageDecodeRequest::
 operator=(const ImageDecodeRequest& other) = default;
 
 }  // namespace cc