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..4ca4a58393846b3ef58b4557b0e87f65631277be 100644
--- a/cc/test/scheduler_test_common.h
+++ b/cc/test/scheduler_test_common.h
@@ -9,6 +9,8 @@
 #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 +48,64 @@ class FakeDelayBasedTimeSource : public DelayBasedTimeSource {
   base::TimeTicks now_;
 };
 
+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;
+
+    return make_scoped_ptr(new TestScheduler(
+        client, scheduler_settings, layer_tree_host_id, test_task_runner));
+  }
+
+  virtual ~TestScheduler() {}
+
+  bool IsBeginRetroFrameArgsEmpty() const {
+    return begin_retro_frame_args_.empty();
+  }
+
+  bool IsSyntheticBeginFrameSourceActive() const {
+    return synthetic_begin_frame_source_->IsActive();
+  }
+
+  /*
+    virtual void SetupSyntheticBeginFrames() {

[Sami, 2014/07/11 17:22:46]

Not needed?

[brianderson, 2014/07/14 23:42:31]

And if it's not needed can the virtual in the base class be removed again?

[mithro-old, 2014/07/16 14:18:12]

Acknowledged.

[mithro-old, 2014/07/16 14:18:12]

Done.

+      DCHECK(!synthetic_begin_frame_source_);
+      synthetic_begin_frame_source_.reset(
+          new SyntheticBeginFrameSource(this, test_task_runner_.get()));
+    }
+  */
+
+  void SetNow(base::TimeTicks time) { test_task_runner_->SetNow(time); }
+
+  virtual base::TimeTicks Now() const OVERRIDE;
+
+  void RunPendingTasks(bool advance_now) {

[Sami, 2014/07/11 17:22:46]

AFAICT we never call this with |false| -- does it really need to be
parameterized? In any case I'd either suggest an enum or a separate entrypoint
to make the call site more readable.

[brianderson, 2014/07/14 23:42:31]

+1 to a separate entry point or enum.

[mithro-old, 2014/07/16 14:18:12]

Done.

[mithro-old, 2014/07/16 14:18:12]

I refactored this code slightly to be more inline with the original source. This
extra level of indirection was left over from my first attempt where Now was
managed by the scheduler rather than the task runner.

I was sure there were 2 tests which needed the non-advancing behaviour but they
seem to have disappeared (or maybe I imagined them in the first place).

+    test_task_runner_->SetAdvanceNow(advance_now);
+    test_task_runner_->RunPendingTasks();
+  }
+
+  base::TimeDelta NextPendingTaskDelay() {
+    return test_task_runner_->NextPendingTaskDelay();
+  }
+
+ private:
+  TestScheduler(SchedulerClient* client,
+                const SchedulerSettings& scheduler_settings,
+                int layer_tree_host_id,
+                const scoped_refptr<OrderedSimpleTaskRunner>& test_task_runner)
+      : Scheduler(client,
+                  scheduler_settings,
+                  layer_tree_host_id,
+                  test_task_runner),
+        test_task_runner_(test_task_runner.get()) {}
+
+  OrderedSimpleTaskRunner* test_task_runner_;
+};
+
 }  // namespace cc
 
 #endif  // CC_TEST_SCHEDULER_TEST_COMMON_H_