Unified BeginFrame scheduling

ui/compositor/compositor.h

 // Copyright (c) 2012 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 UI_COMPOSITOR_COMPOSITOR_H_
 #define UI_COMPOSITOR_COMPOSITOR_H_
 
 #include <string>
 
 #include "base/containers/hash_tables.h"
@@ skipping 12 matching lines @@
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/size.h"
 #include "ui/gfx/vector2d.h"
 
 namespace base {
 class MessageLoopProxy;
 class RunLoop;
 }
 
 namespace cc {
+class BeginFrameManager;
 class ContextProvider;
 class Layer;
 class LayerTreeDebugState;
 class LayerTreeHost;
 class SharedBitmapManager;
 }
 
 namespace gfx {
 class Rect;
 class Size;
 }
 
 namespace gpu {
 struct Mailbox;
 }
 
 namespace ui {
 
 class Compositor;
-class CompositorVSyncManager;
 class Layer;
 class Reflector;
 class Texture;
 struct LatencyInfo;
 
 // This class abstracts the creation of the 3D context for the compositor. It is
 // a global object.
 class COMPOSITOR_EXPORT ContextFactory {
  public:
   virtual ~ContextFactory() {}
@@ skipping 114 matching lines @@
   // Returns the size of the widget that is being drawn to in pixel coordinates.
   const gfx::Size& size() const { return size_; }
 
   // Sets the background color used for areas that aren't covered by
   // the |root_layer|.
   void SetBackgroundColor(SkColor color);
 
   // Returns the widget for this compositor.
   gfx::AcceleratedWidget widget() const { return widget_; }
 
-  // Returns the vsync manager for this compositor.
-  scoped_refptr<CompositorVSyncManager> vsync_manager() const;
+  // See cc::BeginFrameManager::Delegate comment.
+  void SetAuthoritativeVSyncInterval(base::TimeDelta interval) const;
+
+  cc::BeginFrameManager* begin_frame_manager() const {

[brianderson, 2014/08/28 02:48:07]

Instead of exposing the BeginFrameManager to everyone, I think it would be
cleaner to only expose it through forwarding methods instead.

[simonhong, 2014/09/03 21:02:23]

BeginFrameManager is removed.
Instead, {Add|Remove}BeginFrameObserver() is added and will be used
by RWHV.

+    return begin_frame_manager_.get();
+  }
 
   // Compositor does not own observers. It is the responsibility of the
   // observer to remove itself when it is done observing.
   void AddObserver(CompositorObserver* observer);
   void RemoveObserver(CompositorObserver* observer);
   bool HasObserver(CompositorObserver* observer);
 
   void AddAnimationObserver(CompositorAnimationObserver* observer);
   void RemoveAnimationObserver(CompositorAnimationObserver* observer);
   bool HasAnimationObserver(CompositorAnimationObserver* observer);
@@ skipping 61 matching lines @@
 
   ObserverList<CompositorObserver> observer_list_;
   ObserverList<CompositorAnimationObserver> animation_observer_list_;
 
   gfx::AcceleratedWidget widget_;
   scoped_refptr<cc::Layer> root_web_layer_;
   scoped_ptr<cc::LayerTreeHost> host_;
   scoped_refptr<base::MessageLoopProxy> compositor_thread_loop_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
-  // The manager of vsync parameters for this compositor.
-  scoped_refptr<CompositorVSyncManager> vsync_manager_;
+  // The manager of BeginFrame routing.
+  scoped_ptr<cc::BeginFrameManager> begin_frame_manager_;

[brianderson, 2014/08/28 02:48:07]

Would it make sense for the LTH or Proxy to own the BeginFrameManager instead? I
don't think all platforms use ui::Compositor.

[simonhong, 2014/08/29 00:03:01]

My plan was making BeginFrameManager as a member variable of each
compositor(ui::Compositor and CompositorImpl).
After seeing your this and above comment, each compositor only needs to have
BeginFrameObserver that manage RWHV.
After removing BeginFrameManager, current BeginFrameManger::Delegate's api
should
move into LTH. Then, only interaction between compositor and LTH is needed.
With this, we don't need to expose Scheduler to compositor as you said and all
requests
(ex, SetChildrenNeedsBeginFrame, authoritative, StartBeginFrame) will be
delivered to scheduler
only through LTH. (StartBeginFrame will be used to call scheduler's BeginFrame()
when vsync is triggered on Mac and Android)
What do you think about this change?

 
   // The device scale factor of the monitor that this compositor is compositing
   // layers on.
   float device_scale_factor_;
 
   int last_started_frame_;
   int last_ended_frame_;
 
   bool disable_schedule_composite_;
 
   CompositorLock* compositor_lock_;
 
   LayerAnimatorCollection layer_animator_collection_;
 
   DISALLOW_COPY_AND_ASSIGN(Compositor);
 };
 
 }  // namespace ui
 
 #endif  // UI_COMPOSITOR_COMPOSITOR_H_