cc: Add image decode queue functionality to image manager.

cc/tiles/image_controller.cc

Index: cc/tiles/image_controller.cc
diff --git a/cc/tiles/image_controller.cc b/cc/tiles/image_controller.cc
index 9572956864817d9a69dd6e45dbb93802b043e7f3..e03a109632966341d405f748de5139ed5bb65287 100644
--- a/cc/tiles/image_controller.cc
+++ b/cc/tiles/image_controller.cc
@@ -4,17 +4,146 @@
 
 #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 {
+namespace {
+void SignalCompletionEvent(CompletionEvent* event) {
+  event->Signal();
+}
+
+const int kNumFramesToLock = 2;
+}  // namespace
+
+ImageController::ImageDecodeRequestId
+    ImageController::s_next_image_decode_queue_id_ = 1;
+
+ImageController::ImageController(
+    base::SequencedTaskRunner* origin_task_runner,
+    scoped_refptr<base::SequencedTaskRunner> worker_task_runner)

[enne (OOO), 2017/01/03 21:15:25]

Do you need the "sequenced" part of sequenced task runner? It seems like the
guarantees about ordering are not important for an image decode service.

[vmpstr, 2017/01/03 21:40:34]

I don't really need it, but it's what was recommended I use <_< I can
investigate whether there's a non sequenced thing I could use. 

+    : origin_task_runner_(origin_task_runner),
+      worker_task_runner_(std::move(worker_task_runner)),
+      weak_ptr_factory_(this) {}
+
+ImageController::~ImageController() {
+  StopWorkerTasks();
+}
+
+void ImageController::ResumeWorkerTasks() {
+  if (!worker_task_runner_)
+    return;
+
+  base::AutoLock hold(lock_);
+  if (!image_decode_queue_.empty()) {
+    worker_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&ImageController::ProcessNextImageDecodeOnWorkerThread,
+                   base::Unretained(this)));
+  }
+}
+
+void ImageController::StopWorkerTasks() {
+  // We can't have worker threads without a cache_ or a worker_task_runner_, so
+  // terminate early.
+  if (!cache_ || !worker_task_runner_)
+    return;
+
+  // Abort all tasks that are currently scheduled to run (we'll wait for them to
+  // finish next).
+  {
+    base::AutoLock hold(lock_);
+    abort_tasks_ = true;
+  }
 
-ImageController::ImageController() = default;
-ImageController::~ImageController() = default;
+  // Post a task that will simply signal a completion event to ensure that we
+  // "flush" any scheduled tasks (they will abort).
+  CompletionEvent completion_event;
+  worker_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&SignalCompletionEvent, base::Unretained(&completion_event)));
+  completion_event.Wait();
+
+  // Reset the abort flag so that new tasks can be scheduled.
+  {
+    base::AutoLock hold(lock_);
+    abort_tasks_ = false;
+  }
+
+  // Now that we flushed everything, if there was a task running and it
+  // finished. It would have posted a completion callback back to the compositor

[ericrk, 2017/01/04 07:48:31]

nit: sentence is a bit weird - did you mean "finished it", rather than
"finished. It"?

[vmpstr, 2017/01/06 01:19:03]

Yes, my bad. Fixed.

