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

cc/resources/raster_worker_pool.cc

Index: cc/resources/raster_worker_pool.cc
diff --git a/cc/resources/raster_worker_pool.cc b/cc/resources/raster_worker_pool.cc
index 5f356e5b2122a304eb8eda800af5fd09e0914d3d..9ccef6ac924e839b9d8742ad3261752b047e2ba9 100644
--- a/cc/resources/raster_worker_pool.cc
+++ b/cc/resources/raster_worker_pool.cc
@@ -335,7 +335,8 @@ 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.
 }
 
 RasterWorkerPool::RasterTaskGraph::~RasterTaskGraph() {
@@ -343,7 +344,8 @@ 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());
 
@@ -380,6 +382,12 @@ void RasterWorkerPool::RasterTaskGraph::InsertRasterTask(
   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();

[vmpstr, 2013/06/21 18:38:33]

Could you maybe add required_for_activation_task as a dependency of
raster_finished task? I mean this works as well, it would just seem nicer I
think. That is, any raster task would only have one dependent, and the dependent
of required_for_activation is always raster_finished.

[reveman, 2013/06/24 16:04:03]

Done.

+    raster_node->add_dependent(
+        raster_required_for_activation_finished_node_.get());
+  }
+
   graph_.set(raster_task, raster_node.Pass());
 }
 
@@ -436,12 +444,12 @@ void RasterWorkerPool::SetClient(RasterWorkerPoolClient* 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) {
@@ -453,15 +461,11 @@ void RasterWorkerPool::SetRasterTasks(RasterTask::Queue* queue) {
 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(
@@ -469,12 +473,34 @@ void RasterWorkerPool::SetRasterTaskGraph(RasterTaskGraph* graph) {
           base::MessageLoopProxy::current(),
           base::Bind(&RasterWorkerPool::OnRasterFinished,
                      weak_ptr_factory_.GetWeakPtr(),
-                     schedule_raster_tasks_count_)));
+                     schedule_raster_tasks_count_,
+                     base::Bind(&RasterWorkerPool::OnRasterTasksFinished,
+                                weak_ptr_factory_.GetWeakPtr()))));
   raster_finished_node->set_task(new_raster_finished_task.get());
-  // Insert "raster finished" task before switching to new graph.
+  raster_finished_node->set_priority(1u);
   new_graph.set(new_raster_finished_task.get(), raster_finished_node.Pass());
+
+  scoped_refptr<internal::WorkerPoolTask>
+      new_raster_required_for_activation_finished_task(
+          new RasterFinishedWorkerPoolTaskImpl(
+              base::MessageLoopProxy::current(),
+              base::Bind(&RasterWorkerPool::OnRasterFinished,
+                         weak_ptr_factory_.GetWeakPtr(),
+                         schedule_raster_tasks_count_,
+                         base::Bind(&RasterWorkerPool::

[vmpstr, 2013/06/21 18:38:33]

Would it be better if we just explicitly call this from OnRasterFinished? This
is kind of messy: a bind within a bind but both binds are for the same class

[reveman, 2013/06/24 16:04:03]

Done. I used two levels of bind here to avoid OnRasterFinished code duplication
but that's probably not worth it.

+                                    OnRasterTasksRequiredForActivationFinished,
+                                    weak_ptr_factory_.GetWeakPtr()))));
+  raster_required_for_activation_finished_node->set_task(
+      new_raster_required_for_activation_finished_task.get());
+  raster_required_for_activation_finished_node->set_priority(0u);
+  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(
@@ -484,14 +510,16 @@ bool RasterWorkerPool::IsRasterTaskRequiredForActivation(
       raster_tasks_required_for_activation_.end();
 }
 
-void RasterWorkerPool::OnRasterFinished(int64 schedule_raster_tasks_count) {
+void RasterWorkerPool::OnRasterFinished(
+    int64 schedule_raster_tasks_count,
+    const base::Closure& on_raster_tasks_finished_callback) {
   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.
+  // Run |on_raster_tasks_finished_callback| if this callback is associated
+  // with the last call to SetRasterTaskGraph().
   if (schedule_raster_tasks_count_ == schedule_raster_tasks_count)
-    OnRasterTasksFinished();
+    on_raster_tasks_finished_callback.Run();
 }
 
 }  // namespace cc