Fixing and enhancing OrderedSimpleTaskRunner to allow 100% deterministic tests.

cc/test/scheduler_test_common.h

Index: cc/test/scheduler_test_common.h
diff --git a/cc/test/scheduler_test_common.h b/cc/test/scheduler_test_common.h
index 596358a205877ad26517cbcd54182ab602348254..7d5002bdc3d7285bb2e81f5a23e8cde3cb719486 100644
--- a/cc/test/scheduler_test_common.h
+++ b/cc/test/scheduler_test_common.h
@@ -5,10 +5,14 @@
 #ifndef CC_TEST_SCHEDULER_TEST_COMMON_H_
 #define CC_TEST_SCHEDULER_TEST_COMMON_H_
 
+#include <string>
+
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
 #include "cc/scheduler/delay_based_time_source.h"
+#include "cc/scheduler/scheduler.h"
+#include "cc/test/ordered_simple_task_runner.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace cc {
@@ -46,6 +50,77 @@ class FakeDelayBasedTimeSource : public DelayBasedTimeSource {
   base::TimeTicks now_;
 };
 
+class TestDelayBasedTimeSource : public DelayBasedTimeSource {
+ public:
+  static scoped_refptr<TestDelayBasedTimeSource> Create(
+      base::TimeDelta interval,
+      OrderedSimpleTaskRunner* task_runner) {
+    return make_scoped_refptr(
+        new TestDelayBasedTimeSource(interval, task_runner));
+  }
+
+  // Overridden from DelayBasedTimeSource
+  virtual base::TimeTicks Now() const OVERRIDE;

[brianderson, 2014/08/20 20:04:31]

Please make this protected, not public. The DelayBasedTimeSource should make its
Now() protected too.

[mithro-old, 2014/08/21 17:39:19]

Done.

+
+  // Extra test helper functionality
+  void SetNow(base::TimeTicks time);
+  OrderedSimpleTaskRunner& task_runner() { return *test_task_runner_; }

[brianderson, 2014/08/20 20:04:31]

Please remove SetNow and task_runner from this class.

[mithro-old, 2014/08/21 17:39:19]

Done.

+
+ protected:
+  TestDelayBasedTimeSource(base::TimeDelta interval,
+                           OrderedSimpleTaskRunner* task_runner)
+      : DelayBasedTimeSource(interval, task_runner),
+        test_task_runner_(task_runner) {}
+  virtual ~TestDelayBasedTimeSource() {}
+
+  virtual std::string TypeString() const OVERRIDE;

[Sami, 2014/08/19 18:45:04]

Is this used anywhere?

[mithro-old, 2014/08/21 17:39:19]

Yes, in the parent's AsValue function. It means traces include
TestDelayBasedTimeSource rather than DelayBasedTimeSource which is really useful
for making sure that the test is using the right time source :).

+
+  OrderedSimpleTaskRunner* test_task_runner_;
+};
+
+class TestScheduler : public Scheduler {
+ public:
+  static scoped_ptr<TestScheduler> Create(
+      SchedulerClient* client,
+      const SchedulerSettings& scheduler_settings,
+      int layer_tree_host_id) {
+    scoped_refptr<OrderedSimpleTaskRunner> test_task_runner =
+        new OrderedSimpleTaskRunner;
+
+    // A bunch of tests require Now() to be > BeginFrameArgs::DefaultInterval()

[Sami, 2014/08/19 18:45:04]

Oh, really? That seems broken :)

[brianderson, 2014/08/20 20:04:31]

Yes, that seems broken. Is it because some operations cause the timeline to go
negative and we don't handle that properly? Or is there another reason?

[mithro-old, 2014/08/21 17:39:19]

There are two things which go wrong;
 * base::TimeTicks with a value of zero is treated specially by a huge amount of
code (it's assume to be null / invalid value).
 * As Brain suggested a bunch of stuff gets a bit grumpy when the time value
goes negative. There are a couple of DCHECKS for negative values and one place
where negative values are replace with the invalid value.

+    test_task_runner->SetNow(base::TimeTicks() +
+                             base::TimeDelta::FromMilliseconds(100));
+
+    return make_scoped_ptr(new TestScheduler(
+        client, scheduler_settings, layer_tree_host_id, test_task_runner));
+  }
+
+  virtual ~TestScheduler() {}
+
+  // Overridden from Scheduler.
+  virtual base::TimeTicks Now() const OVERRIDE;

[brianderson, 2014/08/20 20:04:31]

protected, not public.

[mithro-old, 2014/08/21 17:39:19]

Done.

+
+  // Extra test helper functionality
+  bool IsBeginRetroFrameArgsEmpty() const {
+    return begin_retro_frame_args_.empty();
+  }
+
+  bool IsSyntheticBeginFrameSourceActive() const {
+    return synthetic_begin_frame_source_->IsActive();
+  }
+
+  void SetNow(base::TimeTicks time);

[brianderson, 2014/08/20 20:04:31]

SetNow should be removed.

[mithro-old, 2014/08/21 17:39:19]

Done.

+  OrderedSimpleTaskRunner& task_runner() { return *test_task_runner_; }
+
+ private:
+  TestScheduler(SchedulerClient* client,
+                const SchedulerSettings& scheduler_settings,
+                int layer_tree_host_id,
+                const scoped_refptr<OrderedSimpleTaskRunner>& test_task_runner);
+
+  OrderedSimpleTaskRunner* test_task_runner_;
+};
+
 }  // namespace cc
 
 #endif  // CC_TEST_SCHEDULER_TEST_COMMON_H_