sync compositor: Add output_surface_id

content/browser/android/synchronous_compositor_host.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/android/synchronous_compositor_host.h"
 
 #include <utility>
 
 #include "base/containers/hash_tables.h"
 #include "base/memory/shared_memory.h"
@@ skipping 24 matching lines @@
     : rwhva_(rwhva),
       client_(client),
       ui_task_runner_(
           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI)),
       routing_id_(rwhva_->GetRenderWidgetHost()->GetRoutingID()),
       sender_(rwhva_->GetRenderWidgetHost()),
       async_input_(async_input),
       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),
       need_invalidate_count_(0u),
       need_begin_frame_(false),
       did_activate_pending_tree_count_(0u),
       weak_ptr_factory_(this) {
   client_->DidInitializeCompositor(this);
 }
 
@@ skipping 10 matching lines @@
     IPC_MESSAGE_HANDLER(SyncCompositorHostMsg_OverScroll, OnOverScroll)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 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,
     const gfx::Rect& clip,
     const gfx::Rect& viewport_rect_for_tile_priority,
     const gfx::Transform& transform_for_tile_priority) {
   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;
 }
 
 void SynchronousCompositorHost::UpdateFrameMetaData(
     const cc::CompositorFrameMetadata& frame_metadata) {
   rwhva_->SynchronousFrameMetadata(frame_metadata);
 }
 
 namespace {
 
@@ skipping 151 matching lines @@
   software_draw_shm_ = std::move(software_draw_shm);
   ProcessCommonParams(common_renderer_params);
 }
 
 void SynchronousCompositorHost::SendZeroMemory() {
   // No need to check return value.
   sender_->Send(new SyncCompositorMsg_ZeroSharedMemory(routing_id_));
 }
 
 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());
 }
 
 void SynchronousCompositorHost::SetMemoryPolicy(size_t bytes_limit) {
   if (bytes_limit_ == bytes_limit)
     return;
   size_t current_bytes_limit = bytes_limit_;
   bytes_limit_ = bytes_limit;
@@ skipping 99 matching lines @@
     const DidOverscrollParams& over_scroll_params) {
   ProcessCommonParams(params);
   DidOverscroll(over_scroll_params);
 }
 
 void SynchronousCompositorHost::PopulateCommonParams(
     SyncCompositorCommonBrowserParams* params) {
   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_;
     params->update_root_scroll_offset = root_scroll_offset_updated_by_browser_;
     root_scroll_offset_updated_by_browser_ = false;
   }
   params->begin_frame_source_paused = !is_active_;
 
   weak_ptr_factory_.InvalidateWeakPtrs();
 }
@@ skipping 34 matching lines @@
         params.scrollable_size, params.page_scale_factor,
         params.min_page_scale_factor, params.max_page_scale_factor);
   }
 }
 
 void SynchronousCompositorHost::UpdateNeedsBeginFrames() {
   rwhva_->OnSetNeedsBeginFrames(is_active_ && need_begin_frame_);
 }
 
 }  // namespace content