cc: Generalize raster task notifications

cc/resources/rasterizer.h

Index: cc/resources/rasterizer.h
diff --git a/cc/resources/rasterizer.h b/cc/resources/rasterizer.h
index c718eb9be9e3515aed6fac57856b1df564af63fc..07aef036f104c02ab52bc8d2707625016ed04d36 100644
--- a/cc/resources/rasterizer.h
+++ b/cc/resources/rasterizer.h
@@ -5,12 +5,19 @@
 #ifndef CC_RESOURCES_RASTERIZER_H_
 #define CC_RESOURCES_RASTERIZER_H_
 
+#include <bitset>
 #include <vector>
 
 #include "base/callback.h"
 #include "cc/resources/resource_format.h"
 #include "cc/resources/task_graph_runner.h"
 
+namespace base {
+namespace debug {
+class TraceEventSyntheticDelay;
+}
+}
+
 namespace cc {
 class ImageDecodeTask;
 class RasterTask;
@@ -85,11 +92,16 @@ class CC_EXPORT RasterTask : public RasterizerTask {
   ImageDecodeTask::Vector dependencies_;
 };
 
+static const size_t kMaxTaskSet = 2;

[reveman, 2014/09/05 08:38:59]

This is really number of task sets. Either set it to 1 or change the name to
something like kNumberOfTaskSet.

[ernstm, 2014/09/05 21:36:11]

Done.

+typedef size_t TaskSet;
+
 class CC_EXPORT RasterizerClient {
  public:
-  virtual bool ShouldForceTasksRequiredForActivationToComplete() const = 0;
   virtual void DidFinishRunningTasks() = 0;
-  virtual void DidFinishRunningTasksRequiredForActivation() = 0;
+  virtual void DidFinishRunningTaskSet(TaskSet task_set) = 0;
+  virtual bool ShouldForceTaskSetToComplete(TaskSet task_set) const = 0;
+  virtual base::debug::TraceEventSyntheticDelay* SyntheticDelayForTaskSet(
+      TaskSet task_set) const = 0;

[reveman, 2014/09/05 08:38:59]

Not a fan of how much influence this synthetic delay debugging stuff has on the
design. Is there any way we could hide all this in one place and not be part of
the RasterizeClient API.

[ernstm, 2014/09/05 21:36:11]

I don't think so, because now only the TileManager knows the semantics of the
TaskSets and you need that knowledge to determine the delay.

 
  protected:
   virtual ~RasterizerClient() {}
@@ -97,6 +109,8 @@ class CC_EXPORT RasterizerClient {
 
 struct CC_EXPORT RasterTaskQueue {
   struct CC_EXPORT Item {
+    typedef std::bitset<kMaxTaskSet> TaskSetCollection;

[ernstm, 2014/08/29 23:32:10]

should we call this TaskSetFlags, because it is not just used to indicate
membership in a set, but also to remember for which tasks we should send
notifications, etc. in the worker pools? Because it is used not only on the
Items, should we move it up into cc namesapce?

[reveman, 2014/09/05 08:38:59]

Moving it up next to the TaskSet typedef sgtm. I would keep the current name
though. TaskSetCollection name still makes sense to me when used for other
things.

[ernstm, 2014/09/05 21:36:11]

Done.

+
     class TaskComparator {
      public:
       explicit TaskComparator(const RasterTask* task) : task_(task) {}
@@ -109,25 +123,36 @@ struct CC_EXPORT RasterTaskQueue {
 
     typedef std::vector<Item> Vector;
 
-    Item(RasterTask* task, bool required_for_activation);
+    Item(RasterTask* task, const TaskSetCollection& task_sets);
     ~Item();
 
-    static bool IsRequiredForActivation(const Item& item) {
-      return item.required_for_activation;
-    }
-
     RasterTask* task;
-    bool required_for_activation;
+    TaskSetCollection task_sets;
+  };
+
+  class TaskSetSizes {

[ernstm, 2014/08/29 23:32:10]

Should we move this into cc namespace, because it is used in various places?

+   public:
+    TaskSetSizes();
+    Item::TaskSetCollection ToTaskSetCollection() const;
+    size_t& operator[](TaskSet task_set);
+    const size_t& operator[](TaskSet task_set) const;
+    bool operator==(const TaskSetSizes& other) const;
+    void operator+=(const Item::TaskSetCollection& task_set_collection);
+    void operator-=(const Item::TaskSetCollection& task_set_collection);
+
+   private:
+    size_t sizes_[kMaxTaskSet];
   };
 
   RasterTaskQueue();
   ~RasterTaskQueue();
 
+  bool VerifyTaskSetSizes() const;

[reveman, 2014/09/05 08:38:59]

I'd prefer if this was not part of the API as it's just for internal debug
checks.

[ernstm, 2014/09/05 21:36:11]

Done. We don't need it anymore, if TaskSetSizes is removed from the queue.

   void Swap(RasterTaskQueue* other);
   void Reset();
 
   Item::Vector items;
-  size_t required_for_activation_count;
+  TaskSetSizes task_set_sizes;

[reveman, 2014/09/05 08:38:59]

I think we can remove this and make it internal to RasterWorkerPool
implementations instead. They all iterate over this in ScheduleTasks so they can
compute it there instead and not having all this here would be much cleaner.

[ernstm, 2014/09/05 21:36:11]

Done. Although I'm not sure if this is the best trade-off. We now have to
collect that information in ScheduleTasks _before_ we iterate over all the
items, which means we iterate the queue twice. This is necessary, because we
need to create the RasterFinishedTask before iterating the queue, and we need to
know the sizes of the task sets at creation of the RasterFinishedTask to
determine if a synthetic delay needs to be added. With the TaskSetSize on the
queue, we were able to compute the size of the sets as a byproduct of adding
items.

 };
 
 // This interface can be used to schedule and run raster tasks. The client will