cc: Simplify task and its derived classes.

cc/raster/dependency_task.h

Index: cc/raster/dependency_task.h
diff --git a/cc/raster/dependency_task.h b/cc/raster/dependency_task.h
new file mode 100644
index 0000000000000000000000000000000000000000..698bcf33f6d6253db1a3ecb6c1e965cc28c287df
--- /dev/null
+++ b/cc/raster/dependency_task.h
@@ -0,0 +1,86 @@
+// Copyright 2016 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_RASTER_DEPENDENCY_TASK_H_
+#define CC_RASTER_DEPENDENCY_TASK_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <algorithm>
+#include <map>

[ericrk, 2016/04/01 21:34:31]

nit: map/algorithm are not used

+#include <vector>
+
+#include "base/logging.h"

[ericrk, 2016/04/01 21:34:30]

nit: logging is not used.

+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "cc/base/cc_export.h"
+
+namespace cc {
+
+class Resource;
+class RasterBuffer;
+
+class CC_EXPORT DependencyTaskClient {

[ericrk, 2016/04/01 21:34:30]

This does a lot of things other than dependencies, and the dependencies are
optional. Why not just call this Task?

[prashant.n, 2016/04/01 23:48:05]

I called this as DependencyTask to differetiate from Task that it is run by task
runner who is capable of handling dependecies, e.g. TaskGraphRunner (which is to
be renamed as DependencyTaskRunner.)

Graph is iternal concept to handle dependecies. We can abstract it by may be
having following iterfaces

BeginSchdeduling()
AddTask(task, dependency)
AddTask(task, dependency)
.
.
.
EndScheduling()

DependencyTask means either it can have dependencies or not. Later if possible
we can make this more generic to be moved to base/.

+ public:
+  virtual scoped_ptr<RasterBuffer> AcquireBufferForRaster(

[prashant.n, 2016/04/01 14:23:39]

These methods will be made generic in subsequent patch and will be renamed as
OnScheduled(DependencyTask* task) and OnCompleted(DependencyTask* task)

[ericrk, 2016/04/01 21:34:30]

While I agree that dependencies are a generic concept - I think this
DependencyTaskClient isn't really generic - only raster tasks actually make use
of this functionality - not sure we want to make this generic.

I think you could still do something like:
using RasterTask = DependencyTask;
class RasterTaskClient { ...

to make the division clear?

Unless you think we need this functionality for other kinds of dependency tasks
in the future... but I don't think so.

[prashant.n, 2016/04/01 23:48:05]

Yes, I agree with you. Scheduling and Completing the task is generic concept.
May be we can do something like this -

virtual void ScheduleOnOriginThread() = 0;
virtual void CompleteOnOriginThread() = 0;

Removing DependencyTaskClient as argument.

And subclass RasterTaskImpl : DependencyTask
can take one more argument as RasterTaskClient and do the needed alloc, dealloc
in above methods. And other subclasses can take care their own handling.

WDYT?

+      const Resource* resource,
+      uint64_t resource_content_id,
+      uint64_t previous_content_id) = 0;
+  virtual void ReleaseBufferForRaster(scoped_ptr<RasterBuffer> buffer) = 0;
+
+ protected:
+  virtual ~DependencyTaskClient() {}
+};
+
+// A task with or without dependencies which can be run by a TaskGraphRunner. To
+// run a DependencyTask, it should be inserted into a TaskGraph, which can then
+// be scheduled on the TaskGraphRunner.
+class CC_EXPORT DependencyTask
+    : public base::RefCountedThreadSafe<DependencyTask> {
+ public:
+  typedef std::vector<scoped_refptr<DependencyTask>> Vector;
+
+  const DependencyTask::Vector& dependencies() const { return dependencies_; }
+
+  // Subclasses should implement these methods. RunOnWorkerThread may be called
+  // on any thread (origin or worker), ScheduleOnOriginThread and
+  // CompleteOnOriginThread must be called on origin thread. The subclasses are
+  // responsible for locking and thread safety.
+  virtual void RunOnWorkerThread() = 0;
+  virtual void ScheduleOnOriginThread(DependencyTaskClient* client) = 0;
+  virtual void CompleteOnOriginThread(DependencyTaskClient* client) = 0;
+
+  void WillRun();
+  void DidRun();
+  bool HasFinishedRunning() const;
+
+  void WillSchedule();
+  void DidSchedule();
+  bool HasBeenScheduled() const;
+
+  void WillComplete();
+  void DidComplete();
+  bool HasCompleted() const;
+
+ protected:
+  friend class base::RefCountedThreadSafe<DependencyTask>;
+
+  // For creating task without dependencies.
+  DependencyTask();
+  // For creating task with dependencies.
+  explicit DependencyTask(DependencyTask::Vector* dependencies);

[ericrk, 2016/04/01 21:34:30]

maybe take by value to allow us to std::move the vector in here?

+  virtual ~DependencyTask();
+
+  bool will_run_;
+  bool did_run_;
+  bool did_schedule_;
+  bool did_complete_;
+
+  DependencyTask::Vector dependencies_;
+};
+
+}  // namespace cc
+
+#endif  // CC_RASTER_DEPENDENCY_TASK_H_