Aura: Browser-side changes for Composite-To-Mailbox

content/browser/renderer_host/render_widget_host_view_aura.cc

Index: content/browser/renderer_host/render_widget_host_view_aura.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index 18cbbf007a7152015c1e58def86d067a9a158c9b..676ce8795c757f94e3cc6e60d087ca9811070946 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -11,6 +11,8 @@
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
+#include "cc/compositor_frame.h"
+#include "cc/compositor_frame_ack.h"
 #include "content/browser/renderer_host/backing_store_aura.h"
 #include "content/browser/renderer_host/dip_util.h"
 #include "content/browser/renderer_host/overscroll_controller.h"
@@ -274,6 +276,74 @@ bool PointerEventActivates(const ui::Event& event) {
   return false;
 }
 
+// Swap ack for the renderer when kCompositeToMailbox is enabled.
+void SendCompositorFrameAck(
+    int32 route_id,
+    int renderer_host_id,
+    const scoped_refptr<ui::Texture>& texture_to_produce) {
+  cc::CompositorFrameAck ack;
+  ack.gl_frame_data.reset(new cc::GLFrameData());
+  // If we produced a texture, we have to synchronize with the consumer of
+  // that texture.
+  if (texture_to_produce) {
+    std::string mailbox_name = texture_to_produce->Produce();
+    std::copy(mailbox_name.data(),
+              mailbox_name.data() + mailbox_name.length(),
+              reinterpret_cast<char*>(ack.gl_frame_data->mailbox.name));
+    ack.gl_frame_data->size = texture_to_produce->size();
+    ack.gl_frame_data->sync_point =
+        content::ImageTransportFactory::GetInstance()->InsertSyncPoint();
+  }
+
+  RenderWidgetHostImpl::SendSwapCompositorFrameAck(
+      route_id, renderer_host_id, ack);
+}
+
+// Tells the producer to reuse the same buffer that it previously pushed
+// into the mailbox, because we are skipping this frame.
+void SkipAndSendCompositorFrameAck(
+    int32 route_id,
+    int renderer_host_id,
+    const cc::CompositorFrame& frame) {
+  DCHECK(!frame.gl_frame_data->mailbox.isZero());
+  cc::CompositorFrameAck ack;
+  ack.gl_frame_data.reset(new cc::GLFrameData());
+  ack.gl_frame_data->mailbox = frame.gl_frame_data->mailbox;
+  RenderWidgetHostImpl::SendSwapCompositorFrameAck(
+      route_id, renderer_host_id, ack);
+}
+
+void AcknowledgeBufferForGpu(
+    int32 route_id,
+    int gpu_host_id,
+    const scoped_refptr<ui::Texture>& texture_to_produce) {
+  AcceleratedSurfaceMsg_BufferPresented_Params ack;
+  // If we produced a texture, we have to synchronize with the consumer of
+  // that texture.
+  uint32 sync_point = 0;
+  if (texture_to_produce) {
+    ack.mailbox_name = texture_to_produce->Produce();
+    sync_point =
+        content::ImageTransportFactory::GetInstance()->InsertSyncPoint();
+  }
+
+  ack.sync_point = sync_point;
+  RenderWidgetHostImpl::AcknowledgeBufferPresent(
+      route_id, gpu_host_id, ack);
+}
+
+void AcknowledgeSkippedBufferForGpu(
+    int32 route_id,
+    int gpu_host_id,
+    const std::string& mailbox_name) {
+  DCHECK(!mailbox_name.empty());
+      AcceleratedSurfaceMsg_BufferPresented_Params ack;
+  ack.mailbox_name = mailbox_name;
+  ack.sync_point = 0;
+  RenderWidgetHostImpl::AcknowledgeBufferPresent(
+      route_id, gpu_host_id, ack);
+}

[piman, 2013/02/27 01:07:13]

I /think/ those 4 functions could be factored into 2 (ie merging the skipped vs
success cases), and factor some of the logic into the callers (by having the
BuffersSwapped failure case call the callback).

Essentially they all want to send a message one way or another, with:
- a mailbox name
- an optional sync point

Both 'success' functions call Produce on the texture and InsertSyncPoint to
derive the mailbox name and sync point. Both 'skip' functions use the mailbox
name from the incoming frame, and a 0 sync point.

So you could have the logic to Produce and InsertSyncPoint (success case) in
SwapBuffersCompleted (no compositor case) and an intermediate callback
(compositor case), and have SwapBuffersPrepare directly call the ack_callback
with the incoming frame mailbox name (and 0 sync point). The advantage is that
it would make it a lot more obvious that both paths do the same thing.

[no sievers, 2013/02/27 21:07:40]

Done.

+
 }  // namespace
 
 // We need to watch for mouse events outside a Web Popup or its parent
@@ -521,16 +591,6 @@ class RenderWidgetHostViewAura::ResizeLock {
   DISALLOW_COPY_AND_ASSIGN(ResizeLock);
 };
 
