Re-land: cc: Add raster finished signals to RasterWorkerPool.

cc/resources/raster_worker_pool.cc

 // Copyright 2013 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/resources/raster_worker_pool.h"
 
 #include "base/json/json_writer.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "cc/debug/devtools_instrumentation.h"
@@ skipping 206 matching lines @@
 
  private:
   virtual ~RasterFinishedWorkerPoolTaskImpl() {}
 
   scoped_refptr<base::MessageLoopProxy> origin_loop_;
   const base::Closure on_raster_finished_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(RasterFinishedWorkerPoolTaskImpl);
 };
 
-void Noop() {}
-
 const char* kWorkerThreadNamePrefix = "CompositorRaster";
 
 }  // namespace
 
 namespace internal {
 
 RasterWorkerPoolTask::RasterWorkerPoolTask(
     const Resource* resource, TaskVector* dependencies)
     : did_run_(false),
       did_complete_(false),
@@ skipping 89 matching lines @@
 
 void RasterWorkerPool::RasterTask::Reset() {
   internal_ = NULL;
 }
 
 RasterWorkerPool::RasterTask::~RasterTask() {
 }
 
 RasterWorkerPool::RasterTaskGraph::RasterTaskGraph()
     : raster_finished_node_(new GraphNode),
-      next_priority_(1u) {
+      raster_required_for_activation_finished_node_(new GraphNode),
+      next_priority_(2u) {  // priority 0-1 is reserved for OnRasterFinished.
+  raster_finished_node_->add_dependency();
+  raster_required_for_activation_finished_node_->add_dependent(
+      raster_finished_node_.get());
 }
 
 RasterWorkerPool::RasterTaskGraph::~RasterTaskGraph() {
 }
 
 void RasterWorkerPool::RasterTaskGraph::InsertRasterTask(
     internal::WorkerPoolTask* raster_task,
-    const TaskVector& decode_tasks) {
+    const TaskVector& decode_tasks,
+    bool required_for_activation) {
   DCHECK(!raster_task->HasCompleted());
   DCHECK(graph_.find(raster_task) == graph_.end());
 
   scoped_ptr<GraphNode> raster_node(new GraphNode);
   raster_node->set_task(raster_task);
   raster_node->set_priority(next_priority_++);
 
   // Insert image decode tasks.
   for (TaskVector::const_iterator it = decode_tasks.begin();
        it != decode_tasks.end(); ++it) {
@@ skipping 13 matching lines @@
       continue;
     }
 
     scoped_ptr<GraphNode> decode_node(new GraphNode);
     decode_node->set_task(decode_task);
     decode_node->set_priority(next_priority_++);
     decode_node->add_dependent(raster_node.get());
     graph_.set(decode_task, decode_node.Pass());
   }
 
-  raster_finished_node_->add_dependency();
-  raster_node->add_dependent(raster_finished_node_.get());
+  if (required_for_activation) {
+    raster_required_for_activation_finished_node_->add_dependency();
+    raster_node->add_dependent(
+        raster_required_for_activation_finished_node_.get());
+  } else {
+    raster_finished_node_->add_dependency();
+    raster_node->add_dependent(raster_finished_node_.get());
+  }
 
   graph_.set(raster_task, raster_node.Pass());
 }
 
 // static
 RasterWorkerPool::RasterTask RasterWorkerPool::CreateRasterTask(
     const Resource* resource,
     PicturePileImpl* picture_pile,
     gfx::Rect content_rect,
     float contents_scale,
@@ skipping 37 matching lines @@
 }
 
 RasterWorkerPool::~RasterWorkerPool() {
 }
 
 void RasterWorkerPool::SetClient(RasterWorkerPoolClient* client) {
   client_ = client;
 }
 
 void RasterWorkerPool::Shutdown() {
+  // Cancel any pending OnRasterFinished callbacks.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+  raster_tasks_.clear();
   TaskGraph empty;
   SetTaskGraph(&empty);
   WorkerPool::Shutdown();
-  raster_tasks_.clear();
-  // Cancel any pending OnRasterFinished callback.
-  weak_ptr_factory_.InvalidateWeakPtrs();
 }
 
 void RasterWorkerPool::SetRasterTasks(RasterTask::Queue* queue) {
   raster_tasks_.swap(queue->tasks_);
   raster_tasks_required_for_activation_.swap(
       queue->tasks_required_for_activation_);
 }
 
 void RasterWorkerPool::SetRasterTaskGraph(RasterTaskGraph* graph) {
   scoped_ptr<GraphNode> raster_finished_node(
       graph->raster_finished_node_.Pass());
+  scoped_ptr<GraphNode> raster_required_for_activation_finished_node(
+      graph->raster_required_for_activation_finished_node_.Pass());
   TaskGraph new_graph;
   new_graph.swap(graph->graph_);
 
-  if (new_graph.empty()) {
-    SetTaskGraph(&new_graph);
-    raster_finished_task_ = NULL;
-    return;
-  }
-
   ++schedule_raster_tasks_count_;
 
   scoped_refptr<internal::WorkerPoolTask> new_raster_finished_task(
       new RasterFinishedWorkerPoolTaskImpl(
           base::MessageLoopProxy::current(),
           base::Bind(&RasterWorkerPool::OnRasterFinished,
                      weak_ptr_factory_.GetWeakPtr(),
                      schedule_raster_tasks_count_)));
   raster_finished_node->set_task(new_raster_finished_task.get());
-  // Insert "raster finished" task before switching to new graph.
+  raster_finished_node->set_priority(0u);
   new_graph.set(new_raster_finished_task.get(), raster_finished_node.Pass());
+
+  scoped_refptr<internal::WorkerPoolTask>
+      new_raster_required_for_activation_finished_task(

[vmpstr, 2013/06/24 16:34:03]

Should this one be marked as a dependency of the raster_finished_node?

[reveman, 2013/06/24 19:05:17]

That's currently done in RasterTaskGraph ctor.

+          new RasterFinishedWorkerPoolTaskImpl(
+              base::MessageLoopProxy::current(),
+              base::Bind(
+                  &RasterWorkerPool::OnRasterRequiredForActivationFinished,
+                  weak_ptr_factory_.GetWeakPtr(),
+                  schedule_raster_tasks_count_)));
+  raster_required_for_activation_finished_node->set_task(
+      new_raster_required_for_activation_finished_task.get());
+  raster_required_for_activation_finished_node->set_priority(1u);

[vmpstr, 2013/06/24 16:34:03]

As a follow-up, we might want to do something like set_task_with_priority, since
any time we call set_task, we immediately call set_priority.

[reveman, 2013/06/24 19:05:17]

Sounds good. This will likely be done as part of the switch to using a vector
which I think will require similar changes.

+  new_graph.set(new_raster_required_for_activation_finished_task.get(),
+                raster_required_for_activation_finished_node.Pass());
+
   SetTaskGraph(&new_graph);
+
   raster_finished_task_.swap(new_raster_finished_task);
+  raster_required_for_activation_finished_task_.swap(
+      new_raster_required_for_activation_finished_task);
 }
 
 bool RasterWorkerPool::IsRasterTaskRequiredForActivation(
     internal::RasterWorkerPoolTask* task) const {
   return
       raster_tasks_required_for_activation_.find(task) !=
       raster_tasks_required_for_activation_.end();
 }
 
-void RasterWorkerPool::OnRasterFinished(int64 schedule_raster_tasks_count) {
+void RasterWorkerPool::OnRasterFinished(
+    int64 schedule_raster_tasks_count) {
   TRACE_EVENT1("cc", "RasterWorkerPool::OnRasterFinished",
                "schedule_raster_tasks_count", schedule_raster_tasks_count);
   DCHECK_GE(schedule_raster_tasks_count_, schedule_raster_tasks_count);
-  // Call OnRasterTasksFinished() when we've finished running all raster
-  // tasks needed since last time SetRasterTaskGraph() was called.
+  // Call OnRasterTasksFinished() if this callback is
+  // associated with the last call to SetRasterTaskGraph().
   if (schedule_raster_tasks_count_ == schedule_raster_tasks_count)
     OnRasterTasksFinished();
 }
 
+void RasterWorkerPool::OnRasterRequiredForActivationFinished(
+    int64 schedule_raster_tasks_count) {
+  TRACE_EVENT1("cc", "RasterWorkerPool::OnRasterRequiredForActivationFinished",
+               "schedule_raster_tasks_count", schedule_raster_tasks_count);
+  DCHECK_GE(schedule_raster_tasks_count_, schedule_raster_tasks_count);
+  // Call OnRasterTasksRequiredForActivationFinished() if this callback is
+  // associated with the last call to SetRasterTaskGraph().
+  if (schedule_raster_tasks_count_ == schedule_raster_tasks_count)
+    OnRasterTasksRequiredForActivationFinished();
+}
+
 }  // namespace cc