+  // thread. We don't want that, so invalidate the weak ptrs again. Note that
+  // nothing can start running between wait and this invalidate, since it would
+  // only run on the current (compositor) thread.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
+  // Now, begin cleanup.
+
+  // Unlock all of the locked images (note that this vector would only be
+  // populated if we actually need to unref the image.
+  for (auto image_pair : requested_locked_images_)
+    cache_->UnrefImage(image_pair.first);
+  requested_locked_images_.clear();
+
+  // Now, complete the tasks that already ran but haven't completed. These would
+  // be posted in the run loop, but since we invalidated the weak ptrs, we need
+  // to run everything manually.
+  for (auto& request_to_complete : requests_needing_completion_) {
+    ImageDecodeRequestId id = request_to_complete.first;
+    ImageDecodeRequest& request = request_to_complete.second;
+
+    // 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);
+    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);
+    cache_->UnrefImage(request.draw_image);
+  }
+  image_decode_queue_.clear();
+}
 
 void ImageController::SetImageDecodeCache(ImageDecodeCache* cache) {
   if (!cache) {
     SetPredecodeImages(std::vector<DrawImage>(),
                        ImageDecodeCache::TracingInfo());
+    StopWorkerTasks();
   }
   cache_ = cache;
+  if (cache_)
+    ResumeWorkerTasks();
 }
 
 void ImageController::GetTasksForImagesAndRef(
@@ -56,4 +185,160 @@ std::vector<scoped_refptr<TileTask>> ImageController::SetPredecodeImages(
   return new_tasks;
 }
 
+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);
+  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);
+  // 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.
+  if (image_decode_queue_.size() == 1) {
+    // Post a worker task.
+    worker_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&ImageController::ProcessNextImageDecodeOnWorkerThread,
+                   base::Unretained(this)));
+  }
+
+  return id;
+}
+
+void ImageController::NotifyFrameFinished() {
+  // Reduce the locked frame count on all images and unlock them if the count
+  // reaches 0.
+  for (auto it = requested_locked_images_.begin();
+       it != requested_locked_images_.end();) {
+    if (--it->second == 0) {
+      UnrefImages({it->first});
+      it = requested_locked_images_.erase(it);
+    } else {
+      ++it;
+    }
+  }
+}
+
+void ImageController::ProcessNextImageDecodeOnWorkerThread() {
+  TRACE_EVENT0("cc", "ImageController::ProcessNextImageDecodeOnWorkerThread");
+  base::AutoLock hold(lock_);
+
+  // If we don't have any work, abort.
+  if (image_decode_queue_.empty() || abort_tasks_)
+    return;
+
+  // Take the next request from the queue.
+  auto decode_it = image_decode_queue_.begin();
+  DCHECK(decode_it != image_decode_queue_.end());
+  ImageDecodeRequest decode = std::move(decode_it->second);
+  image_decode_queue_.erase(decode_it);
+
+  // Notify that the task will need completion. Note that there are two cases
+  // where we process this. First, we might complete this task as a response
+  // to the posted task below. Second, we might complete it in
+  // StopWorkerTasks(). In either case, the task would have already run
+  // (either post task happens after running, or the thread was already joined
+  // which means the task ran). This means that we can put the decode into
+  // |requests_needing_completion_| here before actually running the task.
+  requests_needing_completion_[decode.id] = decode;
+
+  {
+    base::AutoUnlock unlock(lock_);

[enne (OOO), 2017/01/03 21:15:25]

Will this cause an extra lock/unlock to have this here as opposed to just
scoping the AutoLock above?

[vmpstr, 2017/01/03 21:40:34]

Oh good point.

+    // Run the task if we need to run it. If the task state isn't new, then
+    // there is another task that is responsible for finishing it and cleaning
+    // up (and it already ran); we just need to post a completion callback.
+    // Note that the other tasks's completion will also run first, since the
+    // requests are ordered. So, when we process this task's completion, we
+    // won't actually do anything with the task and simply issue the callback.
+    if (decode.task && decode.task->state().IsNew()) {
+      decode.task->state().DidSchedule();
+      decode.task->state().DidStart();
+      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;
+  {
+    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;
+
+    // If we need to urnef this decode, then we have to put it into the locked

[enne (OOO), 2017/01/03 21:15:25]

⚱ ef

[vmpstr, 2017/01/03 21:40:34]

<_<

+    // images vector.
+    if (request.need_unref) {
+      requested_locked_images_.push_back(
+          std::make_pair(std::move(request.draw_image), kNumFramesToLock));
+    }
+    // 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);
+}
+
+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