cc: Dedup IPCs to return resources to client

content/browser/renderer_host/delegated_frame_host.cc

Index: content/browser/renderer_host/delegated_frame_host.cc
diff --git a/content/browser/renderer_host/delegated_frame_host.cc b/content/browser/renderer_host/delegated_frame_host.cc
index def08103bc3960e83f5be65353f6341b8b4f8b73..07daefee1a683702854e63302489c7db8e21a616 100644
--- a/content/browser/renderer_host/delegated_frame_host.cc
+++ b/content/browser/renderer_host/delegated_frame_host.cc
@@ -14,7 +14,6 @@
 #include "base/memory/ptr_util.h"
 #include "base/time/default_tick_clock.h"
 #include "cc/output/compositor_frame.h"
-#include "cc/output/compositor_frame_ack.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/resources/single_release_callback.h"
 #include "cc/resources/texture_mailbox.h"
@@ -410,15 +409,16 @@ void DelegatedFrameHost::SwapDelegatedFrame(uint32_t output_surface_id,
       gfx::ConvertRectToDIP(frame_device_scale_factor, damage_rect);
 
   if (ShouldSkipFrame(frame_size_in_dip)) {
-    cc::CompositorFrameAck ack;
+    cc::ReturnedResourceArray resources;
     cc::TransferableResource::ReturnResources(frame_data->resource_list,
-                                              &ack.resources);
+                                              &resources);
 
     skipped_latency_info_list_.insert(skipped_latency_info_list_.end(),
                                       frame.metadata.latency_info.begin(),
                                       frame.metadata.latency_info.end());
 
-    client_->DelegatedFrameHostSendCompositorSwapAck(output_surface_id, ack);
+    client_->DelegatedFrameHostSendReclaimCompositorResources(
+        output_surface_id, true /* is_swap_ack*/, resources);
     skipped_frames_ = true;
     return;
   }
@@ -445,7 +445,8 @@ void DelegatedFrameHost::SwapDelegatedFrame(uint32_t output_surface_id,
 
     surface_factory_.reset();
     if (!surface_returned_resources_.empty())
-      SendReturnedDelegatedResources(last_output_surface_id_);
+      SendReclaimCompositorResources(last_output_surface_id_,
+                                     false /* is_swap_ack */);
 
     last_output_surface_id_ = output_surface_id;
   }
@@ -509,7 +510,7 @@ void DelegatedFrameHost::SwapDelegatedFrame(uint32_t output_surface_id,
   // Note that |compositor_| may be reset by SetShowSurface or
   // SetShowDelegatedContent above.
   if (!compositor_ || skip_frame) {
-    SendDelegatedFrameAck(output_surface_id);
+    SendReclaimCompositorResources(output_surface_id, false /* is_swap_ack */);

[piman, 2016/07/13 20:59:14]

this one is a swap ack.

[Fady Samuel, 2016/07/13 22:12:16]

Done.

[danakj, 2016/07/13 22:15:03]

Are there (can there be) tests that cover these mistakes?

[Fady Samuel, 2016/07/14 02:50:10]

This is now covered by
RenderWidgetHostViewAuraTest.ResettingCompositorReturnsResources

   } else {
     can_lock_compositor_ = NO_PENDING_COMMIT;
   }
@@ -524,28 +525,19 @@ void DelegatedFrameHost::ClearDelegatedFrame() {
     EvictDelegatedFrame();
 }
 
-void DelegatedFrameHost::SendDelegatedFrameAck(uint32_t output_surface_id) {
-  cc::CompositorFrameAck ack;
-  if (!surface_returned_resources_.empty())
-    ack.resources.swap(surface_returned_resources_);
-  client_->DelegatedFrameHostSendCompositorSwapAck(output_surface_id, ack);
+void DelegatedFrameHost::SendReclaimCompositorResources(
+    uint32_t output_surface_id,
+    bool is_swap_ack) {
+  client_->DelegatedFrameHostSendReclaimCompositorResources(
+      output_surface_id, is_swap_ack, surface_returned_resources_);
+  surface_returned_resources_.clear();
   DCHECK_GT(pending_delegated_ack_count_, 0);
   pending_delegated_ack_count_--;

[piman, 2016/07/13 20:59:14]

this (the DCHECK and the decrement) should only happen if is_swap_ack is true.

[Fady Samuel, 2016/07/13 22:12:16]

Done.

[Fady Samuel, 2016/07/14 02:50:10]

This is now covered by RenderWidgetHostViewAuraTest.ReturnedResources

 }
 
 void DelegatedFrameHost::SurfaceDrawn(uint32_t output_surface_id,
                                       cc::SurfaceDrawStatus drawn) {
-  SendDelegatedFrameAck(output_surface_id);
-}
-
-void DelegatedFrameHost::SendReturnedDelegatedResources(
-    uint32_t output_surface_id) {
-  cc::CompositorFrameAck ack;
-  DCHECK(!surface_returned_resources_.empty());
-  ack.resources.swap(surface_returned_resources_);
-
-  client_->DelegatedFrameHostSendReclaimCompositorResources(output_surface_id,
-                                                            ack);
+  SendReclaimCompositorResources(output_surface_id, true /* is_swap_ack */);
 }
 
 void DelegatedFrameHost::ReturnResources(
@@ -555,7 +547,8 @@ void DelegatedFrameHost::ReturnResources(
   std::copy(resources.begin(), resources.end(),
             std::back_inserter(surface_returned_resources_));
   if (!pending_delegated_ack_count_)
-    SendReturnedDelegatedResources(last_output_surface_id_);
+    SendReclaimCompositorResources(last_output_surface_id_,
+                                   false /* is_swap_ack */);
 }
 
 void DelegatedFrameHost::WillDrawSurface(const cc::SurfaceId& id,