exo: Improve frame callbacks.

Use the compositor frame ack instead of begin-frame callback to
trigger these callbacks. The frame ack is sufficient to prevent
frames from being dropped, and provides minimal latency
back-pressure for clients that implement more advanced
scheduling.

This makes it possible to improve scheduling of frames for
Linux apps significantly. Sommelier can have the next frame
prepared as a dmabuf and ready to commit when we receive the
callback. This minimizes the chance that we'll miss a frame.

BUG=845659
TEST=exo_unittests --gtest_filter=SurfaceTest.RequestFrameCallback
TEST=sommelier /usr/lib/weston/weston-simple-shm
TEST=sommelier -X glxgears

Change-Id: I20f06618d9d550ab501f3945b3d25d4a5c20c218
Reviewed-on: https://chromium-review.googlesource.com/1097776Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567301}

components/exo/layer_tree_frame_sink_holder.cc

@@ -114,12 +114,6 @@ base::WeakPtr<LayerTreeFrameSinkHolder> LayerTreeFrameSinkHolder::GetWeakPtr() {
 ////////////////////////////////////////////////////////////////////////////////
 // cc::LayerTreeFrameSinkClient overrides:
 
-void LayerTreeFrameSinkHolder::SetBeginFrameSource(
-    viz::BeginFrameSource* source) {
-  if (surface_tree_host_)
-    surface_tree_host_->SetBeginFrameSource(source);
-}
-
 base::Optional<viz::HitTestRegionList>
 LayerTreeFrameSinkHolder::BuildHitTestData() {
   return {};

components/exo/layer_tree_frame_sink_holder.h

@@ -50,7 +50,7 @@ class LayerTreeFrameSinkHolder : public cc::LayerTreeFrameSinkClient,
   base::WeakPtr<LayerTreeFrameSinkHolder> GetWeakPtr();
 
   // Overridden from cc::LayerTreeFrameSinkClient:
-  void SetBeginFrameSource(viz::BeginFrameSource* source) override;
+  void SetBeginFrameSource(viz::BeginFrameSource* source) override {}
   base::Optional<viz::HitTestRegionList> BuildHitTestData() override;
   void ReclaimResources(
       const std::vector<viz::ReturnedResource>& resources) override;

components/exo/surface_tree_host.cc

@@ -121,13 +121,11 @@ void SurfaceTreeHost::SetRootSurface(Surface* root_surface) {
     root_surface_->SurfaceHierarchyResourcesLost();
     root_surface_ = nullptr;
 
-    active_frame_callbacks_.splice(active_frame_callbacks_.end(),
-                                   frame_callbacks_);
     // Call all frame callbacks with a null frame time to indicate that they
     // have been cancelled.
-    while (!active_frame_callbacks_.empty()) {
-      active_frame_callbacks_.front().Run(base::TimeTicks());
-      active_frame_callbacks_.pop_front();
+    while (!frame_callbacks_.empty()) {
+      frame_callbacks_.front().Run(base::TimeTicks());
+      frame_callbacks_.pop_front();
     }
 
     DCHECK(presentation_callbacks_.empty());
@@ -159,9 +157,10 @@ void SurfaceTreeHost::GetHitTestMask(gfx::Path* mask) const {
 }
 
 void SurfaceTreeHost::DidReceiveCompositorFrameAck() {
-  active_frame_callbacks_.splice(active_frame_callbacks_.end(),
-                                 frame_callbacks_);
-  UpdateNeedsBeginFrame();
+  while (!frame_callbacks_.empty()) {
+    frame_callbacks_.front().Run(base::TimeTicks::Now());
+    frame_callbacks_.pop_front();
+  }
 }
 
 void SurfaceTreeHost::DidPresentCompositorFrame(
@@ -174,30 +173,6 @@ void SurfaceTreeHost::DidPresentCompositorFrame(
   active_presentation_callbacks_.erase(it);
 }
 
-void SurfaceTreeHost::SetBeginFrameSource(
-    viz::BeginFrameSource* begin_frame_source) {
-  if (needs_begin_frame_) {
-    DCHECK(begin_frame_source_);
-    begin_frame_source_->RemoveObserver(this);
-    needs_begin_frame_ = false;
-  }
-  begin_frame_source_ = begin_frame_source;
-  UpdateNeedsBeginFrame();
-}
-
-void SurfaceTreeHost::UpdateNeedsBeginFrame() {
-  if (!begin_frame_source_)
-    return;
-  bool needs_begin_frame = !active_frame_callbacks_.empty();
-  if (needs_begin_frame == needs_begin_frame_)
-    return;
-  needs_begin_frame_ = needs_begin_frame;
-  if (needs_begin_frame_)
-    begin_frame_source_->AddObserver(this);
-  else
-    begin_frame_source_->RemoveObserver(this);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // SurfaceDelegate overrides:
 
@@ -217,31 +192,6 @@ bool SurfaceTreeHost::IsInputEnabled(Surface*) const {
   return true;
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// cc::BeginFrameObserverBase overrides:
-
-bool SurfaceTreeHost::OnBeginFrameDerivedImpl(const viz::BeginFrameArgs& args) {
-  current_begin_frame_ack_ =
-      viz::BeginFrameAck(args.source_id, args.sequence_number, false);
-
-  if (!frame_callbacks_.empty()) {
-    // In this case, the begin frame arrives just before
-    // |DidReceivedCompositorFrameAck()|, we need more begin frames to run
-    // |frame_callbacks_| which will be moved to |active_frame_callbacks_| by
-    // |DidReceivedCompositorFrameAck()| shortly.
-    layer_tree_frame_sink_holder_->DidNotProduceFrame(current_begin_frame_ack_);
-    current_begin_frame_ack_.sequence_number =
-        viz::BeginFrameArgs::kInvalidFrameNumber;
-    begin_frame_source_->DidFinishFrame(this);
-  }
-
-  while (!active_frame_callbacks_.empty()) {
-    active_frame_callbacks_.front().Run(args.frame_time);
-    active_frame_callbacks_.pop_front();
-  }
-  return true;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // ui::ContextFactoryObserver overrides:
 
@@ -258,15 +208,8 @@ void SurfaceTreeHost::OnLostResources() {
 void SurfaceTreeHost::SubmitCompositorFrame() {
   DCHECK(root_surface_);
   viz::CompositorFrame frame;
-  // If we commit while we don't have an active BeginFrame, we acknowledge a
-  // manual one.
-  if (current_begin_frame_ack_.sequence_number ==
-      viz::BeginFrameArgs::kInvalidFrameNumber) {
-    current_begin_frame_ack_ = viz::BeginFrameAck::CreateManualAckWithDamage();
-  } else {
-    current_begin_frame_ack_.has_damage = true;
-  }
-  frame.metadata.begin_frame_ack = current_begin_frame_ack_;
+  frame.metadata.begin_frame_ack =
+      viz::BeginFrameAck::CreateManualAckWithDamage();
   root_surface_->AppendSurfaceHierarchyCallbacks(&frame_callbacks_,
                                                  &presentation_callbacks_);
   if (!presentation_callbacks_.empty()) {
@@ -315,18 +258,6 @@ void SurfaceTreeHost::SubmitCompositorFrame() {
   }
 
   layer_tree_frame_sink_holder_->SubmitCompositorFrame(std::move(frame));
-
-  if (current_begin_frame_ack_.sequence_number !=
-      viz::BeginFrameArgs::kInvalidFrameNumber) {
-    if (!current_begin_frame_ack_.has_damage) {
-      layer_tree_frame_sink_holder_->DidNotProduceFrame(
-          current_begin_frame_ack_);
-    }
-    current_begin_frame_ack_.sequence_number =
-        viz::BeginFrameArgs::kInvalidFrameNumber;
-    if (begin_frame_source_)
-      begin_frame_source_->DidFinishFrame(this);
-  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////

components/exo/surface_tree_host.h

@@ -11,7 +11,6 @@
 #include "components/exo/layer_tree_frame_sink_holder.h"
 #include "components/exo/surface.h"
 #include "components/exo/surface_delegate.h"
-#include "components/viz/common/frame_sinks/begin_frame_source.h"
 #include "ui/gfx/geometry/rect.h"
 
 namespace aura {
@@ -22,17 +21,12 @@ namespace gfx {
 class Path;
 }  // namespace gfx
 
-namespace viz {
-class BeginFrameSource;
-}  // namespace viz
-
 namespace exo {
 class LayerTreeFrameSinkHolder;
 
 // This class provides functionality for hosting a surface tree. The surface
 // tree is hosted in the |host_window_|.
 class SurfaceTreeHost : public SurfaceDelegate,
-                        public viz::BeginFrameObserverBase,
                         public ui::ContextFactoryObserver {
  public:
   explicit SurfaceTreeHost(const std::string& window_name);
@@ -58,12 +52,6 @@ class SurfaceTreeHost : public SurfaceDelegate,
   void DidPresentCompositorFrame(uint32_t presentation_token,
                                  const gfx::PresentationFeedback& feedback);
 
-  // Called when the begin frame source has changed.
-  void SetBeginFrameSource(viz::BeginFrameSource* begin_frame_source);
-
-  // Adds/Removes begin frame observer based on state.
-  void UpdateNeedsBeginFrame();
-
   aura::Window* host_window() { return host_window_.get(); }
   const aura::Window* host_window() const { return host_window_.get(); }
 
@@ -87,10 +75,6 @@ class SurfaceTreeHost : public SurfaceDelegate,
   void OnSetStartupId(const char* startup_id) override {}
   void OnSetApplicationId(const char* application_id) override {}
 
-  // Overridden from cc::BeginFrameObserverBase:
-  bool OnBeginFrameDerivedImpl(const viz::BeginFrameArgs& args) override;
-  void OnBeginFrameSourcePausedChanged(bool paused) override {}
-
   // Overridden from ui::ContextFactoryObserver:
   void OnLostResources() override;
 
@@ -110,18 +94,11 @@ class SurfaceTreeHost : public SurfaceDelegate,
   std::unique_ptr<aura::Window> host_window_;
   std::unique_ptr<LayerTreeFrameSinkHolder> layer_tree_frame_sink_holder_;
 
-  // The begin frame source being observed.
-  viz::BeginFrameSource* begin_frame_source_ = nullptr;
-  bool needs_begin_frame_ = false;
-  viz::BeginFrameAck current_begin_frame_ack_;
-
-  // These lists contain the callbacks to notify the client when it is a good
+  // This list contains the callbacks to notify the client when it is a good
   // time to start producing a new frame. These callbacks move to
-  // |frame_callbacks_| when Commit() is called. Later they are moved to
-  // |active_frame_callbacks_| when the effect of the Commit() is scheduled to
-  // be drawn. They fire at the first begin frame notification after this.
+  // |frame_callbacks_| when Commit() is called. They fire when the effect
+  // of the Commit() is scheduled to be drawn.
   std::list<Surface::FrameCallback> frame_callbacks_;
-  std::list<Surface::FrameCallback> active_frame_callbacks_;
 
   // These lists contains the callbacks to notify the client when surface
   // contents have been presented.

components/exo/surface_unittest.cc

@@ -812,44 +812,6 @@ TEST_P(SurfaceTest, Commit) {
   surface->Commit();
 }
 
-TEST_P(SurfaceTest, SendsBeginFrameAcks) {
-  viz::FakeExternalBeginFrameSource source(0.f, false);
-  gfx::Size buffer_size(1, 1);
-  auto buffer = std::make_unique<Buffer>(
-      exo_test_helper()->CreateGpuMemoryBuffer(buffer_size), GL_TEXTURE_2D, 0,
-      true, true);
-  auto surface = std::make_unique<Surface>();
-  auto shell_surface = std::make_unique<ShellSurface>(surface.get());
-  shell_surface->SetBeginFrameSource(&source);
-  surface->Attach(buffer.get());
-
-  // Request a frame callback so that Surface now needs BeginFrames.
-  base::TimeTicks frame_time;
-  surface->RequestFrameCallback(
-      base::Bind(&SetFrameTime, base::Unretained(&frame_time)));
-  surface->Commit();  // Move callback from pending callbacks to current ones.
-  RunAllPendingInMessageLoop();
-
-  // Surface should add itself as observer during
-  // DidReceiveCompositorFrameAck().
-  shell_surface->DidReceiveCompositorFrameAck();
-  EXPECT_EQ(1u, source.num_observers());
-
-  viz::BeginFrameArgs args(source.CreateBeginFrameArgs(BEGINFRAME_FROM_HERE));
-  args.frame_time = base::TimeTicks::FromInternalValue(100);
-  source.TestOnBeginFrame(args);  // Runs the frame callback.
-  EXPECT_EQ(args.frame_time, frame_time);
-
-  surface->Commit();  // Acknowledges the BeginFrame via CompositorFrame.
-  RunAllPendingInMessageLoop();
-
-  const viz::CompositorFrame& frame = GetFrameFromSurface(shell_surface.get());
-  viz::BeginFrameAck expected_ack(args.source_id, args.sequence_number, true);
-  EXPECT_EQ(expected_ack, frame.metadata.begin_frame_ack);
-
-  // TODO(eseckler): Add test for DidNotProduceFrame plumbing.
-}
-
 TEST_P(SurfaceTest, RemoveSubSurface) {
   gfx::Size buffer_size(256, 256);
   std::unique_ptr<Buffer> buffer(