Clean up task category assignment

cc/tiles/tile_manager.cc

Index: cc/tiles/tile_manager.cc
diff --git a/cc/tiles/tile_manager.cc b/cc/tiles/tile_manager.cc
index 9d04f5aae4534ec54406cbcd947a524c4393f48a..62d09d901d823140059933cad7ebecd84e19e21f 100644
--- a/cc/tiles/tile_manager.cc
+++ b/cc/tiles/tile_manager.cc
@@ -58,8 +58,9 @@ class RasterTaskImpl : public TileTask {
                  uint64_t resource_content_id,
                  int source_frame_number,
                  const base::Callback<void(bool)>& reply,
-                 TileTask::Vector* dependencies)
-      : TileTask(true, dependencies),
+                 TileTask::Vector* dependencies,
+                 bool supports_concurrent_execution)
+      : TileTask(supports_concurrent_execution, dependencies),
         resource_(resource),
         raster_source_(std::move(raster_source)),
         content_rect_(content_rect),
@@ -131,6 +132,30 @@ class RasterTaskImpl : public TileTask {
   DISALLOW_COPY_AND_ASSIGN(RasterTaskImpl);
 };
 
+TaskCategory TaskCategoryForTileTask(TileTask* task, bool high_priority) {

[prashant.n, 2016/04/23 04:55:19]

May be replace high_priority with is_foreground or similar.

[ericrk, 2016/05/02 23:04:37]

Done.

+  if (!task->supports_concurrent_execution()) {
+    return TASK_CATEGORY_NONCONCURRENT_FOREGROUND;
+  }

[vmpstr, 2016/04/25 18:23:34]

nit: remove braces please

[ericrk, 2016/05/02 23:04:37]

Done.

+  if (high_priority) {
+    return TASK_CATEGORY_FOREGROUND;
+  }
+  return TASK_CATEGORY_BACKGROUND;
+}
+
+bool IsHighPriorityCategory(uint16_t category) {

[prashant.n, 2016/04/23 04:55:19]

Should we use simple function name IsForeground() for better readability?

[ericrk, 2016/05/02 23:04:37]

Using IsForegroundCategory - I agree that HighPriority is misleading.

+  TaskCategory enum_category = static_cast<TaskCategory>(category);
+  switch (enum_category) {
+    case TASK_CATEGORY_NONCONCURRENT_FOREGROUND:
+    case TASK_CATEGORY_FOREGROUND:
+      return true;
+    case TASK_CATEGORY_BACKGROUND:
+      return false;
+  }
+
+  DCHECK(false);
+  return false;
+}
+
 // Task priorities that make sure that the task set done tasks run before any
 // other remaining tasks.
 const size_t kRequiredForActivationDoneTaskPriority = 1u;
