sync compositor: Memory policy async IPC

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 299ef5052e2a231a874faafed1264cd5ab5ff606..89645711e7ea52446e895ad67441724b710c43b8 100644
--- a/content/browser/android/synchronous_compositor_host.cc
+++ b/content/browser/android/synchronous_compositor_host.cc
@@ -96,6 +96,8 @@ SynchronousCompositorHost::~SynchronousCompositorHost() {
 bool SynchronousCompositorHost::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(SynchronousCompositorHost, message)
+    IPC_MESSAGE_HANDLER(SyncCompositorHostMsg_OutputSurfaceCreated,
+                        OutputSurfaceCreated)
     IPC_MESSAGE_HANDLER(SyncCompositorHostMsg_UpdateState, ProcessCommonParams)
     IPC_MESSAGE_HANDLER(SyncCompositorHostMsg_OverScroll, OnOverScroll)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -316,8 +318,11 @@ void SynchronousCompositorHost::ReturnResources(
 void SynchronousCompositorHost::SetMemoryPolicy(size_t bytes_limit) {

[hush (inactive), 2016/05/12 19:23:03]

again, for multiprocess cross-bit (32bit renderer + 64bit browser) , we need
fixed bit types across IPC. So size_t needs to be uint32_t here.
I'm not sure clang plugins work, but this should have been caught by clang
plugins introduced by this CL?
https://codereview.chromium.org/1810243002/
Or maybe we're just not running these plugins?

[boliu, 2016/05/12 20:16:45]

As discussed over lunch. The wire format is uint32_t, so this is not an issue in
cross-bit. I agree on principle that implicit casting is still bad, but in
practice it's fine since the number won't exceed uint32, and fixing both sides
is way out of scope for this CL

   if (bytes_limit_ == bytes_limit)
     return;
-  bytes_limit_ = bytes_limit;
-  SendAsyncCompositorStateIfNeeded();
+
+  if (sender_->Send(
+          new SyncCompositorMsg_SetMemoryPolicy(routing_id_, bytes_limit))) {
+    bytes_limit_ = bytes_limit;
+  }
 }
 
 void SynchronousCompositorHost::DidChangeRootLayerScrollOffset(
@@ -419,6 +424,13 @@ void SynchronousCompositorHost::BeginFrame(const cc::BeginFrameArgs& args) {
   ProcessCommonParams(common_renderer_params);
 }
 
+void SynchronousCompositorHost::OutputSurfaceCreated() {
+  // New output surface is not aware of state from Browser side. So need to
+  // re-send all browser side state here.
+  sender_->Send(
+      new SyncCompositorMsg_SetMemoryPolicy(routing_id_, bytes_limit_));

[hush (inactive), 2016/05/12 19:23:03]

so... memory policy is *all* the browser side states? What about the states in
PopulateCommonParams?

[boliu, 2016/05/12 20:16:45]

It's not everything. I'm moving things to async IPCs, and out of
SynchronousCompositorProxy one at a time. If you go by state that's in
OutputSurface but *not* in Proxy, then yes, bytes_limit_ is the only one right
now.

[boliu, 2016/05/12 21:12:47]

Actually... this *is* everything for OutputSurface.

If you look at the remaining state in SyncCompositorCommonBrowserParams, scroll
goes to input handler, and begin_frame_source_paused goes to BeginFrameSource.
And that's it.

+}
+
 void SynchronousCompositorHost::OnOverScroll(
     const SyncCompositorCommonRendererParams& params,
     const DidOverscrollParams& over_scroll_params) {
@@ -429,7 +441,6 @@ void SynchronousCompositorHost::OnOverScroll(
 void SynchronousCompositorHost::PopulateCommonParams(
     SyncCompositorCommonBrowserParams* params) {
   DCHECK(params);
-  params->bytes_limit = bytes_limit_;
   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_;