cc: Dedup IPCs to return resources to client

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 11a59b6b65e2b073ee1d08376c8a96a01546e277..3aac52c5376e608add3de68a2c23e38f7a543c9c 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
@@ -229,6 +229,13 @@ class TestWindowObserver : public aura::WindowObserver {
   DISALLOW_COPY_AND_ASSIGN(TestWindowObserver);
 };
 
+class FakeSurfaceDamageObserver : public cc::SurfaceDamageObserver {
+ public:
+  void OnSurfaceDamaged(const cc::SurfaceId& id, bool* changed) override {
+    *changed = true;
+  }
+};
+
 class FakeFrameSubscriber : public RenderWidgetHostViewFrameSubscriber {
  public:
   FakeFrameSubscriber(gfx::Size size, base::Callback<void(bool)> callback)
@@ -348,6 +355,12 @@ class FakeRenderWidgetHostViewAura : public RenderWidgetHostViewAura {
     return GetDelegatedFrameHost()->ReleasedFrontLockActiveForTesting();
   }
 
+  void ReturnResources(const cc::ReturnedResourceArray& resources) {
+    GetDelegatedFrameHost()->ReturnResources(resources);
+  }
+
+  void ResetCompositor() { GetDelegatedFrameHost()->ResetCompositor(); }
+
   // A lock that doesn't actually do anything to the compositor, and does not
   // time out.
   class FakeResizeLock : public ResizeLock {
@@ -1599,9 +1612,143 @@ cc::CompositorFrame MakeDelegatedFrame(float scale_factor,
   pass->SetNew(
       cc::RenderPassId(1, 1), gfx::Rect(size), damage, gfx::Transform());
   frame.delegated_frame_data->render_pass_list.push_back(std::move(pass));
+  cc::TransferableResource resource;
+  resource.id = 1;
+  frame.delegated_frame_data->resource_list.push_back(std::move(resource));
   return frame;
 }
 
+// If the ui::Compositor has been reset then resources are returned back to the
+// client in response to the swap. This test verifies that the returned
+// resources are indeed reported as being in response to a swap.
+TEST_F(RenderWidgetHostViewAuraTest, ResettingCompositorReturnsResources) {
+  FakeSurfaceDamageObserver damage_observer;
+  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+  cc::SurfaceManager* manager = factory->GetSurfaceManager();
+  manager->AddObserver(&damage_observer);
+
+  gfx::Size view_size(100, 100);
+  gfx::Rect view_rect(view_size);
+
+  view_->InitAsChild(NULL);

[danakj, 2016/07/14 19:55:44]

nullptr

[Fady Samuel, 2016/07/14 23:17:04]

Done. I just did a s/NULL/nullptr

+  aura::client::ParentWindowWithContext(
+      view_->GetNativeView(), parent_view_->GetNativeView()->GetRootWindow(),
+      gfx::Rect());
+  view_->SetSize(view_size);
+  view_->Show();
+  sink_->ClearMessages();
+
+  view_->ResetCompositor();
+
+  // Swappinga  frame should trigger a swap ACK IPC because we have reset the

[danakj, 2016/07/14 19:55:44]

Swapping a frame

[Fady Samuel, 2016/07/14 23:17:04]

Done.

+  // compositor.
+  view_->OnSwapCompositorFrame(0,
+                               MakeDelegatedFrame(1.f, view_size, view_rect));
+  EXPECT_EQ(1u, sink_->message_count());
+  {
+    const IPC::Message* msg = sink_->GetMessageAt(0);
+    EXPECT_EQ(ViewMsg_ReclaimCompositorResources::ID, msg->type());
+    ViewMsg_ReclaimCompositorResources::Param params;
+    ViewMsg_ReclaimCompositorResources::Read(msg, &params);
+    EXPECT_EQ(0u, std::get<0>(params));    // output_surface_id
+    EXPECT_EQ(true, std::get<1>(params));  // is_swap_ack

[danakj, 2016/07/14 19:55:44]

EXPECT_TRUE (i've seen EQ(true/false) fail to compile on some platforms?

[Fady Samuel, 2016/07/14 23:17:04]

Done.

+  }
+  manager->RemoveObserver(&damage_observer);
+}
+
+// This test verifies that returned resources do not require a pending ack.
+TEST_F(RenderWidgetHostViewAuraTest, ReturnedResources) {
+  gfx::Size view_size(100, 100);
+  gfx::Rect view_rect(view_size);
+
+  view_->InitAsChild(NULL);
+  aura::client::ParentWindowWithContext(
+      view_->GetNativeView(), parent_view_->GetNativeView()->GetRootWindow(),
+      gfx::Rect());
+  view_->SetSize(view_size);
+  view_->Show();
+  sink_->ClearMessages();
+
+  // Accumulate some returned resources. This should not trigger an IPC.
+  cc::ReturnedResourceArray resources;
+  cc::ReturnedResource resource;
+  resource.id = 1;
+  resources.push_back(resource);
+  view_->ReturnResources(resources);
+  EXPECT_EQ(1u, sink_->message_count());
+  {
+    const IPC::Message* msg = sink_->GetMessageAt(0);
+    EXPECT_EQ(ViewMsg_ReclaimCompositorResources::ID, msg->type());
+    ViewMsg_ReclaimCompositorResources::Param params;
+    ViewMsg_ReclaimCompositorResources::Read(msg, &params);
+    EXPECT_EQ(0u, std::get<0>(params));     // output_surface_id
+    EXPECT_EQ(false, std::get<1>(params));  // is_swap_ack

[danakj, 2016/07/14 19:55:44]

EXPECT_FALSE

[Fady Samuel, 2016/07/14 23:17:04]

Done.

+  }
+}
+
+// This test verifies that when the output_surface_id changes, then
+// DelegateFrameHost

[danakj, 2016/07/14 19:55:44]

weird line wrapping

[Fady Samuel, 2016/07/14 23:17:04]

Done.

+// returns compositor resources without a swap ack.
+TEST_F(RenderWidgetHostViewAuraTest, TwoOutputSurfaces) {
+  FakeSurfaceDamageObserver damage_observer;
+  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+  cc::SurfaceManager* manager = factory->GetSurfaceManager();
+  manager->AddObserver(&damage_observer);
+
+  gfx::Size view_size(100, 100);
+  gfx::Rect view_rect(view_size);
+
+  view_->InitAsChild(NULL);
+  aura::client::ParentWindowWithContext(
+      view_->GetNativeView(), parent_view_->GetNativeView()->GetRootWindow(),
+      gfx::Rect());
+  view_->SetSize(view_size);
+  view_->Show();
+  sink_->ClearMessages();
+
+  view_->OnSwapCompositorFrame(0,
+                               MakeDelegatedFrame(1.f, view_size, view_rect));
+
+  // Accumulate some returned resources. This should not trigger an IPC.
+  cc::ReturnedResourceArray resources;
+  cc::ReturnedResource resource;
+  resource.id = 1;
+  resources.push_back(resource);
+  view_->ReturnResources(resources);
+  EXPECT_EQ(0u, sink_->message_count());
+
+  // Swap another CompositorFrame but this time from another output_surface_id.
+  // This should trigger a non-ACK ReclaimCompositorResources IPC.
+  view_->OnSwapCompositorFrame(1,
+                               MakeDelegatedFrame(1.f, view_size, view_rect));
+  EXPECT_EQ(1u, sink_->message_count());
+  {
+    const IPC::Message* msg = sink_->GetMessageAt(0);
+    EXPECT_EQ(ViewMsg_ReclaimCompositorResources::ID, msg->type());
+    ViewMsg_ReclaimCompositorResources::Param params;
+    ViewMsg_ReclaimCompositorResources::Read(msg, &params);
+    EXPECT_EQ(0u, std::get<0>(params));     // output_surface_id
+    EXPECT_EQ(false, std::get<1>(params));  // is_swap_ack
+  }
+  sink_->ClearMessages();
+
+  // Report that the surface is drawn to trigger an ACK.
+  cc::Surface* surface = manager->GetSurfaceForId(view_->surface_id());
+  EXPECT_TRUE(surface);
+  surface->RunDrawCallbacks(cc::SurfaceDrawStatus::DRAWN);
+  EXPECT_EQ(1u, sink_->message_count());
+  {
+    const IPC::Message* msg = sink_->GetMessageAt(0);
+    EXPECT_EQ(ViewMsg_ReclaimCompositorResources::ID, msg->type());
+    ViewMsg_ReclaimCompositorResources::Param params;
+    ViewMsg_ReclaimCompositorResources::Read(msg, &params);
+    EXPECT_EQ(1u, std::get<0>(params));    // output_surface_id
+    EXPECT_EQ(true, std::get<1>(params));  // is_swap_ack
+  }
+
+  manager->RemoveObserver(&damage_observer);
+}
+
 // Resizing in fullscreen mode should send the up-to-date screen info.
 // http://crbug.com/324350
 TEST_F(RenderWidgetHostViewAuraTest, DISABLED_FullscreenResize) {
@@ -1822,7 +1969,8 @@ TEST_F(RenderWidgetHostViewAuraTest, Resize) {
       0, MakeDelegatedFrame(1.f, size3, gfx::Rect(size3)));
   // Expect the frame ack;
   EXPECT_EQ(1u, sink_->message_count());
-  EXPECT_EQ(ViewMsg_SwapCompositorFrameAck::ID, sink_->GetMessageAt(0)->type());
+  EXPECT_EQ(ViewMsg_ReclaimCompositorResources::ID,
+            sink_->GetMessageAt(0)->type());
   sink_->ClearMessages();
   EXPECT_EQ(size2.ToString(), view_->GetRequestedRendererSize().ToString());
 
@@ -1861,7 +2009,7 @@ TEST_F(RenderWidgetHostViewAuraTest, Resize) {
         EXPECT_EQ(blink::WebInputEvent::MouseMove, event->type);
         break;
       }
-      case ViewMsg_SwapCompositorFrameAck::ID:
+      case ViewMsg_ReclaimCompositorResources::ID:
         break;
       case ViewMsg_Resize::ID: {
         EXPECT_FALSE(has_resize);