Refactoring the way begin frame sources inside scheduler work.

cc/scheduler/frame_source.h

Index: cc/scheduler/frame_source.h
diff --git a/cc/scheduler/frame_source.h b/cc/scheduler/frame_source.h
new file mode 100644
index 0000000000000000000000000000000000000000..747b8003f16c22df0edb65e253cc937faf2c1727
--- /dev/null
+++ b/cc/scheduler/frame_source.h
@@ -0,0 +1,207 @@
+// 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.
+
+#ifndef CC_SCHEDULER_FRAME_SOURCE_H_
+#define CC_SCHEDULER_FRAME_SOURCE_H_
+
+#include <set>
+#include <string>
+
+#include "base/debug/trace_event.h"
+#include "base/logging.h"
+#include "cc/output/begin_frame_args.h"
+#include "cc/scheduler/delay_based_time_source.h"
+#include "ui/compositor/compositor_vsync_manager.h"
+#include "ui/gfx/frame_time.h"

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

Don't think frame_time.h needs to be included in this header.

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

Done.

+
+namespace cc {
+
+/**
+ *
+ */
+class CC_EXPORT VSyncParameterObserver

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

Would it be less code to just use ui::CompositorVSyncManager::Observer rather
than having another class that does almost the same thing?

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

Yeah, I think we can probably ditch this class now. It originally existed
because the scheduler used the vsync interval but that seems to have gone away
now.

+    : public ui::CompositorVSyncManager::Observer {
+ public:
+  VSyncParameterObserver(base::TimeTicks initial_vsync_timebase,
+                         base::TimeDelta initial_vsync_interval);
+  virtual ~VSyncParameterObserver();
+
+  virtual void OnUpdateVSyncParameters(
+      base::TimeTicks new_vsync_timebase,
+      base::TimeDelta new_vsync_interval) OVERRIDE;
+
+  virtual base::TimeTicks VSyncTimebase() const;
+  virtual base::TimeDelta VSyncInterval() const;
+
+  // Tracing support
+  virtual void AsValueInto(base::debug::TracedValue* dict) const;
+
+ protected:
+  virtual void OnTimeBaseAndIntervalChange(
+      const base::TimeTicks new_vsync_timebase,
+      const base::TimeDelta new_vsync_interval) {}
+
+ private:
+  base::TimeTicks vsync_timebase_;
+  base::TimeDelta vsync_interval_;
+};
+
+/**
+ *
+ */
+class CC_EXPORT BeginFrameObserver {
+ public:
+  virtual const BeginFrameArgs& LastBeginFrameArgs() const = 0;
+  virtual void OnBeginFrame(const BeginFrameArgs& args) = 0;
+
+  // Tracing support
+  virtual void AsValueInto(base::debug::TracedValue* dict) const = 0;
+};
+
+/**
+ *
+ */
+class CC_EXPORT BeginFrameSource {
+ public:
+  virtual ~BeginFrameSource() {}
+
+  // Should the frame source be sending begin frames?
+  virtual bool NeedsBeginFrames() const = 0;
+  virtual void SetNeedsBeginFrames(bool needs_begin_frames) = 0;
+  // This method provides backpressure to a frame source rather then toggling

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

then -> than

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

Done.

+  // SetGenerateFrames. It is used by systems like the BackToBackFrameSource to

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

Don't think you need to reference SetGenerateFrames since it's a concept that no
longer exists in your newest patch.

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

Done.

+  // make sure only one frame is pending at a time.
+  virtual void DidFinishFrame(size_t remaining_frames) = 0;
+
+  void AddObserver(BeginFrameObserver* obs);
+  void RemoveObserver(BeginFrameObserver* obs);
+
+  // Tracing support
+  virtual void AsValueInto(base::debug::TracedValue* dict) const = 0;
+
+ protected:
+  void SendBeginFrame(const BeginFrameArgs& args);
+  BeginFrameObserver* observer_ = NULL;
+
+ private:
+  bool inside_as_value_into_ = false;
+};
+
+/**
+ * A frame source which sends a BeginFrame as soon as remaining frames reaches
+ * zero.
+ */
+class CC_EXPORT BackToBackBeginFrameSource : public BeginFrameSource {
+ public:
+  static scoped_ptr<BackToBackBeginFrameSource> Create(
+      base::SingleThreadTaskRunner* task_runner);
+  virtual ~BackToBackBeginFrameSource();
+
+  virtual void SendBeginFrameArgs();
+
+  // BeginFrameSource
+  virtual bool NeedsBeginFrames() const OVERRIDE;
+  virtual void SetNeedsBeginFrames(bool needs_begin_frames) OVERRIDE;
+  virtual void DidFinishFrame(size_t remaining_frames) OVERRIDE;
+
+  // Tracing
+  virtual void AsValueInto(base::debug::TracedValue* dict) const OVERRIDE;
+
+ protected:
+  explicit BackToBackBeginFrameSource(
+      base::SingleThreadTaskRunner* task_runner);
+  virtual base::TimeTicks Now();  // Now overridable for testing
+
+  base::WeakPtrFactory<BackToBackBeginFrameSource> weak_factory_;
+  base::SingleThreadTaskRunner* task_runner_;
+
+  bool needs_begin_frames_ = false;
+  bool send_begin_frame_posted_ = false;
+
+  void ScheduleSendBeginFrameArgs();
+};
+
+/**
+ * A frame source which is locked an external vsync source and generates
+ * BeginFrameArgs for it.
+ */
+class CC_EXPORT SyntheticBeginFrameSource : public BeginFrameSource,
+                                            public VSyncParameterObserver,
+                                            public TimeSourceClient {
+ public:
+  static scoped_ptr<SyntheticBeginFrameSource> Create(
+      base::SingleThreadTaskRunner* task_runner,
+      base::TimeTicks initial_vsync_timebase,
+      base::TimeDelta initial_vsync_interval);
+  virtual ~SyntheticBeginFrameSource();
+
+  // TimeSourceClient
+  virtual void OnTimerTick() OVERRIDE;
+
+  // BeginFrameSource
+  virtual bool NeedsBeginFrames() const OVERRIDE;
+  virtual void SetNeedsBeginFrames(bool needs_begin_frames) OVERRIDE;
+  virtual void DidFinishFrame(size_t remaining_frames) OVERRIDE{};
+
+  // Tracing
+  virtual void AsValueInto(base::debug::TracedValue* dict) const OVERRIDE;
+
+ protected:
+  SyntheticBeginFrameSource(scoped_refptr<DelayBasedTimeSource> time_source,
+                            base::TimeTicks initial_vsync_timebase,
+                            base::TimeDelta initial_vsync_interval);
+
+  virtual void OnTimeBaseAndIntervalChange(

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

// VSyncParameterObserver

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

Done.

+      const base::TimeTicks new_vsync_timebase,
+      const base::TimeDelta new_vsync_interval) OVERRIDE;
+
+  void SendBeginFrameFromTick(base::TimeTicks frame_time);
+
+  scoped_refptr<DelayBasedTimeSource> time_source_;
+};
+
+/**
+ * A virtual frame source which lets you switch between multiple other frame
+ * sources while making sure the BeginFrameArgs stays increasing (possibly
+ * enforcing minimum boundry between BeginFrameArgs messages).
+ */
+class CC_EXPORT BeginFrameSourceMultiplexer : public BeginFrameSource,
+                                              public BeginFrameObserver {
+ public:
+  static scoped_ptr<BeginFrameSourceMultiplexer> Create();
+  virtual ~BeginFrameSourceMultiplexer();
+
+  void SetMinimumInterval(base::TimeDelta new_minimum_interval);
+
+  void AddSource(BeginFrameSource* new_source);
+  void RemoveSource(BeginFrameSource* existing_source);
+  void SetActiveSource(BeginFrameSource* new_source);
+  const BeginFrameSource* ActiveSource();
+
+  // BeginFrameObserver

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

It isn't immediately obvious why this needs to be an Observer. Can you add a
comment indicating that by acting as a observer proxy it can make sure the frame
times are monotonic.

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

Added comment.

+  virtual void OnBeginFrame(const BeginFrameArgs& args) OVERRIDE;
+  virtual const BeginFrameArgs& LastBeginFrameArgs() const OVERRIDE;
+
+  // BeginFrameSource
+  virtual bool NeedsBeginFrames() const OVERRIDE;
+  virtual void SetNeedsBeginFrames(bool needs_begin_frames) OVERRIDE;
+  virtual void DidFinishFrame(size_t remaining_frames) OVERRIDE;
+
+  // Tracing
+  virtual void AsValueInto(base::debug::TracedValue* dict) const OVERRIDE;
+
+ protected:
+  BeginFrameSourceMultiplexer();
+  explicit BeginFrameSourceMultiplexer(base::TimeDelta minimum_interval);
+  bool HasSource(BeginFrameSource* source);
+
+  base::TimeDelta minimum_interval_;
+
+  BeginFrameSource* active_source_;
+  std::set<BeginFrameSource*> source_list_;

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

Since this class doesn't take ownership anymore, it seems funny to track a
source_list.

Could you make this work without AddSource, RemoveSource, HasSource, and
source_list_?

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

I think having AddSource/RemoveSource/HasSource is kind of a nice sanity check
that you are using the right sources with the right Mux. We could making them
NOP operations for NDEBUG builds?

Can remove them if you feel strongly.

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

I don't feel strongly. Don't worry about NDEBUG.

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

Actually, should these be some type of weak_ptr? I don't quite understand when
the correct time/place to use a weak_ptr is...

+};
+
+}  // namespace cc
+
+#endif  // CC_SCHEDULER_FRAME_SOURCE_H_