cc: Make BackToBackBeginFrameSource a SyntheticBeginFrameSource.

cc/scheduler/begin_frame_source.cc

Index: cc/scheduler/begin_frame_source.cc
diff --git a/cc/scheduler/begin_frame_source.cc b/cc/scheduler/begin_frame_source.cc
index 5c20a86caf2c4b11fd1fd54d48b74ca78a893cc7..d4be4d3b4b14d30e98adf6886cd5859d021a4abe 100644
--- a/cc/scheduler/begin_frame_source.cc
+++ b/cc/scheduler/begin_frame_source.cc
@@ -9,6 +9,7 @@
 #include "base/auto_reset.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
@@ -48,109 +49,55 @@ void BeginFrameObserverBase::OnBeginFrame(const BeginFrameArgs& args) {
   }
 }
 
-// BeginFrameSourceBase ------------------------------------------------------
-BeginFrameSourceBase::BeginFrameSourceBase() : paused_(false) {}
-
-BeginFrameSourceBase::~BeginFrameSourceBase() {}
-
-void BeginFrameSourceBase::AddObserver(BeginFrameObserver* obs) {
-  DEBUG_FRAMES("BeginFrameSourceBase::AddObserver", "num observers",
-               observers_.size(), "to add observer", obs);
-  DCHECK(obs);
-  DCHECK(observers_.find(obs) == observers_.end())
-      << "AddObserver cannot be called with an observer that was already added";
-  bool observers_was_empty = observers_.empty();
-  observers_.insert(obs);
-  if (observers_was_empty)
-    OnNeedsBeginFramesChanged(true);
-  obs->OnBeginFrameSourcePausedChanged(paused_);
-}
-
-void BeginFrameSourceBase::RemoveObserver(BeginFrameObserver* obs) {
-  DEBUG_FRAMES("BeginFrameSourceBase::RemoveObserver", "num observers",
-               observers_.size(), "removed observer", obs);
-  DCHECK(obs);
-  DCHECK(observers_.find(obs) != observers_.end())
-      << "RemoveObserver cannot be called with an observer that wasn't added";
-  observers_.erase(obs);
-  if (observers_.empty())
-    OnNeedsBeginFramesChanged(false);
-}
-
-void BeginFrameSourceBase::CallOnBeginFrame(const BeginFrameArgs& args) {
-  DEBUG_FRAMES("BeginFrameSourceBase::CallOnBeginFrame", "num observers",
-               observers_.size(), "args", args.AsValue());
-  std::set<BeginFrameObserver*> observers(observers_);
-  for (BeginFrameObserver* obs : observers)
-    obs->OnBeginFrame(args);
-}
-
-void BeginFrameSourceBase::SetBeginFrameSourcePaused(bool paused) {
-  if (paused_ == paused)
-    return;
-  paused_ = paused;
-  std::set<BeginFrameObserver*> observers(observers_);
-  for (BeginFrameObserver* obs : observers)
-    obs->OnBeginFrameSourcePausedChanged(paused_);
-}
+// SyntheticBeginFrameSource ---------------------------------------------
+SyntheticBeginFrameSource::~SyntheticBeginFrameSource() = default;
 
 // BackToBackBeginFrameSource --------------------------------------------
 BackToBackBeginFrameSource::BackToBackBeginFrameSource(
-    base::SingleThreadTaskRunner* task_runner)
-    : BeginFrameSourceBase(), task_runner_(task_runner), weak_factory_(this) {
-  DCHECK(task_runner);
-}
-
-BackToBackBeginFrameSource::~BackToBackBeginFrameSource() {
+    std::unique_ptr<DelayBasedTimeSource> time_source)
+    : time_source_(std::move(time_source)), weak_factory_(this) {
+  time_source_->SetClient(this);
+  time_source_->SetTimebaseAndInterval(base::TimeTicks(), base::TimeDelta());
 }
 
-base::TimeTicks BackToBackBeginFrameSource::Now() {
-  return base::TimeTicks::Now();
-}
+BackToBackBeginFrameSource::~BackToBackBeginFrameSource() = default;
 
