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..c7d57c3589a6cbefaec8955cf5ebdced39fb6b85
--- /dev/null
+++ b/cc/scheduler/frame_source.cc
@@ -0,0 +1,398 @@
+// 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/auto_reset.h"
+#include "base/debug/trace_event.h"
+#include "base/debug/trace_event_argument.h"
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "cc/scheduler/delay_based_time_source.h"
+#include "cc/scheduler/scheduler.h"
+#include "ui/gfx/frame_time.h"
+
+namespace cc {
+
+VSyncParameterObserver::VSyncParameterObserver(
+    base::TimeTicks initial_vsync_timebase,
+    base::TimeDelta initial_vsync_interval)
+    : vsync_timebase_(initial_vsync_timebase),
+      vsync_interval_(initial_vsync_interval) {
+}
+VSyncParameterObserver::~VSyncParameterObserver() {
+}
+
+void VSyncParameterObserver::OnUpdateVSyncParameters(
+    base::TimeTicks new_vsync_timebase,
+    base::TimeDelta new_vsync_interval) {
+  if (vsync_timebase_ != new_vsync_timebase ||
+      vsync_interval_ != new_vsync_interval) {
+    OnTimeBaseAndIntervalChange(new_vsync_timebase, new_vsync_interval);
+  }
+  vsync_timebase_ = new_vsync_timebase;
+  vsync_interval_ = new_vsync_interval;
+}
+
+base::TimeTicks VSyncParameterObserver::VSyncTimebase() const {
+  return vsync_timebase_;
+}
+
+base::TimeDelta VSyncParameterObserver::VSyncInterval() const {
+  return vsync_interval_;
+}
+
+// Tracing
+void VSyncParameterObserver::AsValueInto(base::debug::TracedValue* dict) const {
+  dict->SetInteger("vsync_timebase_us", vsync_timebase_.ToInternalValue());
+  dict->SetInteger("vsync_interval_us", vsync_interval_.InMicroseconds());
+}
+
+/*************************************************************************/
+
+void BeginFrameSource::AddObserver(BeginFrameObserver* obs) {
+  DCHECK(!observer_);
+  observer_ = obs;
+}
+
+void BeginFrameSource::RemoveObserver(BeginFrameObserver* obs) {
+  DCHECK_EQ(observer_, obs);
+  observer_ = NULL;
+}
+
+void BeginFrameSource::SendBeginFrame(const BeginFrameArgs& args) {

[brianderson, 2014/09/18 00:27:25]

"Send" implies that we're sending a message. Should we just call this
OnBeginFrame too?

[mithro-old, 2014/09/18 13:33:37]

It can't actually be named OnBeginFrame otherwise you can't implement both a
BeginFrameSource and BeginFrameObserver.

I changed to "CallOnBeginFrame", is that better?

[brianderson, 2014/09/18 21:54:18]

CallOnBeginFrame sgtm.

[mithro-old, 2014/09/19 02:45:38]

Acknowledged.

+  if (observer_) {
+    observer_->OnBeginFrame(args);
+  }
+}
+
+// Tracing support
+void BeginFrameSource::AsValueInto(base::debug::TracedValue* dict) const {
+  // As the observer might try to trace the source, prevent an infinte loop
+  // from occuring.
+  if (inside_as_value_into_) {
+    dict->SetString("observer", "<loop detected>");
+    return;
+  }
+
+  if (observer_) {
+    base::AutoReset<bool> prevent_loops(
+        const_cast<bool*>(&inside_as_value_into_), true);
+    dict->BeginDictionary("observer");
+    observer_->AsValueInto(dict);
+    dict->EndDictionary();
+  } else {
+    dict->SetString("observer", "NULL");
+  }
+}
+
+/*************************************************************************/
+scoped_ptr<BackToBackBeginFrameSource> BackToBackBeginFrameSource::Create(
+    base::SingleThreadTaskRunner* task_runner) {
+  return make_scoped_ptr(new BackToBackBeginFrameSource(task_runner));
+}
+
+BackToBackBeginFrameSource::BackToBackBeginFrameSource(
+    base::SingleThreadTaskRunner* task_runner)
+    : weak_factory_(this),
+      task_runner_(task_runner),
+      needs_begin_frames_(false),
+      send_begin_frame_posted_(false) {
+  DCHECK(task_runner);
+}
+
+BackToBackBeginFrameSource::~BackToBackBeginFrameSource() {
+}
+
+base::TimeTicks BackToBackBeginFrameSource::Now() {
+  return gfx::FrameTime::Now();
+}
+
+void BackToBackBeginFrameSource::ScheduleSendBeginFrameArgs() {

[brianderson, 2014/09/18 00:27:25]

ScheduleBeginFrame?

[mithro-old, 2014/09/18 13:33:37]

Done.

+  if (!needs_begin_frames_)
+    return;
+
+  if (send_begin_frame_posted_)
+    return;
+
+  send_begin_frame_posted_ = true;
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&BackToBackBeginFrameSource::SendBeginFrameArgs,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void BackToBackBeginFrameSource::SendBeginFrameArgs() {

[brianderson, 2014/09/18 00:27:25]

SendBeginFrame -> BeginFrame?

[mithro-old, 2014/09/18 13:33:37]

Done.

+  send_begin_frame_posted_ = false;
+
+  // TODO(mithro): Fix the tests so this can be enabled. The tests currently
+  // depend on one begin frame firing after generate_frames_ is set false.
+  // if (!generate_frames_) {

[brianderson, 2014/09/18 00:27:25]

Should this be !needs_begin_frames_?

Why do the tests depend on this?
Are we sure this isn't hiding real issues?

[mithro-old, 2014/09/18 13:33:37]

Fixed.
Good catch.

The tests use to execute tasks out of order which meant this was needed. 
Now we have proper ordering

Removed (plus test updated to check this behavior works correctly).

+  //  return;
+  // }
+
+  base::TimeTicks now = Now();
+  // Set deadline somewhere a long time in the future.

[brianderson, 2014/09/18 00:27:25]

Comment doesn't match code anymore here.

[mithro-old, 2014/09/18 13:33:37]

Removed.

+  BeginFrameArgs args =
+      BeginFrameArgs::Create(now,
+                             now + BeginFrameArgs::DefaultInterval(),
+                             BeginFrameArgs::DefaultInterval());
+  SendBeginFrame(args);
+}
+
+// BeginFrameSource ------------------------------------------------------
+bool BackToBackBeginFrameSource::NeedsBeginFrames() const {
+  return needs_begin_frames_;
+}
+
+void BackToBackBeginFrameSource::SetNeedsBeginFrames(bool needs_begin_frames) {
+  needs_begin_frames_ = needs_begin_frames;
+  ScheduleSendBeginFrameArgs();
+}
+
+void BackToBackBeginFrameSource::DidFinishFrame(size_t remaining_frames) {
+  if (remaining_frames == 0) {
+    ScheduleSendBeginFrameArgs();
+  }
+}
+
+// Tracing
+void BackToBackBeginFrameSource::AsValueInto(
+    base::debug::TracedValue* dict) const {
+  dict->SetString("type", "BackToBackBeginFrameSource");
+  BeginFrameSource::AsValueInto(dict);
+  dict->SetBoolean("send_begin_frame_posted_", send_begin_frame_posted_);
+  dict->SetBoolean("needs_begin_frames", needs_begin_frames_);
+}
+
+/*************************************************************************/
+
+scoped_ptr<SyntheticBeginFrameSource> SyntheticBeginFrameSource::Create(
+    base::SingleThreadTaskRunner* task_runner,
+    base::TimeTicks initial_vsync_timebase,
+    base::TimeDelta initial_vsync_interval) {
+  scoped_refptr<DelayBasedTimeSource> time_source;
+  if (gfx::FrameTime::TimestampsAreHighRes()) {
+    time_source = DelayBasedTimeSourceHighRes::Create(initial_vsync_interval,
+                                                      task_runner);
+  } else {
+    time_source =
+        DelayBasedTimeSource::Create(initial_vsync_interval, task_runner);
+  }
+
+  return make_scoped_ptr(new SyntheticBeginFrameSource(
+      time_source, initial_vsync_timebase, initial_vsync_interval));
+}
+
+SyntheticBeginFrameSource::SyntheticBeginFrameSource(
+    scoped_refptr<DelayBasedTimeSource> time_source,
+    base::TimeTicks initial_vsync_timebase,
+    base::TimeDelta initial_vsync_interval)
+    : BeginFrameSource(),
+      VSyncParameterObserver(initial_vsync_timebase, initial_vsync_interval),
+      time_source_(time_source) {
+  time_source_->SetActive(false);
+  time_source_->SetClient(this);
+}
+
+SyntheticBeginFrameSource::~SyntheticBeginFrameSource() {
+  if (NeedsBeginFrames())
+    time_source_->SetActive(false);
+}
+
+void SyntheticBeginFrameSource::OnTimeBaseAndIntervalChange(
+    const base::TimeTicks new_vsync_timebase,
+    const base::TimeDelta new_vsync_interval) {
+  time_source_->SetTimebaseAndInterval(new_vsync_timebase, new_vsync_interval);
+}
+
+void SyntheticBeginFrameSource::SendBeginFrameFromTick(

[brianderson, 2014/09/18 00:27:25]

Please roll this into OnTimerTick.

[brianderson, 2014/09/18 00:31:25]

Nevermind about this one, I see how it's reused when coming out of idle.

[mithro-old, 2014/09/18 13:33:37]

Acknowledged.

[mithro-old, 2014/09/18 13:33:37]

Acknowledged.

+    base::TimeTicks frame_time) {
+  base::TimeTicks deadline = time_source_->NextTickTime();
+  SendBeginFrame(BeginFrameArgs::Create(frame_time, deadline, VSyncInterval()));
+}
+
+// TimeSourceClient
+void SyntheticBeginFrameSource::OnTimerTick() {
+  SendBeginFrameFromTick(time_source_->LastTickTime());
+}
+
+// BeginFrameSource
+void SyntheticBeginFrameSource::SetNeedsBeginFrames(bool needs_begin_frames) {
+  if (needs_begin_frames == NeedsBeginFrames())
+    return;
+
+  base::TimeTicks missed_tick_time =
+      time_source_->SetActive(needs_begin_frames);
+  if (!missed_tick_time.is_null()) {
+    SendBeginFrameFromTick(missed_tick_time);
+  }
+}
+
+bool SyntheticBeginFrameSource::NeedsBeginFrames() const {
+  return time_source_->Active();
+}
+
+// Tracing
+void SyntheticBeginFrameSource::AsValueInto(
+    base::debug::TracedValue* dict) const {
+  dict->SetString("type", "SyntheticBeginFrameSource");
+  BeginFrameSource::AsValueInto(dict);
+
+  dict->BeginDictionary("last_frame_args");
+  time_source_->AsValueInto(dict);
+  dict->EndDictionary();
+}
+
+/*************************************************************************/
+
+scoped_ptr<BeginFrameSourceMultiplexer> BeginFrameSourceMultiplexer::Create() {
+  return make_scoped_ptr(new BeginFrameSourceMultiplexer());
+}
+
+BeginFrameSourceMultiplexer::BeginFrameSourceMultiplexer()
+    : minimum_interval_(base::TimeDelta()),
+      active_source_(NULL),
+      source_list_() {
+}
+
+BeginFrameSourceMultiplexer::BeginFrameSourceMultiplexer(
+    base::TimeDelta minimum_interval)
+    : minimum_interval_(minimum_interval),
+      active_source_(NULL),
+      source_list_() {
+}
+
+BeginFrameSourceMultiplexer::~BeginFrameSourceMultiplexer() {
+}
+
+void BeginFrameSourceMultiplexer::SetActiveSource(
+    BeginFrameSource* new_source) {
+  DCHECK(HasSource(new_source) || new_source == NULL);
+
+  bool needs_begin_frames = NeedsBeginFrames();
+  if (active_source_) {
+    if (needs_begin_frames)
+      SetNeedsBeginFrames(false);
+
+    // Technically we shouldn't need to remove observation, but this prevents
+    // the case where SetNeedsBeginFrames message gets to the source after a
+    // message has already been sent.
+    active_source_->RemoveObserver(this);
+    active_source_ = NULL;
+  }
+  DCHECK(!active_source_);
+  active_source_ = new_source;
+
+  if (active_source_) {
+    active_source_->AddObserver(this);
+
+    if (needs_begin_frames) {
+      SetNeedsBeginFrames(true);
+    }
+  }
+}
+
+const BeginFrameSource* BeginFrameSourceMultiplexer::ActiveSource() {
+  return active_source_;
+}
+
+bool BeginFrameSourceMultiplexer::HasSource(BeginFrameSource* source) {
+  return (source_list_.find(source) != source_list_.end());
+}
+
+// ThrottledBeginFrameSource
+void BeginFrameSourceMultiplexer::AddSource(BeginFrameSource* new_source) {
+  DCHECK(new_source);
+  DCHECK(!HasSource(new_source));
+
+  source_list_.insert(new_source);
+
+  // If there is no active source, set the new one as the active one.
+  if (!active_source_)
+    SetActiveSource(new_source);
+}
+
+void BeginFrameSourceMultiplexer::RemoveSource(
+    BeginFrameSource* existing_source) {
+  DCHECK(existing_source);
+  DCHECK(HasSource(existing_source));
+  DCHECK_NE(existing_source, active_source_);
+  source_list_.erase(existing_source);
+}
+
+void BeginFrameSourceMultiplexer::SetMinimumInterval(
+    base::TimeDelta new_minimum_interval) {
+  DCHECK_GE(new_minimum_interval.ToInternalValue(), 0);
+  minimum_interval_ = new_minimum_interval;
+}
+
+// BeginFrameObserver
+void BeginFrameSourceMultiplexer::OnBeginFrame(const BeginFrameArgs& args) {
+  if (!observer_)
+    return;
+
+  if (observer_->LastBeginFrameArgs().IsValid() &&
+      observer_->LastBeginFrameArgs().frame_time + minimum_interval_ >
+          args.frame_time)
+    return;
+  SendBeginFrame(args);
+}
+
+const BeginFrameArgs& BeginFrameSourceMultiplexer::LastBeginFrameArgs() const {
+  return observer_->LastBeginFrameArgs();
+}
+
+// BeginFrameSource
+bool BeginFrameSourceMultiplexer::NeedsBeginFrames() const {
+  if (active_source_) {
+    return active_source_->NeedsBeginFrames();
+  } else {
+    return false;
+  }
+}
+
+void BeginFrameSourceMultiplexer::SetNeedsBeginFrames(bool needs_begin_frames) {
+  if (active_source_) {
+    active_source_->SetNeedsBeginFrames(needs_begin_frames);
+  } else {
+    DCHECK(!needs_begin_frames);
+  }
+}
+
+void BeginFrameSourceMultiplexer::DidFinishFrame(size_t remaining_frames) {
+  active_source_->DidFinishFrame(remaining_frames);
+}
+
+// Tracing
+void BeginFrameSourceMultiplexer::AsValueInto(
+    base::debug::TracedValue* dict) const {
+  dict->SetString("type", "BeginFrameSourceMultiplexer");
+
+  dict->SetInteger("minimum_interval_us", minimum_interval_.InMicroseconds());
+  if (observer_) {
+    dict->BeginDictionary("last_begin_frame_args");
+    observer_->LastBeginFrameArgs().AsValueInto(dict);
+    dict->EndDictionary();
+  }
+
+  if (active_source_) {
+    dict->BeginDictionary("active_source");
+    active_source_->AsValueInto(dict);
+    dict->EndDictionary();
+  } else {
+    dict->SetString("active_source", "NULL");
+  }
+
+  for (std::set<BeginFrameSource*>::const_iterator it = source_list_.begin();
+       it != source_list_.end();
+       ++it) {
+    dict->BeginDictionary(
+        base::SizeTToString(std::distance(source_list_.begin(), it)).c_str());
+    (*it)->AsValueInto(dict);
+    dict->EndDictionary();
+  }
+}
+
+}  // namespace cc