Refactoring the way begin frame sources inside scheduler work.

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 88c8c74f701d2f4aa0a8980cff8b01f2a21335ea..2e23e7a507127157ab28bc182251eb2ee13b5172 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -11,74 +11,21 @@
 #include "cc/debug/devtools_instrumentation.h"
 #include "cc/debug/traced_value.h"
 #include "cc/scheduler/delay_based_time_source.h"
+#include "cc/scheduler/frame_source.h"
 #include "ui/gfx/frame_time.h"
 
 namespace cc {
 
-class SyntheticBeginFrameSource : public TimeSourceClient {
- public:
-  SyntheticBeginFrameSource(Scheduler* scheduler,
-                            base::SingleThreadTaskRunner* task_runner)
-      : scheduler_(scheduler) {
-    if (gfx::FrameTime::TimestampsAreHighRes()) {
-      time_source_ = DelayBasedTimeSourceHighRes::Create(
-          scheduler_->VSyncInterval(), task_runner);
-    } else {
-      time_source_ = DelayBasedTimeSource::Create(scheduler_->VSyncInterval(),
-                                                  task_runner);
-    }
-    time_source_->SetClient(this);
-  }
-
-  virtual ~SyntheticBeginFrameSource() {}
-
-  // Updates the phase and frequency of the timer.
-  void CommitVSyncParameters(base::TimeTicks timebase,
-                             base::TimeDelta interval) {
-    time_source_->SetTimebaseAndInterval(timebase, interval);
-  }
-
-  // Activates future BeginFrames and, if activating, pushes the most
-  // recently missed BeginFrame to the back of a retroactive queue.
-  void SetNeedsBeginFrame(bool needs_begin_frame,
-                          std::deque<BeginFrameArgs>* begin_retro_frame_args) {
-    base::TimeTicks missed_tick_time =
-        time_source_->SetActive(needs_begin_frame);
-    if (!missed_tick_time.is_null()) {
-      begin_retro_frame_args->push_back(
-          CreateSyntheticBeginFrameArgs(missed_tick_time));
-    }
-  }
-
-  // TimeSourceClient implementation of OnTimerTick triggers a BeginFrame.
-  virtual void OnTimerTick() OVERRIDE {
-    BeginFrameArgs begin_frame_args(
-        CreateSyntheticBeginFrameArgs(time_source_->LastTickTime()));
-    scheduler_->BeginFrame(begin_frame_args);
-  }
-
- private:
-  BeginFrameArgs CreateSyntheticBeginFrameArgs(base::TimeTicks frame_time) {
-    base::TimeTicks deadline =
-        time_source_->NextTickTime() - scheduler_->EstimatedParentDrawTime();
-    return BeginFrameArgs::Create(
-        frame_time, deadline, scheduler_->VSyncInterval());
-  }
-
-  Scheduler* scheduler_;
-  scoped_refptr<TimeSource> time_source_;
-};
-
 Scheduler::Scheduler(
     SchedulerClient* client,
     const SchedulerSettings& scheduler_settings,
     int layer_tree_host_id,
+    FrameSource* external_frame_source,
     const scoped_refptr<base::SingleThreadTaskRunner>& impl_task_runner)
     : settings_(scheduler_settings),
       client_(client),
       layer_tree_host_id_(layer_tree_host_id),
       impl_task_runner_(impl_task_runner),
-      vsync_interval_(BeginFrameArgs::DefaultInterval()),
       last_set_needs_begin_frame_(false),
       begin_unthrottled_frame_posted_(false),
       begin_retro_frame_posted_(false),
@@ -94,8 +41,9 @@ Scheduler::Scheduler(
 
   begin_retro_frame_closure_ =
       base::Bind(&Scheduler::BeginRetroFrame, weak_factory_.GetWeakPtr());
-  begin_unthrottled_frame_closure_ =
-      base::Bind(&Scheduler::BeginUnthrottledFrame, weak_factory_.GetWeakPtr());
+  // begin_unthrottled_frame_closure_ =
+  //    base::Bind(&Scheduler::BeginUnthrottledFrame,
+  //    weak_factory_.GetWeakPtr());
   begin_impl_frame_deadline_closure_ = base::Bind(
       &Scheduler::OnBeginImplFrameDeadline, weak_factory_.GetWeakPtr());
   poll_for_draw_triggers_closure_ = base::Bind(
@@ -103,22 +51,29 @@ Scheduler::Scheduler(
   advance_commit_state_closure_ = base::Bind(
       &Scheduler::PollToAdvanceCommitState, weak_factory_.GetWeakPtr());
 
-  if (!settings_.begin_frame_scheduling_enabled) {
-    SetupSyntheticBeginFrames();
+  scoped_ptr<FrameSource> primary_frame_source;
+  if (settings_.begin_frame_scheduling_enabled) {
+    TRACE_EVENT0("frame_time", "Scheduler::Scheduler() ProxyFrameSource");

[brianderson, 2014/05/07 17:20:16]

Should the category be "cc" instead of "frame_time"?

[mithro-old, 2014/05/07 22:02:32]

Good question. 

I was pondering introducing "frame_time" as a subcategory of cc disabled by
default which gives more information about the frame time stuff.

I see the options are;
  * Convert to cc
  * Own sub-category under cc
  * Remove before landing

[brianderson, 2014/05/08 00:55:58]

If we don't assign it to cc or a new category, there's also a disabled by
default category called cc.debug.scheduler.

+    primary_frame_source = scoped_ptr<FrameSource>(
+        new ProxyFrameSource(this, external_frame_source));
+  } else if (!settings_.throttle_frame_production) {

[brianderson, 2014/05/07 17:20:16]

throttle_frame_production should take precidence, such that the if/else order
is:

if (throttle_frame_production) {}
else if (begin_frame_scheduling_enabled) {}
else { // synthetic }

[mithro-old, 2014/05/07 23:42:28]

Done.

+    TRACE_EVENT0("frame_time", "Scheduler::Scheduler() BackToBackFrameSource");
+    primary_frame_source = scoped_ptr<FrameSource>(
+        new BackToBackFrameSource(this, impl_task_runner_));
+  } else {
+    TRACE_EVENT0("frame_time", "Scheduler::Scheduler() SyntheticFrameSource");
+    primary_frame_source = scoped_ptr<FrameSource>(new SyntheticFrameSource(
+        this, impl_task_runner_, base::TimeDelta::FromMilliseconds(16)));
   }
-}
+  scoped_ptr<FrameSource> background_frame_source(new SyntheticFrameSource(
+      this, impl_task_runner_, base::TimeDelta::FromSeconds(1)));
 
-Scheduler::~Scheduler() {
-  if (synthetic_begin_frame_source_) {
-    synthetic_begin_frame_source_->SetNeedsBeginFrame(false,
-                                                      &begin_retro_frame_args_);
-  }
+  frame_source_ = scoped_ptr<DualFrameSource>(new DualFrameSource(
+      this, primary_frame_source.Pass(), background_frame_source.Pass()));
 }
 
-void Scheduler::SetupSyntheticBeginFrames() {
-  DCHECK(!synthetic_begin_frame_source_);
-  synthetic_begin_frame_source_.reset(
-      new SyntheticBeginFrameSource(this, impl_task_runner_.get()));
+Scheduler::~Scheduler() {
+  frame_source_->SetNeedsBeginFrame(false);
 }
 
 void Scheduler::CommitVSyncParameters(base::TimeTicks timebase,
@@ -126,9 +81,8 @@ void Scheduler::CommitVSyncParameters(base::TimeTicks timebase,
   // TODO(brianderson): We should not be receiving 0 intervals.
   if (interval == base::TimeDelta())
     interval = BeginFrameArgs::DefaultInterval();
-  vsync_interval_ = interval;
-  if (!settings_.begin_frame_scheduling_enabled)
-    synthetic_begin_frame_source_->CommitVSyncParameters(timebase, interval);
+
+  frame_source_->SetTimeBaseAndInterval(timebase, interval);
 }
 
 void Scheduler::SetEstimatedParentDrawTime(base::TimeDelta draw_time) {
@@ -142,6 +96,11 @@ void Scheduler::SetCanStart() {
 
 void Scheduler::SetVisible(bool visible) {
   state_machine_.SetVisible(visible);
+  if (visible) {
+    frame_source_->SwitchSource(frame_source_->SourceSecondary());
+  } else {
+    frame_source_->SwitchSource(frame_source_->SourcePrimary());
+  }
   ProcessScheduledActions();
 }
 
@@ -269,50 +228,7 @@ base::TimeTicks Scheduler::LastBeginImplFrameTime() {
 void Scheduler::SetupNextBeginFrameIfNeeded() {
   bool needs_begin_frame = state_machine_.BeginFrameNeeded();
 
-  if (settings_.throttle_frame_production) {
-    SetupNextBeginFrameWhenVSyncThrottlingEnabled(needs_begin_frame);
-  } else {
-    SetupNextBeginFrameWhenVSyncThrottlingDisabled(needs_begin_frame);
-  }
-  SetupPollingMechanisms(needs_begin_frame);
-}
-
-// When we are throttling frame production, we request BeginFrames
-// from the OutputSurface.
-void Scheduler::SetupNextBeginFrameWhenVSyncThrottlingEnabled(
-    bool needs_begin_frame) {
-  bool at_end_of_deadline =
-      state_machine_.begin_impl_frame_state() ==
-          SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE;
-
-  bool should_call_set_needs_begin_frame =
-      // Always request the BeginFrame immediately if it wasn't needed before.
-      (needs_begin_frame && !last_set_needs_begin_frame_) ||
-      // Only stop requesting BeginFrames after a deadline.
-      (!needs_begin_frame && last_set_needs_begin_frame_ && at_end_of_deadline);
-
-  if (should_call_set_needs_begin_frame) {
-    if (settings_.begin_frame_scheduling_enabled) {
-      client_->SetNeedsBeginFrame(needs_begin_frame);
-    } else {
-      synthetic_begin_frame_source_->SetNeedsBeginFrame(
-          needs_begin_frame, &begin_retro_frame_args_);
-    }
-    last_set_needs_begin_frame_ = needs_begin_frame;
-  }
-
-  PostBeginRetroFrameIfNeeded();
-}
-
-// When we aren't throttling frame production, we initiate a BeginFrame
-// as soon as one is needed.
-void Scheduler::SetupNextBeginFrameWhenVSyncThrottlingDisabled(
-    bool needs_begin_frame) {
-  last_set_needs_begin_frame_ = needs_begin_frame;
-
-  if (!needs_begin_frame || begin_unthrottled_frame_posted_)
-    return;
-
+  // Only stop requesting BeginFrames after a deadline.
   if (state_machine_.begin_impl_frame_state() !=
           SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE &&
       state_machine_.begin_impl_frame_state() !=
@@ -320,24 +236,15 @@ void Scheduler::SetupNextBeginFrameWhenVSyncThrottlingDisabled(
     return;

[brianderson, 2014/05/07 18:09:59]

I don't think we can't return early here, otherwise we'll skip calling
SetupPOllingMechanisms and PostBeginRetroFrameIfNeeded.

[mithro-old, 2014/05/07 23:42:28]

Will look at this further as I fix the unittests.

   }
 
-  begin_unthrottled_frame_posted_ = true;
-  impl_task_runner_->PostTask(FROM_HERE, begin_unthrottled_frame_closure_);
-}
-
-// BeginUnthrottledFrame is used when we aren't throttling frame production.
-// This will usually be because VSync is disabled.
-void Scheduler::BeginUnthrottledFrame() {
-  DCHECK(!settings_.throttle_frame_production);
-  DCHECK(begin_retro_frame_args_.empty());
-
-  base::TimeTicks now = gfx::FrameTime::Now();
-  base::TimeTicks deadline = now + vsync_interval_;
+  // Always request the BeginFrame immediately if it wasn't needed before.
+  if (needs_begin_frame && !last_set_needs_begin_frame_) {
+    frame_source_->SetNeedsBeginFrame(needs_begin_frame);
+    last_set_needs_begin_frame_ = needs_begin_frame;
+  }
 
-  BeginFrameArgs begin_frame_args =
-      BeginFrameArgs::Create(now, deadline, vsync_interval_);
-  BeginImplFrame(begin_frame_args);
+  SetupPollingMechanisms(needs_begin_frame);
 
-  begin_unthrottled_frame_posted_ = false;
+  PostBeginRetroFrameIfNeeded();
 }
 
 // We may need to poll when we can't rely on BeginFrame to advance certain
@@ -394,7 +301,9 @@ void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
 // If the scheduler is busy, we queue the BeginFrame to be handled later as
 // a BeginRetroFrame.
 void Scheduler::BeginFrame(const BeginFrameArgs& args) {
-  TRACE_EVENT1("cc", "Scheduler::BeginFrame", "frame_time", args.frame_time);
+  TRACE_EVENT1(
+      "cc", "Scheduler::SchedulerBeginFrame", "frame_time", args.frame_time);
+
   DCHECK(settings_.throttle_frame_production);
 
   bool should_defer_begin_frame;
@@ -690,13 +599,12 @@ bool Scheduler::WillDrawIfNeeded() const {
 scoped_ptr<base::Value> Scheduler::StateAsValue() const {
   scoped_ptr<base::DictionaryValue> state(new base::DictionaryValue);
   state->Set("state_machine", state_machine_.AsValue().release());
+  state->Set("frame_source", frame_source_->FrameSourceAsValue().release());
 
   scoped_ptr<base::DictionaryValue> scheduler_state(new base::DictionaryValue);
   scheduler_state->SetDouble(
       "time_until_anticipated_draw_time_ms",
       (AnticipatedDrawTime() - base::TimeTicks::Now()).InMillisecondsF());
-  scheduler_state->SetDouble("vsync_interval_ms",
-                             vsync_interval_.InMillisecondsF());
   scheduler_state->SetDouble("estimated_parent_draw_time_ms",
                              estimated_parent_draw_time_.InMillisecondsF());
   scheduler_state->SetBoolean("last_set_needs_begin_frame_",