sync compositor: Add output_surface_id

content/browser/android/synchronous_compositor_host.cc

Index: content/browser/android/synchronous_compositor_host.cc
diff --git a/content/browser/android/synchronous_compositor_host.cc b/content/browser/android/synchronous_compositor_host.cc
index 6cb4907973b1ba8afb46db17d29d55ef56cd004b..ef7ced4e68749b9111f248ec1b1d3457ce174c86 100644
--- a/content/browser/android/synchronous_compositor_host.cc
+++ b/content/browser/android/synchronous_compositor_host.cc
@@ -42,6 +42,7 @@ SynchronousCompositorHost::SynchronousCompositorHost(
       use_in_process_zero_copy_software_draw_(use_in_proc_software_draw),
       is_active_(false),
       bytes_limit_(0u),
+      output_surface_id_for_returned_resources_(0u),

[no sievers, 2016/03/18 20:03:13]

This looks like it might be potentially brittle combined with the logic in
ReturnResources() below if somebody went and changed the first id from the
renderer to be nonzero.

[boliu, 2016/03/19 00:18:16]

Doesn't matter anymore, that the id is set in draw. :)

       root_scroll_offset_updated_by_browser_(false),
       renderer_param_version_(0u),
       need_animate_scroll_(false),
@@ -72,7 +73,7 @@ void SynchronousCompositorHost::DidBecomeCurrent() {
   client_->DidBecomeCurrent(this);
 }
 
-scoped_ptr<cc::CompositorFrame> SynchronousCompositorHost::DemandDrawHw(
+SynchronousCompositor::Frame SynchronousCompositorHost::DemandDrawHw(
     const gfx::Size& surface_size,
     const gfx::Transform& transform,
     const gfx::Rect& viewport,
@@ -82,22 +83,23 @@ scoped_ptr<cc::CompositorFrame> SynchronousCompositorHost::DemandDrawHw(
   SyncCompositorDemandDrawHwParams params(surface_size, transform, viewport,
                                           clip, viewport_rect_for_tile_priority,
                                           transform_for_tile_priority);
-  scoped_ptr<cc::CompositorFrame> frame(new cc::CompositorFrame);
+  SynchronousCompositor::Frame frame;
+  frame.frame.reset(new cc::CompositorFrame);
   SyncCompositorCommonBrowserParams common_browser_params;
   PopulateCommonParams(&common_browser_params);
   SyncCompositorCommonRendererParams common_renderer_params;
   if (!sender_->Send(new SyncCompositorMsg_DemandDrawHw(
           routing_id_, common_browser_params, params, &common_renderer_params,
-          frame.get()))) {
-    return nullptr;
+          &frame.output_surface_id, frame.frame.get()))) {
+    return SynchronousCompositor::Frame();
   }
   ProcessCommonParams(common_renderer_params);
-  if (!frame->delegated_frame_data) {
+  if (!frame.frame->delegated_frame_data) {
     // This can happen if compositor did not swap in this draw.
-    frame.reset();
+    frame.frame.reset();
   }
-  if (frame)
-    UpdateFrameMetaData(frame->metadata);
+  if (frame.frame)
+    UpdateFrameMetaData(frame.frame->metadata);
   return frame;
 }
 
@@ -269,7 +271,16 @@ void SynchronousCompositorHost::SendZeroMemory() {
 }
 
 void SynchronousCompositorHost::ReturnResources(
+    uint32_t output_surface_id,
     const cc::CompositorFrameAck& frame_ack) {
+  if (output_surface_id_for_returned_resources_ != output_surface_id) {
+    if ((output_surface_id_for_returned_resources_ - output_surface_id) <
+        0x80000000) {
+      return;
+    }
+    returned_resources_.clear();
+    output_surface_id_for_returned_resources_ = output_surface_id;

[no sievers, 2016/03/18 20:03:13]

The logic in the if-block here is a bit confusing.

Isn't it easier to just track the output surface id for the last frame we got,
and whenever the id we are trying to return to mismatches here then drop the
resources?

And for the synchronous swap-ack the renderer already deals with it.

[boliu, 2016/03/19 00:18:16]

Hmm, yeah, that works. hush actually suggested that earlier in the review,
before I realized the out of order problem.

Just need to be careful super careful not to label the returned resources with
the wrong id.

+  }
   returned_resources_.insert(returned_resources_.end(),
                              frame_ack.resources.begin(),
                              frame_ack.resources.end());
@@ -389,6 +400,8 @@ void SynchronousCompositorHost::PopulateCommonParams(
   DCHECK(params);
   DCHECK(params->ack.resources.empty());
   params->bytes_limit = bytes_limit_;
+  params->output_surface_id_for_returned_resources =
+      output_surface_id_for_returned_resources_;
   params->ack.resources.swap(returned_resources_);
   if (root_scroll_offset_updated_by_browser_) {
     params->root_scroll_offset = root_scroll_offset_;