RenderWidgetHostViewMac: Clean up surface synchronize

The function RWHVMac::SynchronizeVisualProperties was inappropriate in
a few ways:
- it belongs in BrowserCompositorMac -- we can see this because all of
  its calls are to BrowserCompositorMac (and two helper functions were
  added to BrowserCompositorMac to allow this function to be put in the
  wrong place)
- it calls DelegatedFrameHost::EmbedSurface, yet several of its callers
  also call DelegatedFrameHost::EmbedSurface just before calling it,
  usually with the almost-the-same-arguments (only differing by
  accident), making the call redundant
- similarly, it calls AllocateNewRendererLocalSurfaceId, which updates
  dfh_local_surface_id_allocator_, usually with arguments that were
  just retrieved from dfh_local_surface_id_allocator_, making it a
  convoluted no-op
- it has two different modes which should be two different functions,
  namely (1) force creation of a new surface id and (2) notify the
  host of a changed surface id

Make some motions in the direction of cleaning this up
- Rename BrowserCompositorMacClient::SynchronizeVisualProperties
  (which is fairly meaningless at this point) to
  OnBrowserCompositorSurfaceIdChanged.
- Also change this function to not take an optional argument. Inline
  the optional-argument-not-passed version of this function at its only
  callsite, RenderWidgetHostViewMac::
  EnsureSurfaceSynchronizedForLayoutTest.
- Rename BrowserCompositorMac::SynchronizeVisualProperties to
  UpdateSizeFromChild

Bug: 897156
Change-Id: Ib01aae5c0ef4e92298697c1840da7bc6e8cbf3a0
Reviewed-on: https://chromium-review.googlesource.com/c/1316678
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605865}

content/browser/renderer_host/browser_compositor_view_mac.h

