cc: Move internal::Task::did_schedule_ to internal::WorkerPoolTask.

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 0da81e247c64a1059e5587cb1d83651ef78c266d..b43bd4edfd9e7fb2abec405e5c8d03ae01aecd7b 100644
--- a/cc/resources/raster_worker_pool.cc
+++ b/cc/resources/raster_worker_pool.cc
@@ -456,13 +456,22 @@ int g_num_raster_threads = 0;
 
 namespace internal {
 
-WorkerPoolTask::WorkerPoolTask() : did_complete_(false) {}
+WorkerPoolTask::WorkerPoolTask() : did_schedule_(false), did_complete_(false) {}
 
 WorkerPoolTask::~WorkerPoolTask() {
   DCHECK_EQ(did_schedule_, did_complete_);
   DCHECK(!did_run_ || did_complete_);
 }
 
+void WorkerPoolTask::WillSchedule() {}
+
+void WorkerPoolTask::DidSchedule() {
+  did_schedule_ = true;
+  did_complete_ = false;
+}
+
+bool WorkerPoolTask::HasBeenScheduled() const { return did_schedule_; }
+
 void WorkerPoolTask::WillComplete() { DCHECK(!did_complete_); }
 
 void WorkerPoolTask::DidComplete() {
@@ -620,6 +629,17 @@ void RasterWorkerPool::SetTaskGraph(TaskGraph* graph) {
   TRACE_EVENT1(
       "cc", "RasterWorkerPool::SetTaskGraph", "num_tasks", graph->size());
 
+  for (internal::GraphNode::Map::iterator it = graph->begin();
+       it != graph->end();
+       ++it) {
+    internal::WorkerPoolTask* task =

[vmpstr, 2014/02/05 16:56:23]

Can you do a DCHECK here to ensure that this task isn't done yet? Something
along the lines of !HasFinishedRunning or !HasBeenScheduled.

My concern is that will/did schedule indicates that this task will eventually
run, is it possible for the task graph runner to not do that for whatever
reason?

[reveman, 2014/02/05 18:00:00]

The task might have finished running. We can't know that here. Schedule can be
called multiple times before CompleteOnOriginThread is called. Maybe we should
reset did_schedule_ in DidComplete() and only call Schedule here if
!HasBeenScheduled. That would make life a bit easier for the task
implementation. Wdyt?
It could be canceled if the next call to SetTaskGraph doesn't include this task.

[vmpstr, 2014/02/05 18:07:42]

I think that makes sense. From the task perspective then it's scheduled once
and it will just keep running until it completes. The fact that it is scheduled
multiple times should be opaque to the task. I mean if it is scheduled once
and run multiple times should be the same as if it goes through multiple
schedule/run cycles from the task perspective.

[reveman, 2014/02/05 19:52:42]

Take a look at latest patch.

+        static_cast<internal::WorkerPoolTask*>(it->first);
+
+    task->WillSchedule();
+    task->ScheduleOnOriginThread(this);
+    task->DidSchedule();
+  }
+
   g_task_graph_runner.Pointer()->SetTaskGraph(namespace_token_, graph);
 }
 
@@ -643,6 +663,7 @@ void RasterWorkerPool::RunGpuRasterTasks(const RasterTaskVector& tasks) {
     internal::RasterWorkerPoolTask* task = it->get();
     DCHECK(task->use_gpu_rasterization());
 
+    task->WillSchedule();

[vmpstr, 2014/02/05 16:56:23]

Does this also need a ScheduleOnOriginThread? There's a DCHECK in the task that
it's not a gpu rasterization task, but I'm wondering whether that DCHECK should
be an early out instead? I mean it kind of makes sense to me that all tasks do a
WillSchedule/Schedule/DidSchedule cycle instead of special casing GPU tasks to
not Schedule. Wdyt?

I was also thinking of having two ScheduleOnOriginThreadForWorkerThread and
ScheduleOnOriginThreadForOriginThread kind of thing, but I think that's
overkill.

[reveman, 2014/02/05 18:00:00]

I agree. Did the same for CompleteOnOriginThread().

[vmpstr, 2014/02/05 18:07:42]

Thanks! I think it looks nice now (ie symmetrical with the way other tasks are
run).

     task->DidSchedule();
     task->WillRun();
     task->RunOnOriginThread(resource_provider_, context_provider_);
@@ -733,8 +754,6 @@ internal::GraphNode* RasterWorkerPool::CreateGraphNodeForRasterTask(
     const internal::Task::Vector& decode_tasks,
     unsigned priority,
     TaskGraph* graph) {
-  DCHECK(!raster_task->HasCompleted());
-
   internal::GraphNode* raster_node =
       CreateGraphNodeForTask(raster_task, priority, graph);