-// BeginFrameSourceBase support
 void BackToBackBeginFrameSource::AddObserver(BeginFrameObserver* obs) {
-  BeginFrameSourceBase::AddObserver(obs);
+  DCHECK(obs);
+  DCHECK(observers_.find(obs) == observers_.end());
+  observers_.insert(obs);
   pending_begin_frame_observers_.insert(obs);
-  PostPendingBeginFramesTask();
+  obs->OnBeginFrameSourcePausedChanged(false);
+  // Generate a new BeginFrame tick.

[enne (OOO), 2016/06/15 19:46:56]

// ... if needed?

(or no comment at all?)

[danakj, 2016/06/15 19:56:05]

Well, so the time source is only active 1 tick at a time, which is what I want
to get across. Hm. I can make that more explicit. I moved it to the place where
we set the 0 interval.

+  time_source_->SetActive(true);
 }
 
 void BackToBackBeginFrameSource::RemoveObserver(BeginFrameObserver* obs) {
-  BeginFrameSourceBase::RemoveObserver(obs);
+  DCHECK(obs);
+  DCHECK(observers_.find(obs) != observers_.end());
+  observers_.erase(obs);
   pending_begin_frame_observers_.erase(obs);
-  if (pending_begin_frame_observers_.empty())
-    begin_frame_task_.Cancel();
+  if (observers_.empty())
+    time_source_->SetActive(false);
 }
 
 void BackToBackBeginFrameSource::DidFinishFrame(BeginFrameObserver* obs,
                                                 size_t remaining_frames) {
-  BeginFrameSourceBase::DidFinishFrame(obs, remaining_frames);
   if (remaining_frames == 0 && observers_.find(obs) != observers_.end()) {
     pending_begin_frame_observers_.insert(obs);
-    PostPendingBeginFramesTask();
+    time_source_->SetActive(true);
   }
 }
 