@@ -156,21 +181,19 @@ void InsertNodeForTask(TaskGraph* graph,
 
 void InsertNodeForDecodeTask(TaskGraph* graph,
                              TileTask* task,
-                             uint16_t category,
+                             bool high_prioirty,

[prashant.n, 2016/04/23 04:55:19]

May be replace high_priority with is_foreground or similar.

[ericrk, 2016/05/02 23:04:37]

Done.

                              uint16_t priority) {
   uint32_t dependency_count = 0u;
   if (task->dependencies().size()) {
     DCHECK_EQ(task->dependencies().size(), 1u);
     auto* dependency = task->dependencies()[0].get();
     if (!dependency->HasCompleted()) {
-      InsertNodeForDecodeTask(graph, dependency, category, priority);
+      InsertNodeForDecodeTask(graph, dependency, high_prioirty, priority);
       graph->edges.push_back(TaskGraph::Edge(dependency, task));
       dependency_count = 1u;
     }
   }
-  InsertNodeForTask(graph, task, task->supports_concurrent_execution()
-                                     ? category
-                                     : TASK_CATEGORY_NONCONCURRENT_FOREGROUND,
+  InsertNodeForTask(graph, task, TaskCategoryForTileTask(task, high_prioirty),
                     priority, dependency_count);
 }
 
@@ -178,27 +201,9 @@ void InsertNodesForRasterTask(TaskGraph* graph,
                               TileTask* raster_task,
                               const TileTask::Vector& decode_tasks,
                               size_t priority,

[prashant.n, 2016/04/23 04:55:19]

Replace this with task_priority.

[ericrk, 2016/05/02 23:04:37]

Should be less confusing now that we only have one "priority".

-                              bool use_gpu_rasterization,
                               bool high_priority) {

[prashant.n, 2016/04/23 04:55:19]

Replace this with tile_priority for better readability.

[ericrk, 2016/05/02 23:04:37]

Done.

   size_t dependencies = 0u;
 
-  // Determine the TaskCategory for raster tasks - if a task uses GPU, it
-  // cannot run concurrently and is assigned
-  // TASK_CATEGORY_NONCONCURRENT_FOREGROUND, regardless of its priority.
-  // Otherwise its category is based on its priority.
-  TaskCategory raster_task_category;
-  if (use_gpu_rasterization) {
-    raster_task_category = TASK_CATEGORY_NONCONCURRENT_FOREGROUND;
-  } else {
-    raster_task_category =
-        high_priority ? TASK_CATEGORY_FOREGROUND : TASK_CATEGORY_BACKGROUND;
-  }
-
-  // Determine the TaskCategory for decode tasks. This category is based on
-  // the priority of the raster task which depends on it.
-  TaskCategory decode_task_category =
-      high_priority ? TASK_CATEGORY_FOREGROUND : TASK_CATEGORY_BACKGROUND;
-
   // Insert image decode tasks.
   for (TileTask::Vector::const_iterator it = decode_tasks.begin();
        it != decode_tasks.end(); ++it) {
@@ -222,21 +227,23 @@ void InsertNodesForRasterTask(TaskGraph* graph,
     // the current task.
     // TODO(ericrk): Task iterators should be updated to avoid this.
     // crbug.com/594851
+    // TODO(ericrk): This should handle dependencies recursively.
+    // crbug.com/605234
     if (decode_it != graph->nodes.end() && high_priority &&
-        decode_it->category != decode_task_category) {
-      decode_it->category = decode_task_category;
+        !IsHighPriorityCategory(decode_it->category)) {
+      decode_it->category = TASK_CATEGORY_FOREGROUND;
     }
 
     if (decode_it == graph->nodes.end()) {
-      InsertNodeForDecodeTask(graph, decode_task, decode_task_category,
-                              priority);
+      InsertNodeForDecodeTask(graph, decode_task, high_priority, priority);
     }
 
     graph->edges.push_back(TaskGraph::Edge(decode_task, raster_task));
   }
 
-  InsertNodeForTask(graph, raster_task, raster_task_category, priority,
-                    dependencies);
+  InsertNodeForTask(graph, raster_task,
+                    TaskCategoryForTileTask(raster_task, high_priority),
+                    priority, dependencies);
 }
 
 class TaskSetFinishedTaskImpl : public TileTask {
@@ -805,7 +812,7 @@ void TileManager::ScheduleTasks(
         tile->required_for_draw() || tile->required_for_activation() ||
         prioritized_tile.priority().priority_bin == TilePriority::NOW;
     InsertNodesForRasterTask(&graph_, task, task->dependencies(), priority++,
-                             use_gpu_rasterization_, high_priority);
+                             high_priority);
   }
 
   // Insert nodes for our task completion tasks. We enqueue these using
@@ -900,6 +907,7 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
       it = images.erase(it);
   }
 
+  bool supports_concurrent_execution = !use_gpu_rasterization_;
   return make_scoped_refptr(new RasterTaskImpl(
       resource, prioritized_tile.raster_source(), tile->content_rect(),
       tile->invalidated_content_rect(), tile->contents_scale(),
@@ -909,7 +917,7 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
       tile->source_frame_number(),
       base::Bind(&TileManager::OnRasterTaskCompleted, base::Unretained(this),
                  tile->id(), resource),
-      &decode_tasks));
+      &decode_tasks, supports_concurrent_execution));
 }
 
 void TileManager::OnRasterTaskCompleted(