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..4b2d36ac87e488bfb62fce59e2767078d611b4b1 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) {
   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,14 @@ void SynchronousCompositorHost::BeginFrame(const cc::BeginFrameArgs& args) {
   ProcessCommonParams(common_renderer_params);
 }
 
+void SynchronousCompositorHost::OutputSurfaceCreated(
+    uint32_t output_surface_id) {

[dcheng, 2016/05/12 03:13:18]

Doesn't appear to be used yet. How will it be used? What are the implications of
forging this ID?

[boliu, 2016/05/12 03:27:32]

It's the same ID in the reply of SyncCompositorMsg_DemandDrawHw. The ID is
opaque on browser side, and sent back to the renderer in
SyncCompositorMsg_ReclaimResources so that resources don't get returned to the
wrong output surface.

In theory, it could be used as key to maps in the browser process. But that
generally shouldn't (and doesn't) happen, as there is only one "current" output
surface and any previous output surface is considered lost, and it's ok to just
drop resources for lost output surface. (There is no ordering issues like zombie
output surface coming back to life.)


Taking a step back, I don't foresee using this id in this method. Only thing I
can think of is saving ReclaimResources IPCs by dropping resources on the
browser side. Doubt that could be a perf issue especially during lost output
surface. So I can just remove this id if you prefer that?

[boliu, 2016/05/12 14:03:27]

Dropped in PS3

+  // 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_));
+}
+
 void SynchronousCompositorHost::OnOverScroll(
     const SyncCompositorCommonRendererParams& params,
     const DidOverscrollParams& over_scroll_params) {
@@ -429,7 +442,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_;