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..d4c0e5013aaab199e3e0ec4dbe850f1d5448562c
--- /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_

[Sami, 2014/09/18 13:55:08]

nit: I'd call this begin_frame_source.h since that matches the classes.

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

Is this suggestion to rename the file?

[Sami, 2014/09/19 13:44:22]

Yes, sorry for not being clear. In my mind BeginFrame and frame are very
different concepts but that may just be me :)

[mithro-old, 2014/09/23 12:43:54]

I'm going to regret this but,

I was just pondering if the "BeginFrameSource/BeginFrameObserver" interface
should be in cc/output with BeginFrameArgs?

+#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"
+
+namespace cc {
+
+/**

[Sami, 2014/09/18 13:55:08]

Missing some documentation here?

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

This class has gone.

+ *
+ */
+class CC_EXPORT VSyncParameterObserver
+    : 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_;
+};
+
+/**
+ *

[Sami, 2014/09/18 13:55:08]

Ditto.

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

Done.

I've expanded the documentation substantially to document all the preconditions
that an Observer can expect.

From what I can see, Chromium style prefers // style comments, so I also made
all comments of that format.

+ */
+class CC_EXPORT BeginFrameObserver {
+ public:
+  virtual const BeginFrameArgs& LastBeginFrameArgs() const = 0;

[Sami, 2014/09/18 13:55:08]

If I understood correctly, this is mostly used for testing and tracing. Only the
multiplexer seems to use this for actual functionality, right? If this is the
case, it feels like this shouldn't be part of the observer interface and the
multiplexer should just keep a local copy of the most recent BeginFrame it got.
I think this may help to simplify the tests since the mock can just have the
last args as a field. WDYT?

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

So, I had a chat with Brian about this topic this morning. Initially he was
skeptical but I believe he has come around to the idea.

I've added a bunch of documentation to this class which should explain why this
method exists. Can you read that and see if your convinced? (It not then I need
to update the documentation I guess).

I agree that this method does make the mock a lot more complicated and wish
there was an easier way but C++ does really limits your options. (I long for the
days of just quickly monkey patching something for testing :)

+  virtual void OnBeginFrame(const BeginFrameArgs& args) = 0;
+
+  // Tracing support
+  virtual void AsValueInto(base::debug::TracedValue* dict) const = 0;
+};
+
+/**
+ *

[Sami, 2014/09/18 13:55:08]

Ditto.

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

Done.

+ */
+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
+  // SetGenerateFrames. It is used by systems like the BackToBackFrameSource to
+  // 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 CallOnBeginFrame(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(
+      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
+  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_;
+};
+
+}  // namespace cc
+
+#endif  // CC_SCHEDULER_FRAME_SOURCE_H_