-void BackToBackBeginFrameSource::PostPendingBeginFramesTask() {
-  DCHECK(needs_begin_frames());
-  DCHECK(!pending_begin_frame_observers_.empty());
-  if (begin_frame_task_.IsCancelled()) {
-    begin_frame_task_.Reset(
-        base::Bind(&BackToBackBeginFrameSource::SendPendingBeginFrames,
-                   weak_factory_.GetWeakPtr()));
-    task_runner_->PostTask(FROM_HERE, begin_frame_task_.callback());
-  }
-}
-
-void BackToBackBeginFrameSource::SendPendingBeginFrames() {
-  DCHECK(needs_begin_frames());
-  DCHECK(!begin_frame_task_.IsCancelled());
-  begin_frame_task_.Cancel();
-
-  base::TimeTicks now = Now();
+void BackToBackBeginFrameSource::OnTimerTick() {
+  base::TimeTicks frame_time = time_source_->LastTickTime();
+  base::TimeDelta default_interval = BeginFrameArgs::DefaultInterval();
   BeginFrameArgs args = BeginFrameArgs::Create(
-      BEGINFRAME_FROM_HERE, now, now + BeginFrameArgs::DefaultInterval(),
-      BeginFrameArgs::DefaultInterval(), BeginFrameArgs::NORMAL);
+      BEGINFRAME_FROM_HERE, frame_time, frame_time + default_interval,
+      default_interval, BeginFrameArgs::NORMAL);
+
+  // This must happen after getting the LastTickTime() from the time source.
+  time_source_->SetActive(false);
 
   std::set<BeginFrameObserver*> pending_observers;
   pending_observers.swap(pending_begin_frame_observers_);
@@ -158,40 +105,36 @@ void BackToBackBeginFrameSource::SendPendingBeginFrames() {
     obs->OnBeginFrame(args);
 }
 
-// SyntheticBeginFrameSource ---------------------------------------------
-SyntheticBeginFrameSource::SyntheticBeginFrameSource(
-    base::SingleThreadTaskRunner* task_runner,
-    base::TimeDelta initial_vsync_interval)
-    : time_source_(
-          DelayBasedTimeSource::Create(initial_vsync_interval, task_runner)) {
-  time_source_->SetClient(this);
-}
-
-SyntheticBeginFrameSource::SyntheticBeginFrameSource(
+// DelayBasedBeginFrameSource ---------------------------------------------
+DelayBasedBeginFrameSource::DelayBasedBeginFrameSource(
     std::unique_ptr<DelayBasedTimeSource> time_source)
     : time_source_(std::move(time_source)) {
   time_source_->SetClient(this);
 }
 
-SyntheticBeginFrameSource::~SyntheticBeginFrameSource() {}
+DelayBasedBeginFrameSource::~DelayBasedBeginFrameSource() = default;
 
-void SyntheticBeginFrameSource::OnUpdateVSyncParameters(
+void DelayBasedBeginFrameSource::OnUpdateVSyncParameters(
     base::TimeTicks timebase,
     base::TimeDelta interval) {
-  if (!authoritative_interval_.is_zero())
+  if (!authoritative_interval_.is_zero()) {
     interval = authoritative_interval_;
+  } else if (interval.is_zero()) {
+    // TODO(brianderson): We should not be receiving 0 intervals.
+    interval = BeginFrameArgs::DefaultInterval();
+  }
 
   last_timebase_ = timebase;
   time_source_->SetTimebaseAndInterval(timebase, interval);
 }
 
-void SyntheticBeginFrameSource::SetAuthoritativeVSyncInterval(
+void DelayBasedBeginFrameSource::SetAuthoritativeVSyncInterval(
     base::TimeDelta interval) {
   authoritative_interval_ = interval;
   OnUpdateVSyncParameters(last_timebase_, interval);
 }
 
-BeginFrameArgs SyntheticBeginFrameSource::CreateBeginFrameArgs(
+BeginFrameArgs DelayBasedBeginFrameSource::CreateBeginFrameArgs(
     base::TimeTicks frame_time,
     BeginFrameArgs::BeginFrameArgsType type) {
   return BeginFrameArgs::Create(BEGINFRAME_FROM_HERE, frame_time,
@@ -199,9 +142,15 @@ BeginFrameArgs SyntheticBeginFrameSource::CreateBeginFrameArgs(
                                 time_source_->Interval(), type);
 }
 
-// BeginFrameSource support
-void SyntheticBeginFrameSource::AddObserver(BeginFrameObserver* obs) {
-  BeginFrameSourceBase::AddObserver(obs);
+void DelayBasedBeginFrameSource::AddObserver(BeginFrameObserver* obs) {
+  DCHECK(obs);
+  DCHECK(observers_.find(obs) == observers_.end());
+
+  DEBUG_FRAMES("DelayBasedBeginFrameSource::AddObserver", "num observers",
+               observers_.size(), "to add observer", obs);
+  observers_.insert(obs);
+  obs->OnBeginFrameSourcePausedChanged(false);
+  time_source_->SetActive(true);
   BeginFrameArgs args = CreateBeginFrameArgs(
       time_source_->NextTickTime() - time_source_->Interval(),
       BeginFrameArgs::MISSED);
@@ -213,23 +162,29 @@ void SyntheticBeginFrameSource::AddObserver(BeginFrameObserver* obs) {
   }
 }
 
-void SyntheticBeginFrameSource::OnNeedsBeginFramesChanged(
-    bool needs_begin_frames) {
-  time_source_->SetActive(needs_begin_frames);
+void DelayBasedBeginFrameSource::RemoveObserver(BeginFrameObserver* obs) {
+  DCHECK(obs);
+  DCHECK(observers_.find(obs) != observers_.end());
+
+  DEBUG_FRAMES("DelayBasedBeginFrameSource::RemoveObserver", "num observers",
+               observers_.size(), "removed observer", obs);
+  observers_.erase(obs);
+  if (observers_.empty())
+    time_source_->SetActive(false);
 }
 
-// DelayBasedTimeSourceClient support
-void SyntheticBeginFrameSource::OnTimerTick() {
+void DelayBasedBeginFrameSource::OnTimerTick() {
   BeginFrameArgs args = CreateBeginFrameArgs(time_source_->LastTickTime(),
                                              BeginFrameArgs::NORMAL);
+  DEBUG_FRAMES("DelayBasedBeginFrameSource::OnTimerTick", "num observers",
+               observers_.size(), "args", args.AsValue());
   std::set<BeginFrameObserver*> observers(observers_);
-  for (auto& it : observers) {
-    BeginFrameArgs last_args = it->LastUsedBeginFrameArgs();
+  for (auto& obs : observers) {
+    BeginFrameArgs last_args = obs->LastUsedBeginFrameArgs();
     if (!last_args.IsValid() ||
         (args.frame_time >
-         last_args.frame_time + args.interval / kDoubleTickDivisor)) {
-      it->OnBeginFrame(args);
-    }
+         last_args.frame_time + args.interval / kDoubleTickDivisor))
+      obs->OnBeginFrame(args);
   }
 }