@@ -34,8 +34,8 @@ class BrowserCompositorMacClient {
   virtual void BrowserCompositorMacOnBeginFrame(base::TimeTicks frame_time) = 0;
   virtual void OnFrameTokenChanged(uint32_t frame_token) = 0;
   virtual void DestroyCompositorForShutdown() = 0;
-  virtual bool SynchronizeVisualProperties(
-      const base::Optional<viz::LocalSurfaceIdAllocation>&
+  virtual bool OnBrowserCompositorSurfaceIdChanged(
+      const viz::LocalSurfaceIdAllocation&
           child_local_surface_id_allocation) = 0;
   virtual std::vector<viz::SurfaceId> CollectSurfaceIdsForEviction() = 0;
 };
@@ -82,12 +82,12 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
   // NSView. This will allocate a new SurfaceId if needed. This will return
   // true if any properties that need to be communicated to the
   // RenderWidgetHostImpl have changed.
-  bool UpdateNSViewAndDisplay(const gfx::Size& new_size_dip,
-                              const display::Display& new_display);
+  bool UpdateSurfaceFromNSView(const gfx::Size& new_size_dip,
+                               const display::Display& new_display);
 
   // Update the renderer's SurfaceId to reflect |new_size_in_pixels| in
   // anticipation of the NSView resizing during auto-resize.
-  void SynchronizeVisualProperties(
+  void UpdateSurfaceFromChild(
       float new_device_scale_factor,
       const gfx::Size& new_size_in_pixels,
       const viz::LocalSurfaceIdAllocation& child_local_surface_id_allocation);

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -91,7 +91,8 @@ bool BrowserCompositorMac::RequestRepaintForTesting() {
   delegated_frame_host_->EmbedSurface(
       new_local_surface_id_allocation.local_surface_id(), dfh_size_dip_,
       cc::DeadlinePolicy::UseExistingDeadline());
-  return client_->SynchronizeVisualProperties(new_local_surface_id_allocation);
+  return client_->OnBrowserCompositorSurfaceIdChanged(
+      new_local_surface_id_allocation);
 }
 
 const gfx::CALayerParams* BrowserCompositorMac::GetLastCALayerParams() const {
@@ -125,7 +126,7 @@ void BrowserCompositorMac::SetBackgroundColor(SkColor background_color) {
     recyclable_compositor_->compositor()->SetBackgroundColor(background_color_);
 }
 
-bool BrowserCompositorMac::UpdateNSViewAndDisplay(
+bool BrowserCompositorMac::UpdateSurfaceFromNSView(
     const gfx::Size& new_size_dip,
     const display::Display& new_display) {
   if (new_size_dip == dfh_size_dip_ && new_display == dfh_display_)
@@ -160,7 +161,7 @@ bool BrowserCompositorMac::UpdateNSViewAndDisplay(
   return true;
 }
 
-void BrowserCompositorMac::SynchronizeVisualProperties(
+void BrowserCompositorMac::UpdateSurfaceFromChild(
     float new_device_scale_factor,
     const gfx::Size& new_size_in_pixels,
     const viz::LocalSurfaceIdAllocation& child_local_surface_id_allocation) {
@@ -179,7 +180,8 @@ void BrowserCompositorMac::SynchronizeVisualProperties(
         dfh_local_surface_id_allocator_.GetCurrentLocalSurfaceId(),
         dfh_size_dip_, GetDeadlinePolicy(true /* is_resize */));
   }
-  client_->SynchronizeVisualProperties(child_local_surface_id_allocation);
+  client_->OnBrowserCompositorSurfaceIdChanged(
+      child_local_surface_id_allocation);
 }
 
 void BrowserCompositorMac::UpdateVSyncParameters(
@@ -368,7 +370,7 @@ void BrowserCompositorMac::DidNavigate() {
   delegated_frame_host_->EmbedSurface(
       local_surface_id_allocation.local_surface_id(), dfh_size_dip_,
       cc::DeadlinePolicy::UseExistingDeadline());
-  client_->SynchronizeVisualProperties(local_surface_id_allocation);
+  client_->OnBrowserCompositorSurfaceIdChanged(local_surface_id_allocation);
   delegated_frame_host_->DidNavigate();
   is_first_navigation_ = false;
 }
@@ -389,7 +391,7 @@ void BrowserCompositorMac::SetParentUiLayer(ui::Layer* new_parent_ui_layer) {
 bool BrowserCompositorMac::ForceNewSurfaceForTesting() {
   display::Display new_display(dfh_display_);
   new_display.set_device_scale_factor(new_display.device_scale_factor() * 2.0f);
-  return UpdateNSViewAndDisplay(dfh_size_dip_, new_display);
+  return UpdateSurfaceFromNSView(dfh_size_dip_, new_display);
 }
 
 void BrowserCompositorMac::GetRendererScreenInfo(

content/browser/renderer_host/render_widget_host_view_mac.h

@@ -392,9 +392,9 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   void BrowserCompositorMacOnBeginFrame(base::TimeTicks frame_time) override;
   void OnFrameTokenChanged(uint32_t frame_token) override;
   void DestroyCompositorForShutdown() override;
-  bool SynchronizeVisualProperties(
-      const base::Optional<viz::LocalSurfaceIdAllocation>&
-          child_local_surface_id_allocation) override;
+  bool OnBrowserCompositorSurfaceIdChanged(
+      const viz::LocalSurfaceIdAllocation& child_local_surface_id_allocation)
+      override;
   std::vector<viz::SurfaceId> CollectSurfaceIdsForEviction() override;
 
   // AcceleratedWidgetMacNSView implementation.

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -97,22 +97,15 @@ void RenderWidgetHostViewMac::DestroyCompositorForShutdown() {
   Destroy();
 }
 
-bool RenderWidgetHostViewMac::SynchronizeVisualProperties(
-    const base::Optional<viz::LocalSurfaceIdAllocation>&
-        child_local_surface_id_allocation) {
-  if (child_local_surface_id_allocation) {
-    browser_compositor_->UpdateRendererLocalSurfaceIdFromChild(
-        *child_local_surface_id_allocation);
-  } else {
-    browser_compositor_->AllocateNewRendererLocalSurfaceId();
-  }
-
+bool RenderWidgetHostViewMac::OnBrowserCompositorSurfaceIdChanged(
+    const viz::LocalSurfaceIdAllocation& child_local_surface_id_allocation) {
+  browser_compositor_->UpdateRendererLocalSurfaceIdFromChild(
+      child_local_surface_id_allocation);
   if (auto* host = browser_compositor_->GetDelegatedFrameHost()) {
     host->EmbedSurface(browser_compositor_->GetRendererLocalSurfaceId(),
                        browser_compositor_->GetRendererSize(),
                        cc::DeadlinePolicy::UseDefaultDeadline());
   }
-
   return host()->SynchronizeVisualProperties();
 }
 
@@ -405,7 +398,7 @@ void RenderWidgetHostViewMac::UpdateNSViewAndDisplayProperties() {
   // to send to the renderer, so it is required that BrowserCompositorMac be
   // updated first. Only notify RenderWidgetHostImpl of the update if any
   // properties it will query have changed.
-  if (browser_compositor_->UpdateNSViewAndDisplay(
+  if (browser_compositor_->UpdateSurfaceFromNSView(
           view_bounds_in_window_dip_.size(), display_)) {
     host()->NotifyScreenInfoChanged();
   }
@@ -822,7 +815,13 @@ void RenderWidgetHostViewMac::CopyFromSurface(
 
 void RenderWidgetHostViewMac::EnsureSurfaceSynchronizedForLayoutTest() {
   ++latest_capture_sequence_number_;
-  SynchronizeVisualProperties(base::nullopt);
+  browser_compositor_->AllocateNewRendererLocalSurfaceId();
+  if (auto* host = browser_compositor_->GetDelegatedFrameHost()) {
+    host->EmbedSurface(browser_compositor_->GetRendererLocalSurfaceId(),
+                       browser_compositor_->GetRendererSize(),
+                       cc::DeadlinePolicy::UseDefaultDeadline());
+  }
+  host()->SynchronizeVisualProperties();
 }
 
 void RenderWidgetHostViewMac::SetNeedsBeginFrames(bool needs_begin_frames) {
@@ -836,7 +835,7 @@ void RenderWidgetHostViewMac::UpdateNeedsBeginFramesInternal() {
 
 void RenderWidgetHostViewMac::OnDidUpdateVisualPropertiesComplete(
     const cc::RenderFrameMetadata& metadata) {
-  browser_compositor_->SynchronizeVisualProperties(
+  browser_compositor_->UpdateSurfaceFromChild(
       metadata.device_scale_factor, metadata.viewport_size_in_pixels,
       metadata.local_surface_id_allocation.value_or(
           viz::LocalSurfaceIdAllocation()));