cc: Make separate interface for BeginFrame ipc from OutputSurface

cc/scheduler/scheduler_unittest.cc

Index: cc/scheduler/scheduler_unittest.cc
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index 7da62676c6c4a4aeb19f5a1a5c1451bc6e5c8010..9667d7c449cd8054d92219f1430a032a081a07a6 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -47,13 +47,11 @@ void InitializeOutputSurfaceAndFirstCommit(Scheduler* scheduler,
 
 class FakeSchedulerClient : public SchedulerClient {
  public:
-  struct FakeBeginFrameSourceForFakeSchedulerClient
-      : public FakeBeginFrameSource {
-    FakeSchedulerClient* client_;
-
-    explicit FakeBeginFrameSourceForFakeSchedulerClient(
-        FakeSchedulerClient* client)
+  class FakeExternalBeginFrameSource : public BeginFrameSourceMixIn {
+   public:
+    explicit FakeExternalBeginFrameSource(FakeSchedulerClient* client)
         : client_(client) {}
+    virtual ~FakeExternalBeginFrameSource() {}
 
     void OnNeedsBeginFramesChange(bool needs_begin_frames) override {
       if (needs_begin_frames) {
@@ -63,6 +61,13 @@ class FakeSchedulerClient : public SchedulerClient {
       }
       client_->states_.push_back(client_->scheduler_->AsValue());
     }
+
+    void TestOnBeginFrame(const BeginFrameArgs& args) {
+      return CallOnBeginFrame(args);
+    }
+
+   private:
+    FakeSchedulerClient* client_;
   };
 
   class FakePowerMonitorSource : public base::PowerMonitorSource {
@@ -86,7 +91,7 @@ class FakeSchedulerClient : public SchedulerClient {
         redraw_will_happen_if_update_visible_tiles_happens_(false),
         now_src_(TestNowSource::Create()),
         task_runner_(new OrderedSimpleTaskRunner(now_src_, true)),
-        fake_frame_source_(this),
+        fake_external_begin_frame_source_(nullptr),
         fake_power_monitor_source_(new FakePowerMonitorSource),
         power_monitor_(make_scoped_ptr<base::PowerMonitorSource>(
             fake_power_monitor_source_)),
@@ -108,8 +113,21 @@ class FakeSchedulerClient : public SchedulerClient {
   }
 
   TestScheduler* CreateScheduler(const SchedulerSettings& settings) {
-    scheduler_ = TestScheduler::Create(
-        now_src_, this, settings, 0, task_runner_, &power_monitor_);
+    scoped_ptr<FakeExternalBeginFrameSource> fake_external_begin_frame_source;
+    if (settings.begin_frame_scheduling_enabled &&
+        settings.throttle_frame_production) {
+      fake_external_begin_frame_source.reset(
+          new FakeExternalBeginFrameSource(this));
+      fake_external_begin_frame_source_ =
+          fake_external_begin_frame_source.get();
+    }
+    scheduler_ = TestScheduler::Create(now_src_,
+                                       this,
+                                       settings,
+                                       0,
+                                       task_runner_,
+                                       &power_monitor_,
+                                       fake_external_begin_frame_source.Pass());
     DCHECK(scheduler_);
     return scheduler_.get();
   }
@@ -119,7 +137,10 @@ class FakeSchedulerClient : public SchedulerClient {
   void set_log_anticipated_draw_time_change(bool log) {
     log_anticipated_draw_time_change_ = log;
   }
-  bool needs_begin_frames() { return fake_frame_source_.NeedsBeginFrames(); }
+  bool needs_begin_frames() {
+    DCHECK(ExternalBeginFrame());
+    return fake_external_begin_frame_source_->NeedsBeginFrames();
+  }
   int num_draws() const { return num_draws_; }
   int num_actions_() const { return static_cast<int>(actions_.size()); }
   const char* Action(int i) const { return actions_[i]; }
@@ -132,8 +153,9 @@ class FakeSchedulerClient : public SchedulerClient {
     return scheduler_->settings().begin_frame_scheduling_enabled &&
            scheduler_->settings().throttle_frame_production;
   }
-  FakeBeginFrameSource* ExternalBeginFrameSource() override {
-    return &fake_frame_source_;
+
+  FakeExternalBeginFrameSource* ExternalBeginFrameSource() {

[dcheng, 2014/11/10 22:56:15]

Nit: since this isn't a virtual anymore, this should probably be:
FakeExternalBeginFrameSource* fake_external_begin_frame_source() const { return
fake_external_begin_frame_source_; }

or something, since it's just a simple getter.

[simonhong, 2014/11/11 12:54:15]

made this as a const.

[dcheng, 2014/11/11 18:59:01]

My main point is this should be named_with_accessor_style()

[simonhong, 2014/11/11 19:33:07]

Ah! changed to fake_external_begin_frame_source().

+    return fake_external_begin_frame_source_;
   }
 
   base::PowerMonitor* PowerMonitor() { return &power_monitor_; }
@@ -145,12 +167,11 @@ class FakeSchedulerClient : public SchedulerClient {
   void AdvanceFrame() {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames"),
                  "FakeSchedulerClient::AdvanceFrame");
-    // EXPECT_TRUE(needs_begin_frames());
     if (ExternalBeginFrame()) {
       // Creep the time forward so that any BeginFrameArgs is not equal to the
       // last one otherwise we violate the BeginFrameSource contract.
       now_src_->AdvanceNowMicroseconds(1);
-      fake_frame_source_.TestOnBeginFrame(
+      fake_external_begin_frame_source_->TestOnBeginFrame(
           CreateBeginFrameArgsForTesting(now_src_));
       EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
     }
@@ -288,7 +309,7 @@ class FakeSchedulerClient : public SchedulerClient {
   std::vector<scoped_refptr<base::debug::ConvertableToTraceFormat>> states_;
   scoped_refptr<TestNowSource> now_src_;
   scoped_refptr<OrderedSimpleTaskRunner> task_runner_;
-  FakeBeginFrameSourceForFakeSchedulerClient fake_frame_source_;
+  FakeExternalBeginFrameSource* fake_external_begin_frame_source_;
   FakePowerMonitorSource* fake_power_monitor_source_;
   base::PowerMonitor power_monitor_;
   scoped_ptr<TestScheduler> scheduler_;
@@ -327,8 +348,6 @@ void InitializeOutputSurfaceAndFirstCommit(Scheduler* scheduler,
   EXPECT_TRUE(scheduler->BeginImplFrameDeadlinePending());
   client->task_runner().RunTasksWhile(client->ImplFrameDeadlinePending(true));
   EXPECT_FALSE(scheduler->BeginImplFrameDeadlinePending());
-
-  // EXPECT_FALSE(client->needs_begin_frames());
 }
 
 TEST(SchedulerTest, InitializeOutputSurfaceDoesNotBeginImplFrame) {
@@ -1408,11 +1427,12 @@ void BeginFramesNotFromClient(bool begin_frame_scheduling_enabled,
   scheduler->SetCanDraw(true);
   InitializeOutputSurfaceAndFirstCommit(scheduler, &client);
 
+  DCHECK(!client.ExternalBeginFrameSource());
+
   // SetNeedsCommit should begin the frame on the next BeginImplFrame
   // without calling SetNeedsBeginFrame.
   client.Reset();
   scheduler->SetNeedsCommit();
-  EXPECT_FALSE(client.needs_begin_frames());
   EXPECT_NO_ACTION(client);
   client.Reset();
 
@@ -1422,21 +1442,18 @@ void BeginFramesNotFromClient(bool begin_frame_scheduling_enabled,
   EXPECT_ACTION("WillBeginImplFrame", client, 0, 2);
   EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client, 1, 2);
   EXPECT_TRUE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // If we don't swap on the deadline, we wait for the next BeginFrame.
   client.task_runner().RunPendingTasks();  // Run posted deadline.
   EXPECT_NO_ACTION(client);
   EXPECT_FALSE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // NotifyReadyToCommit should trigger the commit.
   scheduler->NotifyBeginMainFrameStarted();
   scheduler->NotifyReadyToCommit();
   EXPECT_SINGLE_ACTION("ScheduledActionCommit", client);
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // BeginImplFrame should prepare the draw.
@@ -1444,14 +1461,12 @@ void BeginFramesNotFromClient(bool begin_frame_scheduling_enabled,
   EXPECT_ACTION("WillBeginImplFrame", client, 0, 2);
   EXPECT_ACTION("ScheduledActionAnimate", client, 1, 2);
   EXPECT_TRUE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // BeginImplFrame deadline should draw.
   client.task_runner().RunTasksWhile(client.ImplFrameDeadlinePending(true));
   EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 0, 1);
   EXPECT_FALSE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // The following BeginImplFrame deadline should SetNeedsBeginFrame(false)
@@ -1465,7 +1480,6 @@ void BeginFramesNotFromClient(bool begin_frame_scheduling_enabled,
   // when the BeginFrame is no longer needed.
   client.task_runner().RunPendingTasks();  // Run posted deadline.
   EXPECT_NO_ACTION(client);
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 }
 
@@ -1503,6 +1517,8 @@ void BeginFramesNotFromClient_SwapThrottled(bool begin_frame_scheduling_enabled,
   scheduler->SetCanDraw(true);
   InitializeOutputSurfaceAndFirstCommit(scheduler, &client);
 
+  DCHECK(!client.ExternalBeginFrameSource());
+
   // To test swap ack throttling, this test disables automatic swap acks.
   scheduler->SetMaxSwapsPending(1);
   client.SetAutomaticSwapAck(false);
@@ -1510,7 +1526,6 @@ void BeginFramesNotFromClient_SwapThrottled(bool begin_frame_scheduling_enabled,
   // SetNeedsCommit should begin the frame on the next BeginImplFrame.
   client.Reset();
   scheduler->SetNeedsCommit();
-  EXPECT_FALSE(client.needs_begin_frames());
   EXPECT_NO_ACTION(client);
   client.Reset();
 
@@ -1519,14 +1534,12 @@ void BeginFramesNotFromClient_SwapThrottled(bool begin_frame_scheduling_enabled,
   EXPECT_ACTION("WillBeginImplFrame", client, 0, 2);
   EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client, 1, 2);
   EXPECT_TRUE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // NotifyReadyToCommit should trigger the pending commit and draw.
   scheduler->NotifyBeginMainFrameStarted();
   scheduler->NotifyReadyToCommit();
   EXPECT_SINGLE_ACTION("ScheduledActionCommit", client);
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // Swapping will put us into a swap throttled state.
@@ -1534,7 +1547,6 @@ void BeginFramesNotFromClient_SwapThrottled(bool begin_frame_scheduling_enabled,
   EXPECT_ACTION("ScheduledActionAnimate", client, 0, 2);
   EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 1, 2);
   EXPECT_FALSE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // While swap throttled, BeginFrames should trigger BeginImplFrames,
@@ -1543,14 +1555,12 @@ void BeginFramesNotFromClient_SwapThrottled(bool begin_frame_scheduling_enabled,
   client.task_runner().RunPendingTasks();  // Run posted BeginFrame.
   EXPECT_ACTION("WillBeginImplFrame", client, 0, 1);
   EXPECT_TRUE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // Take us out of a swap throttled state.
   scheduler->DidSwapBuffersComplete();
   EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client, 0, 1);
   EXPECT_TRUE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 
   // BeginImplFrame deadline should draw.
@@ -1559,7 +1569,6 @@ void BeginFramesNotFromClient_SwapThrottled(bool begin_frame_scheduling_enabled,
   EXPECT_ACTION("ScheduledActionAnimate", client, 0, 2);
   EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client, 1, 2);
   EXPECT_FALSE(scheduler->BeginImplFrameDeadlinePending());
-  EXPECT_FALSE(client.needs_begin_frames());
   client.Reset();
 }