cc: Make separate interface for BeginFrame ipc from OutputSurface

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>
 
@@ skipping 99 matching lines @@
   // is pending at a time.
   virtual void DidFinishFrame(size_t remaining_frames) = 0;
 
   // Add/Remove an 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.
   virtual void AddObserver(BeginFrameObserver* obs) = 0;
   virtual void RemoveObserver(BeginFrameObserver* obs) = 0;
 
+  // Tells the Source that client is ready to handle BeginFrames messages.
+  virtual void SetClientReady() = 0;

[brianderson, 2014/10/23 02:12:44]

Anyway we can avoid having this method as part of the BeginFrameSource
interface? It's only needed for external sources to setup message routing. Is
there a different hook we can use?

[simonhong, 2014/10/24 14:30:18]

At first, I created ExternalBeginFrameSource deriving BeginFrameSourceMixIn.
It has SetClientReady() as virtual abstract method.
But, danakj@ pointed out that BeginFrameSourceMixIn should not be a base class
of abstract class.
Instead, danakj@ suggested adding SetClientReady() into BeginFrameSource.

Other option is adding SetClientReady() into BFSMixIn.
But I think all of Source inherits BFSMixIn so, it is same as current.

In this case, what is the best solution? :)

[brianderson, 2014/10/24 20:46:58]

Ok, what you have now is good then. Any other solution I can think of is going
to add more boilerplate than is really worth it.

[simonhong, 2014/10/29 14:47:13]

Yep!

+
   // Tracing support - Recommend (but not required) to call this implementation
   // in any override.
   virtual void AsValueInto(base::debug::TracedValue* dict) const = 0;
 };
 
 // Simple mix in which implements a BeginFrameSource.
 // Implementation classes should:
 //  - Implement the pure virtual (Set)NeedsBeginFrames methods from
 //    BeginFrameSource.
 //  - Use the CallOnBeginFrame method to call to the observer(s).
 //  - Recommended (but not required) to call BeginFrameSourceMixIn::AsValueInto
 //    in their own AsValueInto implementation.
 class CC_EXPORT BeginFrameSourceMixIn : public BeginFrameSource {
  public:
   ~BeginFrameSourceMixIn() override {}
 
   // BeginFrameSource
   bool NeedsBeginFrames() const override;
   void SetNeedsBeginFrames(bool needs_begin_frames) override;
   void DidFinishFrame(size_t remaining_frames) override {}
   void AddObserver(BeginFrameObserver* obs) override;
   void RemoveObserver(BeginFrameObserver* obs) override;
+  void SetClientReady() override {}

[brianderson, 2014/10/23 02:12:44]

Delete this line so we do not override SetClientReady() and all subclasses must
implement it.

The other implementations above are expected to be the same for all subclasses,
which is why they aren't virtual.

[brianderson, 2014/10/24 20:46:58]

Ignore my comment here. I didn't know that override implies virtual before, but
I do now. Also, inheriting an empty implementation should be fine in this case.

It'll probably be a good idea to make NeedsBeginFrames, SetNeedsBeginFrames,
AddObserver, and RemoveObserver "final" though since MixIn subclasses shouldn't
override them.

[simonhong, 2014/10/29 14:47:12]

Some of them are still used by its subclass.
AddObserver & RemoveObserver only can be final candidate. 

[brianderson, 2014/11/01 01:08:32]

Hmm, NeedsBeginFrames and SetNeedsBeginFrames should also be able to be final
since BeginFrameSourceMixIn has OnNeedsBeginFramesChange that it expects
subclasses to implement. Don't worry about cleaning it up in this patch though,
I've opened crbug.com/429391 for followup.

 
   // Tracing support - Recommend (but not required) to call this implementation
   // in any override.
   void AsValueInto(base::debug::TracedValue* dict) const override;
 
  protected:
   BeginFrameSourceMixIn();
 
   // These methods should be used by subclasses to make the call to the
   // observers.
@@ skipping 118 matching lines @@
 
   base::TimeDelta minimum_interval_;
 
   BeginFrameSource* active_source_;
   std::set<BeginFrameSource*> source_list_;
 };
 
 }  // namespace cc
 
 #endif  // CC_SCHEDULER_BEGIN_FRAME_SOURCE_H_