Refactoring the way begin frame sources inside scheduler work.

cc/scheduler/begin_frame_source.h

+// 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_BEGIN_FRAME_SOURCE_H_
+#define CC_SCHEDULER_BEGIN_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/output/vsync_parameter_observer.h"
+#include "cc/scheduler/delay_based_time_source.h"
+
+namespace cc {
+
+// (Pure) Interface for observing BeginFrame messages from BeginFrameSource
+// objects.
+class CC_EXPORT BeginFrameObserver {
+ public:
+  // The |args| given to OnBeginFrame is guaranteed to have
+  // |args|.IsValid()==true and have |args|.frame_time
+  // field be strictly greater than the previous call.
+  //
+  // Side effects: This function can (and most of the time *will*) change the
+  // return value of the LastUsedBeginFrameArgs method. See the documentation
+  // on that method for more information.
+  virtual void OnBeginFrame(const BeginFrameArgs& args) = 0;
+
+  // Some frame sources *may* provide the last missed begin frame when
+  // SetNeedsBeginFrames is set true (such as the SyntheticBeginFrameSource).
+  //
+  // **Don't** depend on this function ever being called.
+  virtual void OnMissedBeginFrame(const BeginFrameArgs& args) = 0;
+
+  // Returns the last BeginFrameArgs used by the observer. This method's return
+  // value is affected by the OnBeginFrame method!
+  //
+  //  - Before the first call of OnBeginFrame, this method should return a
+  //    BeginFrameArgs on which IsValid() returns false.
+  //
+  //  - If the |args| passed to OnBeginFrame is (or *will be*) used, then
+  //    LastUsedBeginFrameArgs return value should become the |args| given to
+  //    OnBeginFrame.
+  //
+  //  - If the |args| passed to OnBeginFrame is dropped, then
+  //    LastUsedBeginFrameArgs return value should *not* change.
+  //
+  // These requirements are designed to allow chaining and nesting of
+  // BeginFrameObservers which filter the incoming BeginFrame messages while
+  // preventing "double dropping" and other bad side effects.
+  virtual const BeginFrameArgs LastUsedBeginFrameArgs() const = 0;
+
+  // Tracing support
+  virtual void AsValueInto(base::debug::TracedValue* dict) const = 0;
+};
+
+// Simple mix in which implements a BeginFrameObserver which checks the
+// incoming values meet the BeginFrameObserver requirements and implements the
+// required LastUsedBeginFrameArgs behaviour.
+//
+// Users of this mix in should;
+//  - Implement the ProcessBeginFrameArgs function.
+//  - Recommended (but not required) to call
+//    BeginFrameObserverMixIn::OnValueInto in their overridden OnValueInto
+//    function.
+//
+// Advance observers may also wish to override the OnMissedBeginFrame method.
+class CC_EXPORT BeginFrameObserverMixIn : public BeginFrameObserver {
+ public:
+  // BeginFrameObserver
+  BeginFrameObserverMixIn();
+
+  // Traces |args| and DCHECK |args| satisfies pre-conditions then calls
+  // ProcessBeginFrameArgs and updates the last_begin_frame_args_ value on true.
+  virtual void OnBeginFrame(const BeginFrameArgs& args) OVERRIDE;
+
+  // Forwards call to the OnBeginFrame callback.
+  virtual void OnMissedBeginFrame(const BeginFrameArgs& args) OVERRIDE;
+
+  virtual const BeginFrameArgs LastUsedBeginFrameArgs() const OVERRIDE;
+
+  // Outputs last_begin_frame_args_
+  virtual void AsValueInto(base::debug::TracedValue* dict) const OVERRIDE;
+
+ protected:
+  // Subclasses should override this method!
+  // Return true if the given argument is (or will be) used.
+  virtual bool ProcessBeginFrameArgs(const BeginFrameArgs& args) = 0;
+  virtual bool ProcessMissedBeginFrameArgs(const BeginFrameArgs& args);

[brianderson, 2014/09/24 22:45:38]

Was imagining this would be pure virtual.

Also, I feel like Process***Args doesn't imply as strong a link between
BeginFrame/OnBeginFrame as there is. Maybe OnBeginFrameMixInDelegate or
OnBeginFrameDelegate would imply a stronger link without conflicting?

[danakj, 2014/09/24 23:35:24]

+1. Please avoid situations where we have non-pure virtual methods that are
overridden by another class.

[danakj, 2014/09/24 23:59:25]

(Or classes that mix pure virtual and concrete virtual methods, if possible.
Pure virtual interfaces are great, but things get more complex when they
implement methods/have members also.)

[mithro-old, 2014/09/25 01:53:00]

I'd really like implementation of this method to be totally optional to help
reinforce that this functionality may not be enabled for a given frame source.
Naming things is hard, these names are long but better. Changed.

[mithro-old, 2014/09/25 01:53:00]

How do you provide "default" implementations without doing this? For example, we
want missed begin frame support to be totally optional.

I also split the dropped count for each type of message.

[danakj, 2014/09/27 00:05:34]

Brian and I talked about this some more the other day and I think we got to a
pretty reasonable conclusion. Maybe an is-a relationship really makes sense here
so you want to use inheritance instead of composition. In that case please keep
the interface (class with pure virtual methods) as entirely an interface, don't
add anything concrete to it, virtual or not. You can have a concrete subclass
add some default implementations for other things to inherit and use if needed,
but I think it will help a lot to keep our interfaces pure, with no concrete
methods in them.

+
+  BeginFrameArgs last_begin_frame_args_;
+  int64_t dropped_begin_frame_args_;
+};
+
+// Interface for a class which produces BeginFrame calls to a
+// BeginFrameObserver.
+//
+// BeginFrame calls *normally* occur just after a vsync interrupt when input
+// processing has been finished and provide information about the time values
+// of the vsync times. *However*, these values can be heavily modified or even
+// plain made up (when no vsync signal is available or vsync throttling is
+// turned off). See the BeginFrameObserver for information about the guarantees
+// all BeginFrameSources *must* provide.
+//
+// Implementation classes should:
+//  - Use the CallOn(Missed)BeginFrame method to call to the observer(s).
+//  - Implement the pure virtual methods.
+//  - Recommend (but not required) to call BeginFrameSource::AsValueInto in
+//    their own AsValueInto implementation.
+class CC_EXPORT BeginFrameSource {
+ public:
+  virtual ~BeginFrameSource() {}
+
+  // SetNeedsBeginFrames is the on/off "switch" for the BeginFrameSource. When
+  // set to false no more BeginFrame messages should be sent to observer.
+  virtual bool NeedsBeginFrames() const = 0;
+  virtual void SetNeedsBeginFrames(bool needs_begin_frames) = 0;
+
+  // DidFinishFrame provides back pressure to a frame source about frame
+  // processing (rather than toggling SetNeedsBeginFrames every frame). 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;
+
+  // Add/Remove and observer from the source.
+  // *At the moment* only a single observer can be added to the source, however
+  // in the future this may be extended to allow multiple observers.
+  // If making this change, please use base::ObserverList to do so.
+  void AddObserver(BeginFrameObserver* obs);
+  void RemoveObserver(BeginFrameObserver* obs);
+
+  // Tracing support - Recommend (but not required) to call this implementation
+  // in any override.
+  virtual void AsValueInto(base::debug::TracedValue* dict) const;
+
+ protected:
+  BeginFrameSource();
+
+  // These methods should be used by subclasses to make the call to the
+  // observers.
+  void CallOnBeginFrame(const BeginFrameArgs& args);
+  void CallOnMissedBeginFrame(const BeginFrameArgs& args);
+
+  BeginFrameObserver* observer_;
+
+ private:
+  bool inside_as_value_into_;
+};
+
+// A frame source which calls BeginFrame (at the next possible time) 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();
+
+  // 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_;
+  bool send_begin_frame_posted_;
+
+  void ScheduleBeginFrame();
+  void BeginFrame();
+};
+
+// A frame source which is locked to an external parameters provides from a
+// 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();
+
+  // 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;
+
+  // VSyncParameterObserver
+  virtual void OnUpdateVSyncParameters(
+      base::TimeTicks new_vsync_timebase,
+      base::TimeDelta new_vsync_interval) OVERRIDE;
+
+  // TimeSourceClient
+  virtual void OnTimerTick() OVERRIDE;
+
+ protected:
+  explicit SyntheticBeginFrameSource(
+      scoped_refptr<DelayBasedTimeSource> time_source);
+
+  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
+  // The mux is an BeginFrameObserver as it needs to proxy the OnBeginFrame
+  // calls to preserve the monotonicity of the BeginFrameArgs when switching
+  // sources.
+  virtual void OnBeginFrame(const BeginFrameArgs& args) OVERRIDE;
+  virtual void OnMissedBeginFrame(const BeginFrameArgs& args) OVERRIDE;
+  virtual const BeginFrameArgs LastUsedBeginFrameArgs() 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);
+  bool IsIncreasing(const BeginFrameArgs& args);
+
+  base::TimeDelta minimum_interval_;
+
+  BeginFrameSource* active_source_;
+  std::set<BeginFrameSource*> source_list_;
+};
+
+}  // namespace cc
+
+#endif  // CC_SCHEDULER_BEGIN_FRAME_SOURCE_H_