Clean up task category assignment

cc/tiles/tile_manager.cc

 // Copyright 2012 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/tiles/tile_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 40 matching lines @@
                  const RasterSource::PlaybackSettings& playback_settings,
                  TileResolution tile_resolution,
                  int layer_id,
                  uint64_t source_prepare_tiles_id,
                  const void* tile,
                  uint64_t new_content_id,
                  uint64_t previous_content_id,
                  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),
         invalid_content_rect_(invalid_content_rect),
         contents_scale_(contents_scale),
         playback_settings_(playback_settings),
         tile_resolution_(tile_resolution),
         layer_id_(layer_id),
         source_prepare_tiles_id_(source_prepare_tiles_id),
         tile_(tile),
@@ skipping 51 matching lines @@
   uint64_t new_content_id_;
   uint64_t previous_content_id_;
   uint64_t resource_content_id_;
   int source_frame_number_;
   const base::Callback<void(bool)> reply_;
   std::unique_ptr<RasterBuffer> raster_buffer_;
 
   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;
 const size_t kRequiredForDrawDoneTaskPriority = 2u;
 const size_t kAllDoneTaskPriority = 3u;
 
 // For correctness, |kTileTaskPriorityBase| must be greater than
 // all task set done task priorities.
 size_t kTileTaskPriorityBase = 10u;
 
 void InsertNodeForTask(TaskGraph* graph,
                        TileTask* task,
                        uint16_t category,
                        uint16_t priority,
                        size_t dependencies) {
   DCHECK(std::find_if(graph->nodes.begin(), graph->nodes.end(),
                       [task](const TaskGraph::Node& node) {
                         return node.task == task;
                       }) == graph->nodes.end());
   graph->nodes.push_back(
       TaskGraph::Node(task, category, priority, dependencies));
 }
 
 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);
 }
 
 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) {
     TileTask* decode_task = it->get();
 
     // Skip if already decoded.
     if (decode_task->HasCompleted())
       continue;
 
     dependencies++;
 
     // Add decode task if it doesn't already exists in graph.
     TaskGraph::Node::Vector::iterator decode_it =
         std::find_if(graph->nodes.begin(), graph->nodes.end(),
                      [decode_task](const TaskGraph::Node& node) {
                        return node.task == decode_task;
                      });
 
     // In rare circumstances, a low priority task may come in before a high
     // priority task. In these cases, upgrade any low-priority dependencies of
     // 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 {
  public:
   explicit TaskSetFinishedTaskImpl(
       base::SequencedTaskRunner* task_runner,
       const base::Closure& on_task_set_finished_callback)
       : TileTask(true),
         task_runner_(task_runner),
         on_task_set_finished_callback_(on_task_set_finished_callback) {}
@@ skipping 548 matching lines @@
     all_count++;
     graph_.edges.push_back(TaskGraph::Edge(task, all_done_task.get()));
 
     // A tile is high priority if it is either blocking future compositing
     // (required for draw or required for activation), or if it has a priority
     // bin of NOW for another reason (low resolution tiles).
     bool high_priority =
         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
   // NONCONCURRENT_FOREGROUND priority as this is the highest prioirty and we'd
   // like to run these tasks as soon as possible.
   InsertNodeForTask(&graph_, required_for_activation_done_task.get(),
                     TASK_CATEGORY_NONCONCURRENT_FOREGROUND,
                     kRequiredForActivationDoneTaskPriority,
                     required_for_activate_count);
   InsertNodeForTask(&graph_, required_for_draw_done_task.get(),
@@ skipping 74 matching lines @@
             *it, prepare_tiles_count_, &task);
     if (task)
       decode_tasks.push_back(task);
 
     if (need_to_unref_when_finished)
       ++it;
     else
       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(),
       playback_settings, prioritized_tile.priority().resolution,
       tile->layer_id(), prepare_tiles_count_, static_cast<const void*>(tile),
       tile->id(), tile->invalidated_id(), resource_content_id,
       tile->source_frame_number(),
       base::Bind(&TileManager::OnRasterTaskCompleted, base::Unretained(this),
                  tile->id(), resource),
-      &decode_tasks));
+      &decode_tasks, supports_concurrent_execution));
 }
 
 void TileManager::OnRasterTaskCompleted(
     Tile::Id tile_id,
     Resource* resource,
     bool was_canceled) {
   DCHECK(tiles_.find(tile_id) != tiles_.end());
 
   Tile* tile = tiles_[tile_id];
   TileDrawInfo& draw_info = tile->draw_info();
@@ skipping 316 matching lines @@
 void TileManager::Signals::reset() {
   ready_to_activate = false;
   did_notify_ready_to_activate = false;
   ready_to_draw = false;
   did_notify_ready_to_draw = false;
   all_tile_tasks_completed = false;
   did_notify_all_tile_tasks_completed = false;
 }
 
 }  // namespace cc