cc: Generalize raster task notifications

cc/resources/rasterizer.h

 // Copyright 2014 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.
 
 #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;
 class Resource;
 class RasterBuffer;
 
 class CC_EXPORT RasterizerTaskClient {
  public:
   virtual RasterBuffer* AcquireBufferForRaster(RasterTask* task) = 0;
   virtual void ReleaseBufferForRaster(RasterTask* task) = 0;
@@ skipping 54 matching lines @@
 
  protected:
   RasterTask(const Resource* resource, ImageDecodeTask::Vector* dependencies);
   virtual ~RasterTask();
 
  private:
   const Resource* resource_;
   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() {}
 };
 
 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) {}
 
       bool operator()(const Item& item) const { return item.task == task_; }
 
      private:
       const RasterTask* task_;
     };
 
     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;
+    TaskSetCollection task_sets;
+  };
 
-    RasterTask* task;
-    bool required_for_activation;
+  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
 // be notified asynchronously when the set of tasks marked as "required for
 // activation" have finished running and when all scheduled tasks have finished
 // running. The client can call CheckForCompletedTasks() at any time to dispatch
 // pending completion callbacks for all tasks that have finished running.
 class CC_EXPORT Rasterizer {
  public:
   // Set the client instance to be notified when finished running tasks.
@@ skipping 14 matching lines @@
   // Check for completed tasks and dispatch reply callbacks.
   virtual void CheckForCompletedTasks() = 0;
 
  protected:
   virtual ~Rasterizer() {}
 };
 
 }  // namespace cc
 
 #endif  // CC_RESOURCES_RASTERIZER_H_