[cc] Plumb BeginFrameAcks through SurfaceManager to DisplayScheduler.

content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Index: content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
index 3bbc617245152047df8e1be9c181df80bf8e1434..682b59d7ea1a22f840ab9f8b493fedb1c17b2c8f 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
@@ -32,6 +32,7 @@
 #include "cc/surfaces/surface_manager.h"
 #include "cc/test/begin_frame_args_test.h"
 #include "cc/test/fake_external_begin_frame_source.h"
+#include "cc/test/test_begin_frame_ack_tracker.h"
 #include "components/viz/display_compositor/gl_helper.h"
 #include "components/viz/display_compositor/host_shared_bitmap_manager.h"
 #include "content/browser/browser_thread_impl.h"
@@ -283,8 +284,13 @@ class TestWindowObserver : public aura::WindowObserver {
 class FakeSurfaceObserver : public cc::SurfaceObserver {
  public:
   void OnSurfaceCreated(const cc::SurfaceInfo& surface_info) override {}
+  void OnSurfaceDestroyed(const cc::SurfaceId& surface_id) override {}
+  void OnSurfaceDamageExpected(const cc::SurfaceId& surface_id,
+                               const cc::BeginFrameArgs& args) override {}
 
-  void OnSurfaceDamaged(const cc::SurfaceId& id, bool* changed) override {
+  void OnSurfaceDamaged(const cc::SurfaceId& id,
+                        const cc::BeginFrameAck& ack,
+                        bool* changed) override {
     *changed = true;
   }
 
@@ -3298,21 +3304,120 @@ TEST_F(RenderWidgetHostViewAuraTest, SourceEventTypeExistsInLatencyInfo) {
   view_->OnTouchEvent(&release);
 }
 
-namespace {
-class LastObserverTracker : public cc::FakeExternalBeginFrameSource::Client {
- public:
-  void OnAddObserver(cc::BeginFrameObserver* obs) override {
-    last_observer_ = obs;
+// Tests that BeginFrameAcks are forwarded correctly from the

[sunnyps, 2017/05/25 20:49:14]

nit: The only extra behavior that's tested here is about latest confirmed
sequence numbers. Maybe ok to keep for now but we'll remove it along with LCFS.

[Eric Seckler, 2017/05/26 10:57:52]

It also tests that DidNotProduceFrame is called while DFH is locked. I'll keep
it for now and we can simplify when LCSN goes away.

+// SwapCompositorFrame and OnDidNotProduceFrame IPCs through DelegatedFrameHost
+// and its CompositorFrameSinkSupport.
+TEST_F(RenderWidgetHostViewAuraTest, ForwardsBeginFrameAcks) {
+  gfx::Rect view_rect(100, 100);
+  gfx::Size frame_size = view_rect.size();
+  cc::LocalSurfaceId local_surface_id = kArbitraryLocalSurfaceId;
+
+  view_->InitAsChild(nullptr);
+  aura::client::ParentWindowWithContext(
+      view_->GetNativeView(), parent_view_->GetNativeView()->GetRootWindow(),
+      gfx::Rect());
+  view_->SetSize(view_rect.size());
+
+  cc::TestBeginFrameAckTracker ack_tracker;
+  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+  cc::SurfaceManager* surface_manager =
+      factory->GetContextFactoryPrivate()->GetSurfaceManager();
+  surface_manager->AddObserver(&ack_tracker);
+
+  view_->SetNeedsBeginFrames(true);
+  uint32_t source_id = 10;
+
+  {
+    // Ack from CompositorFrame is forwarded.
+    cc::BeginFrameAck ack(source_id, 5, 4, true);
+    cc::CompositorFrame frame = MakeDelegatedFrame(1.f, frame_size, view_rect);
+    frame.metadata.begin_frame_ack = ack;
+    view_->SubmitCompositorFrame(local_surface_id, std::move(frame));
+    view_->RunOnCompositingDidCommit();
+    EXPECT_EQ(ack, ack_tracker.last_ack());
   }
-  void OnRemoveObserver(cc::BeginFrameObserver* obs) override {}
 
-  cc::BeginFrameObserver* last_observer_ = nullptr;
-};
-}  // namespace
+  {
+    // Explicit ack through OnDidNotProduceFrame is forwarded.
+    cc::BeginFrameAck ack(source_id, 6, 4, false);
+    view_->OnDidNotProduceFrame(ack);
+    EXPECT_EQ(ack, ack_tracker.last_ack());
+  }
+
+  // Lock the compositor. Now we should drop frames and, thus,
+  // latest_confirmed_sequence_number should not change.
+  view_rect = gfx::Rect(150, 150);
+  view_->SetSize(view_rect.size());
+
+  {
+    // Ack from CompositorFrame is forwarded with old
+    // latest_confirmed_sequence_number and without damage.
+    cc::BeginFrameAck ack(source_id, 7, 7, true);
+    gfx::Rect dropped_damage_rect(10, 20, 30, 40);
+    cc::CompositorFrame frame =
+        MakeDelegatedFrame(1.f, frame_size, dropped_damage_rect);
+    frame.metadata.begin_frame_ack = ack;
+    view_->SubmitCompositorFrame(local_surface_id, std::move(frame));
+    view_->RunOnCompositingDidCommit();
+    ack.latest_confirmed_sequence_number = 4;
+    ack.has_damage = false;
+    EXPECT_EQ(ack, ack_tracker.last_ack());
+  }
+
+  // Change source_id known to the view. This should reset the
+  // latest_confirmed_sequence_number tracked by the view.
+  source_id = 20;
+
+  {
+    // Ack from CompositorFrame is forwarded with invalid
+    // latest_confirmed_sequence_number and without damage.
+    cc::BeginFrameAck ack(source_id, 10, 10, true);
+    gfx::Rect dropped_damage_rect(10, 20, 30, 40);
+    cc::CompositorFrame frame =
+        MakeDelegatedFrame(1.f, frame_size, dropped_damage_rect);
+    frame.metadata.begin_frame_ack = ack;
+    view_->SubmitCompositorFrame(local_surface_id, std::move(frame));
+    view_->RunOnCompositingDidCommit();
+    ack.latest_confirmed_sequence_number =
+        cc::BeginFrameArgs::kInvalidFrameNumber;
+    ack.has_damage = false;
+    EXPECT_EQ(ack, ack_tracker.last_ack());
+  }
+
+  {
+    // Explicit ack through OnDidNotProduceFrame is forwarded with invalid
+    // latest_confirmed_sequence_number.
+    cc::BeginFrameAck ack(source_id, 11, 11, false);
+    view_->OnDidNotProduceFrame(ack);
+    ack.latest_confirmed_sequence_number =
+        cc::BeginFrameArgs::kInvalidFrameNumber;
+    EXPECT_EQ(ack, ack_tracker.last_ack());
+  }
+
+  // Unlock the compositor again with a new CompositorFrame of correct size.
+  frame_size = view_rect.size();
+  local_surface_id = local_surface_id_allocator_.GenerateId();
+
+  {
+    // Ack from CompositorFrame is forwarded.
+    cc::BeginFrameAck ack(source_id, 12, 12, true);
+    cc::CompositorFrame frame = MakeDelegatedFrame(1.f, frame_size, view_rect);
+    frame.metadata.begin_frame_ack = ack;
+    view_->SubmitCompositorFrame(local_surface_id, std::move(frame));
+    view_->RunOnCompositingDidCommit();
+    EXPECT_EQ(ack, ack_tracker.last_ack());
+  }
+
+  {
+    // Explicit ack through OnDidNotProduceFrame is forwarded.
+    cc::BeginFrameAck ack(source_id, 13, 13, false);
+    view_->OnDidNotProduceFrame(ack);
+    EXPECT_EQ(ack, ack_tracker.last_ack());
+  }
 
-// TODO(eseckler): Add back tests for BeginFrameAck forwarding through
-// RenderWidgetHostViewAura and CompositorFrameSinkSupport when we add plumbing
-// of BeginFrameAcks through SurfaceObservers.
+  surface_manager->RemoveObserver(&ack_tracker);
+  view_->SetNeedsBeginFrames(false);
+}
 
 class RenderWidgetHostViewAuraCopyRequestTest
     : public RenderWidgetHostViewAuraShutdownTest {