Surface synchronization: Simplify OOPIF and BrowserPlugin

With CL 669444, if a primary SurfaceInfo is specified on a layer
and no fallback is specified, then SurfaceAggregator will use an
embedder provided default background color to fill the space of the
primary Surface if the primary Surface is unavailable at surface
aggregation time.

Previously, SurfaceAggregator reported this case as an error and
logged UMA for a missing surface. Given this state is no longer considered
an error, there is room for simplifying code in BrowserPlugin and
RenderFrameProxy.

The default background color is set to SK_ColorTRANSPARENT to match the
current behavior. The primary surface is set as soon as it can be and
doesn't wait for the first fallback surface to become available.

Bug: 672962
Change-Id: I0ec6153356978e8994b50bfd4a024369e6f320d7
Reviewed-on: https://chromium-review.googlesource.com/686354
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504869}

content/browser/renderer_host/render_widget_host_view_aura.cc

@@ -1587,13 +1587,6 @@ bool RenderWidgetHostViewAura::HasHitTestMask() const {
 void RenderWidgetHostViewAura::GetHitTestMask(gfx::Path* mask) const {
 }
 
-void RenderWidgetHostViewAura::OnFirstSurfaceActivation(
-    const viz::SurfaceInfo& surface_info) {
-  if (!is_guest_view_hack_)
-    return;
-  host_->GetView()->OnFirstSurfaceActivation(surface_info);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewAura, ui::EventHandler implementation:
 

content/browser/renderer_host/render_widget_host_view_aura.h

@@ -261,7 +261,6 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
   void OnWindowTargetVisibilityChanged(bool visible) override;
   bool HasHitTestMask() const override;
   void GetHitTestMask(gfx::Path* mask) const override;
-  void OnFirstSurfaceActivation(const viz::SurfaceInfo& surface_info) override;
 
   // Overridden from ui::EventHandler:
   void OnKeyEvent(ui::KeyEvent* event) override;

content/browser/renderer_host/render_widget_host_view_base.h

@@ -64,7 +64,6 @@ struct DidOverscrollParams;
 
 namespace viz {
 class SurfaceHittestDelegate;
-class SurfaceInfo;
 }
 
 namespace content {
@@ -235,7 +234,6 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView,
       viz::CompositorFrame frame) = 0;
 
   virtual void OnDidNotProduceFrame(const viz::BeginFrameAck& ack) {}
-  virtual void OnFirstSurfaceActivation(const viz::SurfaceInfo& surface_info) {}
 
   // This method exists to allow removing of displayed graphics, after a new
   // page has been loaded, to prevent the displayed URL from being out of sync

content/renderer/browser_plugin/browser_plugin.cc

@@ -104,9 +104,6 @@ BrowserPlugin::BrowserPlugin(
 BrowserPlugin::~BrowserPlugin() {
   Detach();
 
-  if (compositing_helper_)
-    compositing_helper_->OnContainerDestroy();
-
   if (delegate_) {
     delegate_->DidDestroyElement();
     delegate_.reset();
@@ -139,26 +136,6 @@ void BrowserPlugin::OnSetChildFrameSurface(
   if (!attached())
     return;
 
-  if (!compositing_helper_) {
-    compositing_helper_.reset(
-        ChildFrameCompositingHelper::CreateForBrowserPlugin(
-            weak_ptr_factory_.GetWeakPtr()));
-    if (enable_surface_synchronization_) {
-      // We wait until there is a single CompositorFrame guaranteed to be
-      // available and ready for display in the display compositor before using
-      // surface synchronization. This guarantees that we will have something to
-      // display when the compositor goes to produce a display frame.
-      //
-      // Once there's an available fallback surface that can be employed, then
-      // the primary surface is updated as soon as the frame rect changes.
-      //
-      // The compositor will attempt to composite the primary surface within a
-      // give deadline (4 frames is the default). If the primary surface isn't
-      // available for four frames, then the fallback surface will be used.
-      compositing_helper_->SetPrimarySurfaceInfo(surface_info);
-    }
-  }
-
   if (!enable_surface_synchronization_)
     compositing_helper_->SetPrimarySurfaceInfo(surface_info);
   compositing_helper_->SetFallbackSurfaceInfo(surface_info, sequence);
@@ -226,10 +203,7 @@ void BrowserPlugin::Detach() {
 
   attached_ = false;
   guest_crashed_ = false;
-  if (compositing_helper_) {
   compositing_helper_->OnContainerDestroy();
-    compositing_helper_.reset();
-  }
 
   BrowserPluginManager::Get()->Send(
       new BrowserPluginHostMsg_Detach(browser_plugin_instance_id_));
@@ -250,12 +224,6 @@ void BrowserPlugin::OnAdvanceFocus(int browser_plugin_instance_id,
 
 void BrowserPlugin::OnGuestGone(int browser_plugin_instance_id) {
   guest_crashed_ = true;
-
-  if (!compositing_helper_) {
-    compositing_helper_.reset(
-        ChildFrameCompositingHelper::CreateForBrowserPlugin(
-            weak_ptr_factory_.GetWeakPtr()));
-  }
   compositing_helper_->ChildFrameGone();
 }
 
@@ -327,8 +295,7 @@ void BrowserPlugin::ViewRectsChanged(const gfx::Rect& view_rect) {
       !view_rect_ || view_rect_->size() != view_rect.size();
   if (rect_size_changed || !local_surface_id_.is_valid()) {
     local_surface_id_ = local_surface_id_allocator_.GenerateId();
-    if (compositing_helper_ && enable_surface_synchronization_ &&
-        frame_sink_id_.is_valid()) {
+    if (enable_surface_synchronization_ && frame_sink_id_.is_valid()) {
       RenderWidget* render_widget =
           RenderFrameImpl::FromWebFrame(Container()->GetDocument().GetFrame())
               ->GetRenderWidget();
@@ -395,6 +362,10 @@ bool BrowserPlugin::Initialize(WebPluginContainer* container) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(&BrowserPlugin::UpdateInternalInstanceId,
                                 weak_ptr_factory_.GetWeakPtr()));
+
+  compositing_helper_.reset(ChildFrameCompositingHelper::CreateForBrowserPlugin(
+      weak_ptr_factory_.GetWeakPtr()));
+
   return true;
 }
 
@@ -483,7 +454,6 @@ void BrowserPlugin::UpdateVisibility(bool visible) {
   if (!attached())
     return;
 
-  if (compositing_helper_)
   compositing_helper_->UpdateVisibility(visible);
 
   BrowserPluginManager::Get()->Send(new BrowserPluginHostMsg_SetVisibility(

content/renderer/child_frame_compositing_helper.cc

@@ -173,11 +173,12 @@ ChildFrameCompositingHelper::ChildFrameCompositingHelper(
   enable_surface_references_ =
       !base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableSurfaceReferences);
-  scoped_refptr<ThreadSafeSender> sender(
-      RenderThreadImpl::current()->thread_safe_sender());
   if (enable_surface_references_) {
     surface_reference_factory_ = new viz::StubSurfaceReferenceFactory();
-  } else if (render_frame_proxy_) {
+  } else {
+    scoped_refptr<ThreadSafeSender> sender(
+        RenderThreadImpl::current()->thread_safe_sender());
+    if (render_frame_proxy_) {
       surface_reference_factory_ =
           new IframeSurfaceReferenceFactory(sender, host_routing_id_);
     } else {
@@ -185,6 +186,7 @@ ChildFrameCompositingHelper::ChildFrameCompositingHelper(
           sender, host_routing_id_,
           browser_plugin_->browser_plugin_instance_id());
     }
+  }
 }
 
 ChildFrameCompositingHelper::~ChildFrameCompositingHelper() {
@@ -271,6 +273,7 @@ void ChildFrameCompositingHelper::SetPrimarySurfaceInfo(
 
   surface_layer_ = cc::SurfaceLayer::Create(surface_reference_factory_);
   surface_layer_->SetMasksToBounds(true);
+  surface_layer_->SetDefaultBackgroundColor(SK_ColorTRANSPARENT);
 
   viz::SurfaceInfo modified_surface_info(surface_info.id(), scale_factor,
                                          surface_info.size_in_pixels());

content/renderer/render_frame_proxy.cc

@@ -218,6 +218,8 @@ void RenderFrameProxy::Init(blink::WebRemoteFrame* web_frame,
       *base::CommandLine::ForCurrentProcess();
   enable_surface_synchronization_ =
       command_line.HasSwitch(switches::kEnableSurfaceSynchronization);
+  compositing_helper_.reset(
+      ChildFrameCompositingHelper::CreateForRenderFrameProxy(this));
 }
 
 void RenderFrameProxy::ResendFrameRects() {
@@ -228,7 +230,7 @@ void RenderFrameProxy::ResendFrameRects() {
 }
 
 void RenderFrameProxy::WillBeginCompositorFrame() {
-  if (compositing_helper_ && compositing_helper_->surface_id().is_valid()) {
+  if (compositing_helper_->surface_id().is_valid()) {
     FrameHostMsg_HittestData_Params params;
     params.surface_id = compositing_helper_->surface_id();
     params.ignored_for_hittest = web_frame_->IsIgnoredForHitTest();
@@ -335,7 +337,6 @@ void RenderFrameProxy::OnDeleteProxy() {
 }
 
 void RenderFrameProxy::OnChildFrameProcessGone() {
-  if (compositing_helper_)
   compositing_helper_->ChildFrameGone();
 }
 
@@ -349,25 +350,6 @@ void RenderFrameProxy::OnSetChildFrameSurface(
   if (!web_frame()->Parent())
     return;
 
-  if (!compositing_helper_) {
-    compositing_helper_.reset(
-        ChildFrameCompositingHelper::CreateForRenderFrameProxy(this));
-    if (enable_surface_synchronization_) {
-      // We wait until there is a single CompositorFrame guaranteed to be
-      // available and ready for display in the display compositor before using
-      // surface synchronization. This guarantees that we will have something to
-      // display when the compositor goes to produce a display frame.
-      //
-      // Once there's an available fallback surface that can be employed, then
-      // the primary surface is updated as soon as the frame rect changes.
-      //
-      // The compositor will attempt to composite the primary surface within a
-      // give deadline (4 frames is the default). If the primary surface isn't
-      // available for four frames, then the fallback surface will be used.
-      compositing_helper_->SetPrimarySurfaceInfo(surface_info);
-    }
-  }
-
   if (!enable_surface_synchronization_)
     compositing_helper_->SetPrimarySurfaceInfo(surface_info);
   compositing_helper_->SetFallbackSurfaceInfo(surface_info, sequence);
@@ -544,8 +526,7 @@ void RenderFrameProxy::FrameRectsChanged(const blink::WebRect& frame_rect) {
   gfx::Rect rect = frame_rect;
   if (frame_rect_.size() != rect.size() || !local_surface_id_.is_valid()) {
     local_surface_id_ = local_surface_id_allocator_.GenerateId();
-    if (compositing_helper_ && enable_surface_synchronization_ &&
-        frame_sink_id_.is_valid()) {
+    if (enable_surface_synchronization_ && frame_sink_id_.is_valid()) {
       float device_scale_factor =
           render_widget()->GetOriginalDeviceScaleFactor();
       viz::SurfaceInfo surface_info(