-RenderWidgetHostViewAura::BufferPresentedParams::BufferPresentedParams(
-    int route_id,
-    int gpu_host_id)
-    : route_id(route_id),
-      gpu_host_id(gpu_host_id) {
-}
-
-RenderWidgetHostViewAura::BufferPresentedParams::~BufferPresentedParams() {
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewAura, public:
 
@@ -1117,10 +1177,8 @@ void RenderWidgetHostViewAura::UpdateExternalTexture() {
 bool RenderWidgetHostViewAura::SwapBuffersPrepare(
     const gfx::Rect& surface_rect,
     const gfx::Rect& damage_rect,
-    const std::string& mailbox_name,
-    BufferPresentedParams* params) {
+    const std::string& mailbox_name) {
   DCHECK(!mailbox_name.empty());
-  DCHECK(!params->texture_to_produce);
 
   if (last_swapped_surface_size_ != surface_rect.size()) {
     // The surface could have shrunk since we skipped an update, in which
@@ -1132,16 +1190,9 @@ bool RenderWidgetHostViewAura::SwapBuffersPrepare(
 
   if (ShouldSkipFrame(surface_rect.size())) {
     skipped_damage_.op(RectToSkIRect(damage_rect), SkRegion::kUnion_Op);
-    AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
-    ack_params.mailbox_name = mailbox_name;
-    ack_params.sync_point = 0;
-    RenderWidgetHostImpl::AcknowledgeBufferPresent(
-        params->route_id, params->gpu_host_id, ack_params);
     return false;
   }
 
-  params->texture_to_produce = current_surface_;
-
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
   current_surface_ = factory->CreateTransportClient(device_scale_factor_);
   if (!current_surface_) {
@@ -1157,15 +1208,16 @@ bool RenderWidgetHostViewAura::SwapBuffersPrepare(
 }
 
 void RenderWidgetHostViewAura::SwapBuffersCompleted(
-    const BufferPresentedParams& params) {
+    const BufferPresentedCallback& ack_callback,
+      const scoped_refptr<ui::Texture>& texture_to_return) {

[piman, 2013/02/27 01:07:13]

nit:indent

[no sievers, 2013/02/27 21:07:40]

Done.

   ui::Compositor* compositor = GetCompositor();
   if (!compositor) {
-    InsertSyncPointAndACK(params);
+    ack_callback.Run(texture_to_return);
   } else {
     // Add sending an ACK to the list of things to do OnCompositingDidCommit
     can_lock_compositor_ = NO_PENDING_COMMIT;
     on_compositing_did_commit_callbacks_.push_back(
-        base::Bind(&RenderWidgetHostViewAura::InsertSyncPointAndACK, params));
+        base::Bind(ack_callback, texture_to_return));
     if (!compositor->HasObserver(this))
       compositor->AddObserver(this);
   }
@@ -1203,39 +1255,87 @@ void RenderWidgetHostViewAura::UpdateCutoutRects() {
 void RenderWidgetHostViewAura::AcceleratedSurfaceBuffersSwapped(
     const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params_in_pixel,
     int gpu_host_id) {
-  const gfx::Rect surface_rect = gfx::Rect(gfx::Point(), params_in_pixel.size);
-  BufferPresentedParams ack_params(params_in_pixel.route_id, gpu_host_id);
-  if (!SwapBuffersPrepare(
-      surface_rect, surface_rect, params_in_pixel.mailbox_name, &ack_params))
+  BufferPresentedCallback ack_callback = base::Bind(
+      &AcknowledgeBufferForGpu, params_in_pixel.route_id, gpu_host_id);
+  if (!BuffersSwapped(params_in_pixel.size, params_in_pixel.mailbox_name,
+                      ack_callback)) {
+    AcknowledgeSkippedBufferForGpu(params_in_pixel.route_id,
+                                   gpu_host_id,
+                                   params_in_pixel.mailbox_name);
+  }
+}
+
+void RenderWidgetHostViewAura::OnSwapCompositorFrame(
+    const cc::CompositorFrame& frame, int render_host_id, int route_id) {
+  if (!frame.gl_frame_data)
     return;
 
+  DCHECK(!frame.gl_frame_data->size.IsEmpty());
+  DCHECK(!frame.gl_frame_data->mailbox.isZero());
+  DCHECK(frame.gl_frame_data->sync_point);

[piman, 2013/02/27 01:07:13]

note: all these DCHECKs are based on untrusted data coming from the renderer.
It'd be better probably to skip the frame.

[no sievers, 2013/02/27 21:07:40]

Done.

+
+  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+  factory->WaitSyncPoint(frame.gl_frame_data->sync_point);
+
+  BufferPresentedCallback ack_callback = base::Bind(
+      &SendCompositorFrameAck, route_id, render_host_id);
+
+  std::string mailbox_name(
+      reinterpret_cast<const char*>(frame.gl_frame_data->mailbox.name),
+      sizeof(frame.gl_frame_data->mailbox.name));
+  if (!BuffersSwapped(frame.gl_frame_data->size,
+                      mailbox_name,
+                      ack_callback)) {
+    SkipAndSendCompositorFrameAck(route_id, render_host_id, frame);
+  }
+}
+
+bool RenderWidgetHostViewAura::BuffersSwapped(
+    const gfx::Size& size,
+    const std::string& mailbox_name,
+    const BufferPresentedCallback& ack_callback) {
+  scoped_refptr<ui::Texture> texture_to_return(current_surface_);
+  const gfx::Rect surface_rect = gfx::Rect(gfx::Point(), size);
+  if (!SwapBuffersPrepare(surface_rect, surface_rect, mailbox_name))
+    return false;
+
   previous_damage_.setRect(RectToSkIRect(surface_rect));
   skipped_damage_.setEmpty();
 
   ui::Compositor* compositor = GetCompositor();
   if (compositor) {
-    gfx::Size surface_size = ConvertSizeToDIP(this, params_in_pixel.size);
+    gfx::Size surface_size = ConvertSizeToDIP(this, size);
     window_->SchedulePaintInRect(gfx::Rect(surface_size));
   }
 
   if (paint_observer_)
     paint_observer_->OnUpdateCompositorContent();
-  SwapBuffersCompleted(ack_params);
+
+  SwapBuffersCompleted(ack_callback, texture_to_return);
+
+  return true;
 }
 
 void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
     const GpuHostMsg_AcceleratedSurfacePostSubBuffer_Params& params_in_pixel,
     int gpu_host_id) {
+  scoped_refptr<ui::Texture> previous_texture(current_surface_);
   const gfx::Rect surface_rect =
       gfx::Rect(gfx::Point(), params_in_pixel.surface_size);
   gfx::Rect damage_rect(params_in_pixel.x,
                         params_in_pixel.y,
                         params_in_pixel.width,
                         params_in_pixel.height);
-  BufferPresentedParams ack_params(params_in_pixel.route_id, gpu_host_id);
   if (!SwapBuffersPrepare(
-      surface_rect, damage_rect, params_in_pixel.mailbox_name, &ack_params))
+      surface_rect, damage_rect, params_in_pixel.mailbox_name)) {
+    AcknowledgeSkippedBufferForGpu(params_in_pixel.route_id,
+                                   gpu_host_id,
+                                   params_in_pixel.mailbox_name);
     return;
+  }
+
+  BufferPresentedCallback ack_callback = base::Bind(
+      &AcknowledgeBufferForGpu, params_in_pixel.route_id, gpu_host_id);
 
   SkRegion damage(RectToSkIRect(damage_rect));
   if (!skipped_damage_.isEmpty()) {
@@ -1247,17 +1347,17 @@ void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
   ui::Texture* current_texture = current_surface_.get();
 
   const gfx::Size surface_size_in_pixel = params_in_pixel.surface_size;
-  DLOG_IF(ERROR, ack_params.texture_to_produce &&
-      ack_params.texture_to_produce->size() != current_texture->size() &&
+  DLOG_IF(ERROR, previous_texture &&
+      previous_texture->size() != current_texture->size() &&
       SkIRectToRect(damage.getBounds()) != surface_rect) <<
       "Expected full damage rect after size change";
-  if (ack_params.texture_to_produce && !previous_damage_.isEmpty() &&
-      ack_params.texture_to_produce->size() == current_texture->size()) {
+  if (previous_texture && !previous_damage_.isEmpty() &&
+      previous_texture->size() == current_texture->size()) {
     ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
     GLHelper* gl_helper = factory->GetGLHelper();
     gl_helper->CopySubBufferDamage(
         current_texture->PrepareTexture(),
-        ack_params.texture_to_produce->PrepareTexture(),
+        previous_texture->PrepareTexture(),
         damage,
         previous_damage_);
   }
@@ -1284,7 +1384,7 @@ void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
     window_->SchedulePaintInRect(rect_to_paint);
   }
 
-  SwapBuffersCompleted(ack_params);
+  SwapBuffersCompleted(ack_callback, previous_texture);
 }
 
 void RenderWidgetHostViewAura::AcceleratedSurfaceSuspend() {
@@ -2308,24 +2408,6 @@ void RenderWidgetHostViewAura::RunCompositingDidCommitCallbacks() {
   on_compositing_did_commit_callbacks_.clear();
 }
 
-// static
-void RenderWidgetHostViewAura::InsertSyncPointAndACK(
-    const BufferPresentedParams& params) {
-  uint32 sync_point = 0;
-  AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
-
-  // If we produced a texture, we have to synchronize with the consumer of
-  // that texture.
-  if (params.texture_to_produce) {
-    ack_params.mailbox_name = params.texture_to_produce->Produce();
-    sync_point = ImageTransportFactory::GetInstance()->InsertSyncPoint();
-  }
-
-  ack_params.sync_point = sync_point;
-  RenderWidgetHostImpl::AcknowledgeBufferPresent(
-      params.route_id, params.gpu_host_id, ack_params);
-}
-
 void RenderWidgetHostViewAura::AddedToRootWindow() {
   window_->GetRootWindow()->AddRootWindowObserver(this);
   host_->ParentChanged(GetNativeViewId());