Refactoring the way begin frame sources inside scheduler work.

cc/scheduler/frame_source.cc

Index: cc/scheduler/frame_source.cc
diff --git a/cc/scheduler/frame_source.cc b/cc/scheduler/frame_source.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5bc18c5abed18308b05948a05eff90237c4364df
--- /dev/null
+++ b/cc/scheduler/frame_source.cc
@@ -0,0 +1,297 @@
+// Copyright 2014 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.
+
+#include "cc/scheduler/frame_source.h"
+
+#include "base/debug/trace_event.h"
+#include "base/logging.h"
+#include "cc/scheduler/delay_based_time_source.h"
+#include "cc/scheduler/scheduler.h"
+#include "ui/gfx/frame_time.h"
+
+#define FRAMESOURCE_GETTYPE(x) \
+  std::string x::FrameSourceType() const { return #x; }
+
+namespace cc {
+
+BaseFrameSource::BaseFrameSource(FrameSink* sink) : frame_sink_(sink) {
+}
+
+void BaseFrameSource::SetSink(FrameSink* sink) {
+  frame_sink_ = sink;
+}
+
+void BaseFrameSource::SendBeginFrame(const BeginFrameArgs& args) {
+  DCHECK(frame_sink_);
+  frame_sink_->BeginFrame(args);
+}
+
+void BaseFrameSource::SetTimeBaseAndInterval(base::TimeTicks timebase,
+                                             base::TimeDelta interval) {
+  if (interval != base::TimeDelta())
+    interval_ = interval;
+}
+
+scoped_ptr<base::Value> BaseFrameSource::FrameSourceAsValue() const {
+  return BaseFrameSourceAsValue().PassAs<base::Value>();
+}
+
+scoped_ptr<base::DictionaryValue> BaseFrameSource::BaseFrameSourceAsValue()
+    const {
+  scoped_ptr<base::DictionaryValue> state(new base::DictionaryValue);
+  state->SetString("type", FrameSourceType());
+  state->SetInteger("frame_source", reinterpret_cast<size_t>(this));
+  state->SetInteger("interval_us", interval_.InMicroseconds());
+  return state.PassAs<base::DictionaryValue>();
+}
+
+/**
+ *
+ */
+FRAMESOURCE_GETTYPE(ProxyFrameSource);
+
+ProxyFrameSource::ProxyFrameSource(FrameSink* sink, FrameSource* source)
+    : BaseFrameSource(sink), source_(source) {
+  DCHECK(source_);
+  source->SetSink(this);
+}
+
+void ProxyFrameSource::BeginFrame(const BeginFrameArgs& args) {
+  TRACE_EVENT0("frame_time", "ProxyFrameSource::BeginFrame");
+  SendBeginFrame(args);
+}
+
+void ProxyFrameSource::SetNeedsBeginFrame(bool needs_begin_frame) {
+  DCHECK(source_);
+  source_->SetNeedsBeginFrame(needs_begin_frame);
+}
+
+scoped_ptr<base::Value> ProxyFrameSource::FrameSourceAsValue() const {
+  scoped_ptr<base::DictionaryValue> state =
+      BaseFrameSource::BaseFrameSourceAsValue();
+  state->Set("source", source_->FrameSourceAsValue().release());
+  return state.PassAs<base::Value>();
+}
+
+void ProxyFrameSource::SetTimeBaseAndInterval(base::TimeTicks timebase,
+                                              base::TimeDelta interval) {
+  source_->SetTimeBaseAndInterval(timebase, interval);
+}
+
+/**
+ *
+ */
+FRAMESOURCE_GETTYPE(ThrottledFrameSource)
+
+ThrottledFrameSource::ThrottledFrameSource(FrameSink* sink,
+                                           scoped_ptr<FrameSource> source,
+                                           base::TimeDelta interval)
+    : BaseFrameSource(sink), last_frame_args_(), source_(source.Pass()) {
+  interval_ = interval;
+  DCHECK(source_);
+  source_->SetSink(this);
+}
+
+ThrottledFrameSource::~ThrottledFrameSource() {
+}
+
+void ThrottledFrameSource::BeginFrame(const BeginFrameArgs& args) {
+  if (last_frame_args_.IsValid() &&
+      last_frame_args_.frame_time + interval_ > args.frame_time)
+    return;
+  last_frame_args_ = args;
+  SendBeginFrame(args);
+}
+
+void ThrottledFrameSource::SetNeedsBeginFrame(bool needs_begin_frame) {
+  source_->SetNeedsBeginFrame(needs_begin_frame);
+}
+
+void ThrottledFrameSource::SetTimeBaseAndInterval(base::TimeTicks timebase,
+                                                  base::TimeDelta interval) {
+  source_->SetTimeBaseAndInterval(timebase, interval);
+}
+
+scoped_ptr<base::Value> ThrottledFrameSource::FrameSourceAsValue() const {
+  scoped_ptr<base::DictionaryValue> state =
+      BaseFrameSource::BaseFrameSourceAsValue();
+  state->Set("source", source_->FrameSourceAsValue().release());
+  state->Set("last_frame_args", last_frame_args_.AsValue().release());
+  return state.PassAs<base::Value>();
+}
+
+/**
+ *
+ */
+TaskRunnerFrameSource::TaskRunnerFrameSource(
+    FrameSink* sink,
+    base::SingleThreadTaskRunner* task_runner)
+    : BaseFrameSource(sink), task_runner_(task_runner) {
+  DCHECK(task_runner);
+}
+
+/**
+ * A frame source which sends a BeginFrame as soon as SetNeedsBeginFrame is
+ * requested.
+ */
+FRAMESOURCE_GETTYPE(BackToBackFrameSource)
+
+BackToBackFrameSource::BackToBackFrameSource(
+    FrameSink* sink,
+    base::SingleThreadTaskRunner* task_runner)
+    : TaskRunnerFrameSource(sink, task_runner) {
+}
+
+void BackToBackFrameSource::SetNeedsBeginFrame(bool needs_begin_frame) {
+  if (needs_begin_frame) {

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

Which solution are you leaning towards for solving when to post the next
BackToBack BeginFrame?

I personally like the solution of a FrameSource::ReadyForNextBeginFrame.

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

Yeah, I'm leaning towards the ReadyForNextBeginFrame system too.

If we go this route, I'm wondering if ReadyForNextBeginFrame should also be a
signal to "commit" the "SetNeedsBeginFrame" state?

[brianderson, 2014/05/08 21:08:15]

I've changed my mind here. Let's just keep the single
SetNeedsBeginFrame(needs_begin_frame) entry point and force the scheduler to
call it whenever it's ready for the next BeginFrame. It'll work just like having
a ReadyForNextBeginFrame, but we don't really need an extra entry point.

When vsync is disabled, we can SetNeedsBeginFrame when we are finished with a
frame. When vsync is enabled, we can SetNeedsBeginFrame as part of the
BeginFrame.

[mithro-old, 2014/05/09 02:45:58]

Are you saying that SetNeedsBeginFrame should be edge triggered? Or it is still
level triggered but the level should be updated whenever the scheduler is ready
for a new frame?

What about the pipelining from the browser issue?

+    /*    task_runner_->PostDelayedTask(
+            FROM_HERE,
+            base::Bind(&BackToBackFrameSource::PostBeginFrame,
+                       this)); */
+  }
+}
+
+void BackToBackFrameSource::PostBeginFrame() {
+  base::TimeTicks now = gfx::FrameTime::Now();
+  BeginFrameArgs args =
+      BeginFrameArgs::Create(now, base::TimeTicks(), base::TimeDelta());
+  SendBeginFrame(args);
+}
+
+/**
+ *
+ */
+
+SyntheticFrameSource::~SyntheticFrameSource() {
+}
+
+FRAMESOURCE_GETTYPE(SyntheticFrameSource);
+SyntheticFrameSource::SyntheticFrameSource(
+    FrameSink* sink,
+    base::SingleThreadTaskRunner* task_runner,
+    base::TimeDelta interval)
+    : TaskRunnerFrameSource(sink, task_runner) {
+  interval_ = interval;
+  if (gfx::FrameTime::TimestampsAreHighRes()) {
+    time_source_ = DelayBasedTimeSourceHighRes::Create(interval_, task_runner);
+  } else {
+    time_source_ = DelayBasedTimeSource::Create(interval_, task_runner);
+  }
+  time_source_->SetClient(this);
+}
+
+// Activates future BeginFrames and, if activating, pushes the most
+// recently missed BeginFrame to the back of a retroactive queue.
+void SyntheticFrameSource::SetNeedsBeginFrame(bool needs_begin_frame) {
+  base::TimeTicks missed_tick_time = time_source_->SetActive(needs_begin_frame);
+  if (!missed_tick_time.is_null()) {
+    BeginFrameArgs args(CreateSyntheticBeginFrameArgs(missed_tick_time));
+    SendBeginFrame(args);
+  }
+}
+
+// Updates the phase and frequency of the timer.
+void SyntheticFrameSource::SetTimeBaseAndInterval(base::TimeTicks timebase,
+                                                  base::TimeDelta interval) {
+  TaskRunnerFrameSource::SetTimeBaseAndInterval(timebase, interval);
+  time_source_->SetTimebaseAndInterval(timebase, interval);
+}
+
+scoped_ptr<base::Value> SyntheticFrameSource::FrameSourceAsValue() const {
+  scoped_ptr<base::DictionaryValue> state =
+      BaseFrameSource::BaseFrameSourceAsValue();
+  // state->Set("source", source_->FrameSourceAsValue().Pass());
+  return state.PassAs<base::Value>();
+}
+
+// TimeSourceClient implementation of OnTimerTick triggers a BeginFrame.
+void SyntheticFrameSource::OnTimerTick() {
+  BeginFrameArgs args(
+      CreateSyntheticBeginFrameArgs(time_source_->LastTickTime()));
+  SendBeginFrame(args);
+}
+
+BeginFrameArgs SyntheticFrameSource::CreateSyntheticBeginFrameArgs(
+    base::TimeTicks frame_time) {
+  base::TimeTicks deadline =
+      time_source_->NextTickTime();  // scheduler_->EstimatedParentDrawTime();
+  return BeginFrameArgs::Create(
+      frame_time, deadline, interval_);  // scheduler_->VSyncInterval());
+}
+
+/**
+ * A virtual frame source which lets you switch between two other frame sources
+ * (making sure the BeginFrameArgs stays monotonic).
+ */
+FRAMESOURCE_GETTYPE(DualFrameSource)
+
+DualFrameSource::DualFrameSource(FrameSink* sink,
+                                 scoped_ptr<FrameSource> source_primary,
+                                 scoped_ptr<FrameSource> source_secondary)
+    : BaseFrameSource(sink),
+      needs_begin_frame_(false),
+      last_frame_args_(),
+      source_primary_(source_primary.Pass()),
+      source_secondary_(source_secondary.Pass()) {
+  source_primary_->SetSink(this);
+  source_secondary_->SetSink(this);
+  SwitchSource(SourcePrimary());
+}
+
+DualFrameSource::~DualFrameSource() {
+}
+
+void DualFrameSource::BeginFrame(const BeginFrameArgs& args) {
+  // When switching clock sources you can get a BeginFrameArgs with a
+  // frame_time before the last value.
+  BeginFrameArgs new_args = args;
+  if (last_frame_args_.frame_time > new_args.frame_time)
+    return;
+
+  last_frame_args_ = new_args;
+  BaseFrameSource::SendBeginFrame(new_args);
+}
+
+void DualFrameSource::SetNeedsBeginFrame(bool needs_begin_frame) {
+  DCHECK(active_source_);
+  needs_begin_frame_ = needs_begin_frame;
+  active_source_->SetNeedsBeginFrame(needs_begin_frame);
+}
+
+scoped_ptr<base::Value> DualFrameSource::FrameSourceAsValue() const {
+  scoped_ptr<base::DictionaryValue> state =
+      BaseFrameSource::BaseFrameSourceAsValue();
+  if (active_source_ == SourcePrimary())
+    state->SetString("active", "primary");
+  if (active_source_ == SourceSecondary())
+    state->SetString("active", "secondary");
+  state->Set("primary", source_primary_->FrameSourceAsValue().release());
+  state->Set("secondary", source_secondary_->FrameSourceAsValue().release());
+  state->Set("last_frame_args", last_frame_args_.AsValue().release());
+  return state.PassAs<base::Value>();
+}
+
+// ---------------------------------------------------------------------
+
+void DualFrameSource::SwitchSource(const FrameSource* new_source) {
+  DCHECK(new_source == SourcePrimary() || new_source == SourceSecondary());

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

Can we avoid exposing the underlying frame sources to the here with an enum or
two separate methods?

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

I originally started with an enum.

After adding the methods to allow access the internal FrameSources, it seemed
like it was just easier to use the pointers. This interface does extends to the
case of a "ManyFrameSource" which could have an arbitrary number of internal
frame source. 

Happy to go with whichever you think is a better interface.

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

Do we need to expose the underlying sources at all?

[mithro-old, 2014/05/09 02:45:58]

As the DualFrameSource takes ownership of the sources it seems like a good idea?

We might want to pass a DualFrameSource around without also passing pointers to
it's internal sources?

The below seems kinda of bad?
------------------------------------------------------------
class A {

 FrameSource* frame_f_;
 FrameSource* frame_b_;
 scoped_ptr<FrameSource> frame_source_;

 A() : {
   frame_f_ = new FrameSourceA();
   frame_b_ = new FrameSourceB();
   frame_source_ make_scoped_ptr(new DualFrameSource(frame_f_, frame_b_));
 }

 void do() {
   frame_f_->XXXX();
 }
}
------------------------------------------------------------
verses
------------------------------------------------------------

class A {

 FrameSource* frame_f_;
 FrameSource* frame_b_;
 scoped_ptr<FrameSource> frame_source_;

 A() : {
   frame_source_ make_scoped_ptr(new DualFrameSource(new FrameSourceA(), new
FrameSourceB());
 }

 void do() {
   frame_source->SourceForeground()->XXXX();
 }
}
------------------------------------------------------------

+
+  if (needs_begin_frame_)
+    active_source_->SetNeedsBeginFrame(false);
+
+  active_source_ = const_cast<FrameSource*>(new_source);
+
+  if (needs_begin_frame_)
+    active_source_->SetNeedsBeginFrame(true);
+}
+
+const FrameSource* DualFrameSource::SourcePrimary() const {
+  return source_primary_.get();
+}
+
+const FrameSource* DualFrameSource::SourceSecondary() const {
+  return source_secondary_.get();
+}
+
+}  // namespace cc