Have RenderWidgetHostViewAndroid own sync compositor

content/browser/renderer_host/render_widget_host_view_android.cc

Index: content/browser/renderer_host/render_widget_host_view_android.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index c182e2982a386ead45fdc64039a74409dc498795..72c523dca6ee39d8bbc8ad9aea1e57c61fa05230 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -515,7 +515,7 @@ bool RenderWidgetHostViewAndroid::HasFocus() const {
 }
 
 bool RenderWidgetHostViewAndroid::IsSurfaceAvailableForCopy() const {
-  return HasValidFrame();
+  return !using_browser_compositor_ || HasValidFrame();
 }
 
 void RenderWidgetHostViewAndroid::Show() {
@@ -872,7 +872,7 @@ void RenderWidgetHostViewAndroid::CopyFromCompositingSurface(
     return;
   }
   base::TimeTicks start_time = base::TimeTicks::Now();
-  if (using_browser_compositor_ && !IsSurfaceAvailableForCopy()) {
+  if (!IsSurfaceAvailableForCopy()) {
     callback.Run(SkBitmap(), READBACK_SURFACE_UNAVAILABLE);
     return;
   }
@@ -1176,6 +1176,11 @@ void RenderWidgetHostViewAndroid::RetainFrame(
   last_frame_info_.reset(new LastFrameInfo(output_surface_id, frame.Pass()));
 }
 
+SynchronousCompositorImpl*
+RenderWidgetHostViewAndroid::GetSynchronousCompositorImpl() {
+  return sync_compositor_.get();
+}
+
 void RenderWidgetHostViewAndroid::SynchronousFrameMetadata(
     const cc::CompositorFrameMetadata& frame_metadata) {
   if (!content_view_core_)
@@ -1281,10 +1286,7 @@ void RenderWidgetHostViewAndroid::SynchronousCopyContents(
   int output_width = output_size_in_pixel.width();
   int output_height = output_size_in_pixel.height();
 
-  SynchronousCompositor* compositor =
-      SynchronousCompositorImpl::FromID(host_->GetProcess()->GetID(),
-                                        host_->GetRoutingID());
-  if (!compositor) {
+  if (!sync_compositor_) {
     callback.Run(SkBitmap(), READBACK_FAILED);
     return;
   }
@@ -1298,7 +1300,7 @@ void RenderWidgetHostViewAndroid::SynchronousCopyContents(
   canvas.scale(
       (float)output_width / (float)input_size_in_pixel.width(),
       (float)output_height / (float)input_size_in_pixel.height());
-  compositor->DemandDrawSw(&canvas);
+  sync_compositor_->DemandDrawSw(&canvas);
   callback.Run(bitmap, READBACK_SUCCESS);
 }
 
@@ -1482,16 +1484,12 @@ void RenderWidgetHostViewAndroid::SendBeginFrame(base::TimeTicks frame_time,
         host_->GetRoutingID(),
         cc::BeginFrameArgs::Create(BEGINFRAME_FROM_HERE, frame_time, deadline,
                                    vsync_period, cc::BeginFrameArgs::NORMAL)));
-  } else {
-    SynchronousCompositorImpl* compositor = SynchronousCompositorImpl::FromID(
-        host_->GetProcess()->GetID(), host_->GetRoutingID());
-    if (compositor) {
-      // The synchronous compositor synchronously does it's work in this call.
-      // It does not use a deadline.
-      compositor->BeginFrame(cc::BeginFrameArgs::Create(
-          BEGINFRAME_FROM_HERE, frame_time, base::TimeTicks(), vsync_period,
-          cc::BeginFrameArgs::NORMAL));
-    }
+  } else if (sync_compositor_) {
+    // The synchronous compositor synchronously does it's work in this call.
+    // It does not use a deadline.
+    sync_compositor_->BeginFrame(cc::BeginFrameArgs::Create(
+        BEGINFRAME_FROM_HERE, frame_time, base::TimeTicks(), vsync_period,
+        cc::BeginFrameArgs::NORMAL));
   }
 }
 
@@ -1609,11 +1607,8 @@ InputEventAckState RenderWidgetHostViewAndroid::FilterInputEvent(
       shim->Send(new GpuMsg_WakeUpGpu);
   }
 
-  SynchronousCompositorImpl* compositor =
-      SynchronousCompositorImpl::FromID(host_->GetProcess()->GetID(),
-                                          host_->GetRoutingID());
-  if (compositor)
-    return compositor->HandleInputEvent(input_event);
+  if (sync_compositor_)
+    return sync_compositor_->HandleInputEvent(input_event);
   return INPUT_EVENT_ACK_STATE_NOT_CONSUMED;
 }
 
@@ -1745,6 +1740,8 @@ uint32_t RenderWidgetHostViewAndroid::GetSurfaceIdNamespace() {
 
 void RenderWidgetHostViewAndroid::SetContentViewCore(
     ContentViewCoreImpl* content_view_core) {
+  DCHECK(!content_view_core || !content_view_core_ ||
+         (content_view_core_ == content_view_core));
   RemoveLayers();
   StopObservingRootWindow();
 
@@ -1772,9 +1769,10 @@ void RenderWidgetHostViewAndroid::SetContentViewCore(
   }
 
   AttachLayers();
-
-  if (!content_view_core_)
+  if (!content_view_core_) {
+    sync_compositor_.reset();
     return;
+  }
 
   if (is_showing_)
     StartObservingRootWindow();
@@ -1789,6 +1787,11 @@ void RenderWidgetHostViewAndroid::SetContentViewCore(
       content_view_core_window_android_->GetCompositor()) {
     overscroll_controller_ = CreateOverscrollController(content_view_core_);
   }
+
+  if (!sync_compositor_) {
+    sync_compositor_ = SynchronousCompositorImpl::Create(
+        this, content_view_core_->GetWebContents());

[no sievers, 2015/10/08 22:39:29]

It's a bit ugly to see WebContents in RWHV. Really this is supposed to be
decoupled (the browser-owned object vs. the one with lifetime coupled to
renderer-side object). (*)

Maybe you can at least hide it in CVC by having something like
ContentViewCore::GetSynchronousCompositorClient()?

(*) But so is the nature of SetCVC() and all of this here - I think what we'd
really want to have for all of this is another level of indirection that's more
like how this traditionally was supposed to work, where each RWHV represents its
own platform window instance.

[boliu, 2015/10/09 02:22:16]

The practical argument: lots of code here already does this. I don't think it's
worth "hiding" this one call.
IMO there is nothing wrong with accessing objects with lifetimes tied to
WebContents here. But maybe it could be like a delegate interface instead of
accessing the types directly. (Did I just rephrase exactly what you said..?)

+  }
 }
 
 void RenderWidgetHostViewAndroid::RunAckCallbacks(