sync compsitor: Send software reply in swap

content/renderer/android/synchronous_compositor_proxy.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/renderer/android/synchronous_compositor_proxy.h"
 
 #include "base/auto_reset.h"
 #include "base/memory/shared_memory.h"
 #include "content/common/android/sync_compositor_messages.h"
 #include "content/common/cc_messages.h"
@@ skipping 16 matching lines @@
     ui::SynchronousInputHandlerProxy* input_handler_proxy,
     InputHandlerManagerClient::Handler* handler)
     : routing_id_(routing_id),
       sender_(sender),
       output_surface_(output_surface),
       begin_frame_source_(begin_frame_source),
       input_handler_proxy_(input_handler_proxy),
       input_handler_(handler),
       inside_receive_(false),
       hardware_draw_reply_(nullptr),
+      software_draw_reply_(nullptr),
       bytes_limit_(0u),
       version_(0u),
       page_scale_factor_(0.f),
       min_page_scale_factor_(0.f),
       max_page_scale_factor_(0.f),
       need_animate_scroll_(false),
       need_invalidate_(false),
       need_begin_frame_(false),
       did_activate_pending_tree_(false) {
   DCHECK(output_surface_);
@@ skipping 100 matching lines @@
 void SynchronousCompositorProxy::OnMessageReceived(
     const IPC::Message& message) {
   IPC_BEGIN_MESSAGE_MAP(SynchronousCompositorProxy, message)
     IPC_MESSAGE_HANDLER(SyncCompositorMsg_HandleInputEvent, HandleInputEvent)
     IPC_MESSAGE_HANDLER(SyncCompositorMsg_BeginFrame, BeginFrame)
     IPC_MESSAGE_HANDLER(SyncCompositorMsg_ComputeScroll, OnComputeScroll)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(SyncCompositorMsg_DemandDrawHw,
                                     DemandDrawHw)
     IPC_MESSAGE_HANDLER(SyncCompositorMsg_SetSharedMemory, SetSharedMemory)
     IPC_MESSAGE_HANDLER(SyncCompositorMsg_ZeroSharedMemory, ZeroSharedMemory)
-    IPC_MESSAGE_HANDLER(SyncCompositorMsg_DemandDrawSw, DemandDrawSw)
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(SyncCompositorMsg_DemandDrawSw,
+                                    DemandDrawSw)
     IPC_MESSAGE_HANDLER(SyncCompositorMsg_UpdateState, ProcessCommonParams)
   IPC_END_MESSAGE_MAP()
 }
 
 bool SynchronousCompositorProxy::Send(IPC::Message* message) {
   return sender_->Send(message);
 }
 
 void SynchronousCompositorProxy::HandleInputEvent(
     const SyncCompositorCommonBrowserParams& common_params,
@@ skipping 106 matching lines @@
 
 void SynchronousCompositorProxy::ZeroSharedMemory() {
   DCHECK(!software_draw_shm_->zeroed);
   memset(software_draw_shm_->shm.memory(), 0, software_draw_shm_->buffer_size);
   software_draw_shm_->zeroed = true;
 }
 
 void SynchronousCompositorProxy::DemandDrawSw(
     const SyncCompositorCommonBrowserParams& common_params,
     const SyncCompositorDemandDrawSwParams& params,
-    bool* result,
-    SyncCompositorCommonRendererParams* common_renderer_params,
-    cc::CompositorFrame* frame) {
+    IPC::Message* reply_message) {
   DCHECK(!inside_receive_);
-  base::AutoReset<bool> scoped_inside_receive(&inside_receive_, true);
+  inside_receive_ = true;
+  ProcessCommonParams(common_params);
+  {
+    base::AutoReset<IPC::Message*> scoped_hardware_draw_reply(

[hush (inactive), 2015/12/30 21:51:09]

name scoped_software_draw_reply?

[boliu, 2015/12/30 22:17:46]

Oops. Done

+        &software_draw_reply_, reply_message);
+    DoDemandDrawSw(params);
+  }
+  if (inside_receive_) {
+    // Did not swap.
+    cc::CompositorFrame empty_frame;
+    SendDemandDrawSwReply(false, &empty_frame, reply_message);

[hush (inactive), 2015/12/30 21:51:09]

there seems to be an inconsistency between the IPC APIs of software draw and
hardware draw.
In hardware draw, we don't send the "success" bit. In software draw, we do.

In fact, we can still infer success bit from the emptiness of the frame in
software draw mode right?

[boliu, 2015/12/30 22:17:46]

True. Previously the success bit was so that I could write *result = false at
the beginning and not worry about early outs. I guess this patch solved that by
factoring out DoDemandDrawSw.

Don't want to touch IPC messages in this patch though.

+    inside_receive_ = false;
+  } else {
+    DeliverMessages();
+  }
+}
 
-  DCHECK(frame);
-  ProcessCommonParams(common_params);
-  *result = false;  // Early out ok.
-
+void SynchronousCompositorProxy::DoDemandDrawSw(
+    const SyncCompositorDemandDrawSwParams& params) {
   DCHECK(software_draw_shm_->zeroed);
   software_draw_shm_->zeroed = false;
 
   SkImageInfo info =
       SkImageInfo::MakeN32Premul(params.size.width(), params.size.height());
   size_t stride = info.minRowBytes();
   size_t buffer_size = info.getSafeSize(stride);
   DCHECK_EQ(software_draw_shm_->buffer_size, buffer_size);
 
   SkBitmap bitmap;
   if (!bitmap.installPixels(info, software_draw_shm_->shm.memory(), stride))
     return;
   SkCanvas canvas(bitmap);
   canvas.setMatrix(params.transform.matrix());
   canvas.setClipRegion(SkRegion(gfx::RectToSkIRect(params.clip)));
 
   output_surface_->DemandDrawSw(&canvas);
-  if (software_frame_holder_) {
-    *result = true;
-    software_frame_holder_->AssignTo(frame);
-    software_frame_holder_.reset();
-    DeliverMessages();
-  }
-  PopulateCommonParams(common_renderer_params);
+}
+
+void SynchronousCompositorProxy::SwapBuffersSw(cc::CompositorFrame* frame) {
+  DCHECK(inside_receive_);
+  DCHECK(software_draw_reply_);
+  DCHECK(frame);
+  SendDemandDrawSwReply(true, frame, software_draw_reply_);
+  inside_receive_ = false;
+}
+
+void SynchronousCompositorProxy::SendDemandDrawSwReply(
+    bool success,
+    cc::CompositorFrame* frame,
+    IPC::Message* reply_message) {
+  SyncCompositorCommonRendererParams common_renderer_params;
+  PopulateCommonParams(&common_renderer_params);
+  // Not using WriteParams because cc::CompositorFrame is not copy-able.
+  IPC::ParamTraits<bool>::Write(reply_message, success);
+  IPC::ParamTraits<SyncCompositorCommonRendererParams>::Write(
+      reply_message, common_renderer_params);
+  IPC::ParamTraits<cc::CompositorFrame>::Write(reply_message, *frame);
+  Send(reply_message);
 }
 
 void SynchronousCompositorProxy::SwapBuffers(cc::CompositorFrame* frame) {
+  DCHECK(hardware_draw_reply_ || software_draw_reply_);
+  DCHECK(!(hardware_draw_reply_ && software_draw_reply_));

[hush (inactive), 2015/12/30 21:51:09]

the above 2 lines can be combined into an xor on the 2 booleans:
!hardware_draw_reply_  != !software_draw_reply_

[boliu, 2015/12/30 22:17:46]

Those aren't booleans though

   if (hardware_draw_reply_) {
     SwapBuffersHw(frame);
-    return;
+  } else if (software_draw_reply_) {
+    SwapBuffersSw(frame);
   }
-
-  DCHECK(!software_frame_holder_);
-  software_frame_holder_.reset(new cc::CompositorFrame);
-  frame->AssignTo(software_frame_holder_.get());
 }
 
 void SynchronousCompositorProxy::OnComputeScroll(
     const SyncCompositorCommonBrowserParams& common_params,
     base::TimeTicks animation_time,
     SyncCompositorCommonRendererParams* common_renderer_params) {
   DCHECK(!inside_receive_);
   base::AutoReset<bool> scoped_inside_receive(&inside_receive_, true);
 
   ProcessCommonParams(common_params);
@@ skipping 23 matching lines @@
     total_scroll_offset_ = common_params.root_scroll_offset;
     input_handler_proxy_->SynchronouslySetRootScrollOffset(
         total_scroll_offset_);
   }
   if (!common_params.ack.resources.empty()) {
     output_surface_->ReturnResources(common_params.ack);
   }
 }
 
 }  // namespace content