cc: Make image controller return a status with the callback.

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"
@@ skipping 69 matching lines @@
     // The task (if one exists) would have run already, so we just need to
     // complete it.
     if (request.task)
       request.task->DidComplete();
 
     // Issue the callback, and unref the image immediately. This is so that any
     // code waiting on the callback can proceed, although we're breaking the
     // promise of having this image decoded. This is unfortunate, but it seems
     // like the least complexity to process an image decode controller becoming
     // nullptr.
-    request.callback.Run(id);
+    // TODO(vmpstr): We can move these back to |image_decode_queue_| instead
+    // and defer completion until we get a new cache and process the request
+    // again. crbug.com/693692.
+    request.callback.Run(id, ImageDecodeResult::FAILURE);
     if (request.need_unref)
       cache_->UnrefImage(request.draw_image);
   }
   requests_needing_completion_.clear();
 
   // Finally, complete all of the tasks that never started running. This is
   // similar to the |requests_needing_completion_|, but happens at a different
   // stage in the pipeline.
   for (auto& request_pair : image_decode_queue_) {
     ImageDecodeRequestId id = request_pair.first;
     ImageDecodeRequest& request = request_pair.second;
 
     if (request.task) {
       // This task may have run via a different request, so only cancel it if
       // it's "new". That is, the same task could have been referenced by
       // several different image deque requests for the same image.
       if (request.task->state().IsNew())
         request.task->state().DidCancel();
       request.task->DidComplete();
     }
     // Run the callback and unref the image.
-    request.callback.Run(id);
+    // TODO(vmpstr): We can regenerate the tasks for the new cache if we get
+    // one. crbug.com/693692.
+    request.callback.Run(id, ImageDecodeResult::FAILURE);
     cache_->UnrefImage(request.draw_image);
   }
   image_decode_queue_.clear();
 }
 
 void ImageController::SetImageDecodeCache(ImageDecodeCache* cache) {
   if (!cache) {
     SetPredecodeImages(std::vector<DrawImage>(),
                        ImageDecodeCache::TracingInfo());
     StopWorkerTasks();
@@ skipping 43 matching lines @@
 ImageController::ImageDecodeRequestId ImageController::QueueImageDecode(
     sk_sp<const SkImage> image,
     const ImageDecodedCallback& callback) {
   // We must not receive any image requests if we have no worker.
   CHECK(worker_task_runner_);
 
   // Generate the next id.
   ImageDecodeRequestId id = s_next_image_decode_queue_id_++;
 
   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());
 
   // Get the tasks for this decode.
   scoped_refptr<TileTask> task;
-  bool need_unref =
-      cache_->GetOutOfRasterDecodeTaskForImageAndRef(draw_image, &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);
 
   // Schedule the task and signal that there is more work.
   base::AutoLock hold(lock_);
   image_decode_queue_[id] =
       ImageDecodeRequest(id, draw_image, callback, std::move(task), need_unref);
 
   // If this is the only image decode request, schedule a task to run.
   // Otherwise, the task will be scheduled in the previou task's completion.
@@ skipping 56 matching lines @@
     decode.task->RunOnWorkerThread();
     decode.task->state().DidFinish();
   }
   origin_task_runner_->PostTask(
       FROM_HERE, base::Bind(&ImageController::ImageDecodeCompleted,
                             weak_ptr_factory_.GetWeakPtr(), decode.id));
 }
 
 void ImageController::ImageDecodeCompleted(ImageDecodeRequestId id) {
   ImageDecodedCallback callback;
+  ImageDecodeResult result = ImageDecodeResult::SUCCESS;
   {
     base::AutoLock hold(lock_);
 
     auto request_it = requests_needing_completion_.find(id);
     DCHECK(request_it != requests_needing_completion_.end());
     id = request_it->first;
     ImageDecodeRequest& request = request_it->second;
 
+    // First, Determine the status of the decode. This has to happen here, since
+    // we conditionally move from the draw image below.
+    // Also note that if we don't need an unref for a lazy decoded images, it
+    // implies that we never attempted the decode. Some of the reasons for this
+    // would be that the image is of an empty size, or if the image doesn't fit
+    // into memory. In all cases, this implies that the decode was a failure.

[Khushal, 2017/02/17 23:56:38]

What if we tried to decode the image and it was corrupt or something, it
wouldn't cover that case right? It probably doesn't matter but I was reading the
comment and thought about this.

[vmpstr, 2017/02/24 02:43:49]

As discussed offline, I think this is fine, since the failure case is guaranteed
to be a failure.

+    if (!request.draw_image.image()->isLazyGenerated())

[Khushal, 2017/02/17 23:56:38]

Could you add a ImageControllerTest for this?

[vmpstr, 2017/02/24 02:43:49]

Done.

+      result = ImageDecodeResult::DECODE_NOT_REQUIRED;
+    else if (!request.need_unref)
+      result = ImageDecodeResult::FAILURE;
+    else
+      result = ImageDecodeResult::SUCCESS;
+
     // If we need to unref this decode, then we have to put it into the locked
     // images vector.
     if (request.need_unref)
       requested_locked_images_[id] = std::move(request.draw_image);
 
     // If we have a task that isn't completed yet, we need to complete it.
     if (request.task && !request.task->HasCompleted()) {
       request.task->OnTaskCompleted();
       request.task->DidComplete();
     }
+
     // Finally, save the callback so we can run it without the lock, and erase
     // the request from |requests_needing_completion_|.
     callback = std::move(request.callback);
     requests_needing_completion_.erase(request_it);
   }
 
   // Post another task to run.
   worker_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&ImageController::ProcessNextImageDecodeOnWorkerThread,
                  base::Unretained(this)));
 
   // Finally run the requested callback.
-  callback.Run(id);
+  callback.Run(id, result);
 }
 
 ImageController::ImageDecodeRequest::ImageDecodeRequest() = default;
 ImageController::ImageDecodeRequest::ImageDecodeRequest(
     ImageDecodeRequestId id,
     const DrawImage& draw_image,
     const ImageDecodedCallback& callback,
     scoped_refptr<TileTask> task,
     bool need_unref)
     : id(id),
       draw_image(draw_image),
       callback(callback),
       task(std::move(task)),
       need_unref(need_unref) {}
 ImageController::ImageDecodeRequest::ImageDecodeRequest(
     ImageDecodeRequest&& other) = default;
 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