mac: Ensure surface invariants are not violated

Make BrowserCompositorViewMac be the source of ScreenInfo, and merge
the update for screen properties with the resize update as
BrowserCompositorViewMac::UpdateNSViewAndDisplay.

Merge all RenderWidgetHostViewMac paths that update size or screen
info into RenderWidgetHostViewMac::UpdateNSViewAndDisplayProperties,
which communicates the changes (if any) to RenderWidgetHostImpl.

Change-Id: Idc7f654c93d0622815825fdb6fff46afcb38767e
Reviewed-on: https://chromium-review.googlesource.com/918284
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537240}

content/browser/renderer_host/browser_compositor_view_mac.h

@@ -14,6 +14,7 @@
 #include "content/browser/renderer_host/delegated_frame_host.h"
 #include "ui/compositor/compositor.h"
 #include "ui/compositor/compositor_observer.h"
+#include "ui/display/display.h"
 
 namespace ui {
 class AcceleratedWidgetMacNSView;
@@ -61,20 +62,20 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient {
       viz::mojom::CompositorFrameSinkClient* renderer_compositor_frame_sink);
   void OnDidNotProduceFrame(const viz::BeginFrameAck& ack);
   void SetBackgroundColor(SkColor background_color);
-  void SetDisplayColorSpace(const gfx::ColorSpace& color_space);
   void UpdateVSyncParameters(const base::TimeTicks& timebase,
                              const base::TimeDelta& interval);
   void SetNeedsBeginFrames(bool needs_begin_frames);
   void SetWantsAnimateOnlyBeginFrames();
 
   // Update the renderer's SurfaceId to reflect the current dimensions of the
-  // NSView. This will allocate a new SurfaceId if the dimensions have indeed
-  // changed.
-  void OnNSViewWasResized();
+  // 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();
 
   // Update the renderer's SurfaceId to reflect |size_dip| in anticipation of
   // the NSView resizing during auto-resize.
-  void OnNSViewWillAutoResize(const gfx::Size& size_dip);
+  void UpdateForAutoResize(const gfx::Size& size_dip);
 
   // This is used to ensure that the ui::Compositor be attached to the
   // DelegatedFrameHost while the RWHImpl is visible.
@@ -90,11 +91,10 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient {
 
   viz::FrameSinkId GetRootFrameSinkId();
 
-  const gfx::Size& GetRendererSize() const {
-    return delegated_frame_host_size_dip_;
-  }
+  const gfx::Size& GetRendererSize() const { return dfh_size_dip_; }
+  void GetRendererScreenInfo(ScreenInfo* screen_info) const;
   const viz::LocalSurfaceId& GetRendererLocalSurfaceId() const {
-    return delegated_frame_host_surface_id_;
+    return dfh_surface_id_;
   }
 
   // Indicate that the recyclable compositor should be destroyed, and no future
@@ -175,16 +175,14 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient {
 
   // The surface for the delegated frame host, rendered into by the renderer
   // process.
-  void UpdateDelegatedFrameHostSurface(const gfx::Size& size_dip,
-                                       float scale_factor);
-  viz::LocalSurfaceId delegated_frame_host_surface_id_;
-  gfx::Size delegated_frame_host_size_pixels_;
-  gfx::Size delegated_frame_host_size_dip_;
-  float delegated_frame_host_scale_factor_ = 1.f;
-
-  // The surface for the ui::Compositor, which will embed
-  // |delegated_frame_host_surface_id_| into its tree. Updated to match the
-  // delegated frame host values when attached and at OnFirstSurfaceActivation.
+  viz::LocalSurfaceId dfh_surface_id_;
+  gfx::Size dfh_size_pixels_;
+  gfx::Size dfh_size_dip_;
+  display::Display dfh_display_;
+
+  // The surface for the ui::Compositor, which will embed |dfh_surface_id_|
+  // into its tree. Updated to match the delegated frame host values when
+  // attached and at OnFirstSurfaceActivation.
   viz::LocalSurfaceId compositor_surface_id_;
   gfx::Size compositor_size_pixels_;
   float compositor_scale_factor_ = 1.f;

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -15,6 +15,7 @@
 #include "components/viz/common/features.h"
 #include "content/browser/compositor/image_transport_factory.h"
 #include "content/browser/renderer_host/compositor_resize_lock.h"
+#include "content/browser/renderer_host/display_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/context_factory.h"
 #include "ui/accelerated_widget_mac/accelerated_widget_mac.h"
@@ -189,6 +190,8 @@ BrowserCompositorMac::BrowserCompositorMac(
       frame_sink_id, this, features::IsSurfaceSynchronizationEnabled(),
       base::FeatureList::IsEnabled(features::kVizDisplayCompositor)));
 
+  dfh_display_ = display::Screen::GetScreen()->GetDisplayNearestView(nil);
+
   SetRenderWidgetHostIsHidden(render_widget_host_is_hidden);
   SetNSViewAttachedToWindow(ns_view_attached_to_window);
 }
@@ -259,44 +262,53 @@ void BrowserCompositorMac::SetBackgroundColor(SkColor background_color) {
   }
 }
 
-void BrowserCompositorMac::SetDisplayColorSpace(
-    const gfx::ColorSpace& color_space) {
-  if (recyclable_compositor_)
-    recyclable_compositor_->compositor()->SetDisplayColorSpace(color_space);
-}
+bool BrowserCompositorMac::UpdateNSViewAndDisplay() {
+  NSView* ns_view =
+      accelerated_widget_mac_ns_view_->AcceleratedWidgetGetNSView();
+  display::Display new_display =
+      display::Screen::GetScreen()->GetDisplayNearestView(ns_view);
+  gfx::Size new_size_dip([ns_view bounds].size);
+  if (new_size_dip == dfh_size_dip_ && new_display == dfh_display_)
+    return false;
 
-void BrowserCompositorMac::OnNSViewWasResized() {
-  gfx::Size size_dip;
-  float scale_factor = 1.f;
-  GetViewProperties(&size_dip, &scale_factor, nullptr);
-  UpdateDelegatedFrameHostSurface(size_dip, scale_factor);
-}
+  bool needs_new_surface_id =
+      new_size_dip != dfh_size_dip_ ||
+      new_display.device_scale_factor() != dfh_display_.device_scale_factor();
+
+  dfh_display_ = new_display;
+  dfh_size_dip_ = new_size_dip;
+  dfh_size_pixels_ = gfx::ConvertSizeToPixel(dfh_display_.device_scale_factor(),
+                                             dfh_size_dip_);
+
+  if (needs_new_surface_id) {
+    dfh_surface_id_ = parent_local_surface_id_allocator_.GenerateId();
+    if (recyclable_compositor_)
+      recyclable_compositor_->Suspend();
+    GetDelegatedFrameHost()->WasResized(
+        dfh_surface_id_, dfh_size_dip_,
+        cc::DeadlinePolicy::UseExistingDeadline());
+  }
 
-void BrowserCompositorMac::OnNSViewWillAutoResize(const gfx::Size& size_dip) {
-  UpdateDelegatedFrameHostSurface(size_dip, delegated_frame_host_scale_factor_);
+  if (recyclable_compositor_) {
+    recyclable_compositor_->compositor()->SetDisplayColorSpace(
+        dfh_display_.color_space());
+  }
+  return true;
 }
 
-void BrowserCompositorMac::UpdateDelegatedFrameHostSurface(
-    const gfx::Size& size_dip,
-    float scale_factor) {
-  if (size_dip == delegated_frame_host_size_dip_ &&
-      scale_factor == delegated_frame_host_scale_factor_) {
+void BrowserCompositorMac::UpdateForAutoResize(const gfx::Size& new_size_dip) {
+  if (new_size_dip == dfh_size_dip_)
     return;
-  }
 
-  // Suspend the ui::Compositor until we get a renderer frame of the new size.
+  dfh_size_dip_ = new_size_dip;
+  dfh_size_pixels_ = gfx::ConvertSizeToPixel(dfh_display_.device_scale_factor(),
+                                             dfh_size_dip_);
+  dfh_surface_id_ = parent_local_surface_id_allocator_.GenerateId();
+
   if (recyclable_compositor_)
     recyclable_compositor_->Suspend();
-
-  delegated_frame_host_surface_id_ =
-      parent_local_surface_id_allocator_.GenerateId();
-  delegated_frame_host_size_dip_ = size_dip;
-  delegated_frame_host_scale_factor_ = scale_factor;
-  delegated_frame_host_size_pixels_ = gfx::ConvertSizeToPixel(
-      delegated_frame_host_scale_factor_, delegated_frame_host_size_dip_);
-
   GetDelegatedFrameHost()->WasResized(
-      delegated_frame_host_surface_id_, delegated_frame_host_size_dip_,
+      dfh_surface_id_, dfh_size_dip_,
       cc::DeadlinePolicy::UseExistingDeadline());
 }
 
@@ -334,6 +346,8 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
     recyclable_compositor_ = RecyclableCompositorMac::Create();
     recyclable_compositor_->compositor()->SetRootLayer(root_layer_.get());
     recyclable_compositor_->compositor()->SetBackgroundColor(background_color_);
+    recyclable_compositor_->compositor()->SetDisplayColorSpace(
+        dfh_display_.color_space());
     recyclable_compositor_->accelerated_widget_mac()->SetNSView(
         accelerated_widget_mac_ns_view_);
     state_ = HasDetachedCompositor;
@@ -342,22 +356,17 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
   // Transition HasDetachedCompositor -> HasAttachedCompositor.
   if (state_ == HasDetachedCompositor && new_state == HasAttachedCompositor) {
     delegated_frame_host_->SetCompositor(recyclable_compositor_->compositor());
-    delegated_frame_host_->WasShown(delegated_frame_host_surface_id_,
-                                    delegated_frame_host_size_dip_,
+    delegated_frame_host_->WasShown(dfh_surface_id_, dfh_size_dip_,
                                     ui::LatencyInfo());
 
-    gfx::ColorSpace color_space;
-    GetViewProperties(nullptr, nullptr, &color_space);
-    recyclable_compositor_->compositor()->SetDisplayColorSpace(color_space);
-
     // If there exists a saved frame ready to display, unsuspend the compositor
     // now (if one is not ready, the compositor will unsuspend on first surface
     // activation).
     if (delegated_frame_host_->HasSavedFrame()) {
-      if (compositor_scale_factor_ != delegated_frame_host_scale_factor_ ||
-          compositor_size_pixels_ != delegated_frame_host_size_pixels_) {
-        compositor_scale_factor_ = delegated_frame_host_scale_factor_;
-        compositor_size_pixels_ = delegated_frame_host_size_pixels_;
+      if (compositor_scale_factor_ != dfh_display_.device_scale_factor() ||
+          compositor_size_pixels_ != dfh_size_pixels_) {
+        compositor_scale_factor_ = dfh_display_.device_scale_factor();
+        compositor_size_pixels_ = dfh_size_pixels_;
         compositor_surface_id_ =
             parent_local_surface_id_allocator_.GenerateId();
         root_layer_->SetBounds(gfx::Rect(gfx::ConvertSizeToDIP(
@@ -399,26 +408,6 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
   }
 }
 
-void BrowserCompositorMac::GetViewProperties(
-    gfx::Size* bounds_in_dip,
-    float* scale_factor,
-    gfx::ColorSpace* color_space) const {
-  NSView* ns_view =
-      accelerated_widget_mac_ns_view_->AcceleratedWidgetGetNSView();
-  if (bounds_in_dip) {
-    NSSize dip_ns_size = [ns_view bounds].size;
-    *bounds_in_dip = gfx::Size(dip_ns_size.width, dip_ns_size.height);
-  }
-  if (scale_factor || color_space) {
-    display::Display display =
-        display::Screen::GetScreen()->GetDisplayNearestView(ns_view);
-    if (scale_factor)
-      *scale_factor = display.device_scale_factor();
-    if (color_space)
-      *color_space = display.color_space();
-  }
-}
-
 // static
 void BrowserCompositorMac::DisableRecyclingForShutdown() {
   g_has_shut_down = true;
@@ -484,8 +473,8 @@ void BrowserCompositorMac::OnFirstSurfaceActivation(
   // content is drawn in the GPU process, it may change before we want it to).
   if (repaint_state_ == RepaintState::Paused) {
     bool compositor_is_nsview_size =
-        compositor_size_pixels_ == delegated_frame_host_size_pixels_ &&
-        compositor_scale_factor_ == delegated_frame_host_scale_factor_;
+        compositor_size_pixels_ == dfh_size_pixels_ &&
+        compositor_scale_factor_ == dfh_display_.device_scale_factor();
     if (compositor_is_nsview_size || repaint_auto_resize_enabled_) {
       NSDisableScreenUpdates();
       repaint_state_ = RepaintState::ScreenUpdatesDisabled;
@@ -513,10 +502,8 @@ ui::Compositor* BrowserCompositorMac::CompositorForTesting() const {
 }
 
 void BrowserCompositorMac::DidNavigate() {
-  delegated_frame_host_surface_id_ =
-      parent_local_surface_id_allocator_.GenerateId();
-  delegated_frame_host_->WasResized(delegated_frame_host_surface_id_,
-                                    delegated_frame_host_size_dip_,
+  dfh_surface_id_ = parent_local_surface_id_allocator_.GenerateId();
+  delegated_frame_host_->WasResized(dfh_surface_id_, dfh_size_dip_,
                                     cc::DeadlinePolicy::UseExistingDeadline());
   delegated_frame_host_->DidNavigate();
 }
@@ -541,7 +528,12 @@ bool BrowserCompositorMac::ShouldContinueToPauseForFrame() const {
     return false;
 
   return !recyclable_compositor_->accelerated_widget_mac()->HasFrameOfSize(
-      delegated_frame_host_size_dip_);
+      dfh_size_dip_);
+}
+
+void BrowserCompositorMac::GetRendererScreenInfo(
+    ScreenInfo* screen_info) const {
+  DisplayUtil::DisplayToScreenInfo(screen_info, dfh_display_);
 }
 
 }  // namespace content

content/browser/renderer_host/render_widget_host_view_mac.h

@@ -296,6 +296,7 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
   void SetBackgroundColor(SkColor color) override;
   SkColor background_color() const override;
   void SetNeedsBeginFrames(bool needs_begin_frames) override;
+  void GetScreenInfo(ScreenInfo* screen_info) const override;
   void SetWantsAnimateOnlyBeginFrames() override;
 
   // Implementation of RenderWidgetHostViewBase.
@@ -335,7 +336,6 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
 
   bool ShouldContinueToPauseForFrame() override;
   gfx::Rect GetBoundsInRootWindow() override;
-  void OnSynchronizedDisplayPropertiesChanged() override;
   void ResizeDueToAutoResize(const gfx::Size& new_size,
                              uint64_t sequence_number) override;
   void DidNavigate() override;
@@ -476,9 +476,10 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
 
   int window_number() const;
 
-  // Update properties, such as the scale factor for the backing store
-  // and for any CALayers, and the screen color profile.
-  void UpdateBackingStoreProperties();
+  // Update the size, scale factor, color profile, and any other properties
+  // of the NSView or its NSScreen. Propagate these to the RenderWidgetHostImpl
+  // as well.
+  void UpdateNSViewAndDisplayProperties();
 
   // Ensure that the display link is associated with the correct display.
   void UpdateDisplayLink();

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -726,12 +726,30 @@ RenderWidgetHostViewMac::GetFocusedRenderWidgetHostDelegate() {
   return render_widget_host_->delegate();
 }
 
-void RenderWidgetHostViewMac::UpdateBackingStoreProperties() {
-  UpdateScreenInfo(cocoa_view_);
-  // Update the ui::Compositor's color space here. The other display properties
-  // of the ui::Compositor are updated by frame metadata.
-  if (browser_compositor_)
-    browser_compositor_->SetDisplayColorSpace(current_display_color_space_);
+void RenderWidgetHostViewMac::UpdateNSViewAndDisplayProperties() {
+  if (!render_widget_host_)
+    return;
+
+  // During auto-resize it is the responsibility of the caller to ensure that
+  // the NSView and RenderWidgetHostImpl are kept in sync.
+  if (render_widget_host_->auto_resize_enabled())
+    return;
+
+  if (render_widget_host_->delegate())
+    render_widget_host_->delegate()->SendScreenRects();
+  else
+    render_widget_host_->SendScreenRects();
+
+  // RenderWidgetHostImpl will query BrowserCompositorMac for the dimensions
+  // 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())
+    render_widget_host_->NotifyScreenInfoChanged();
+}
+
+void RenderWidgetHostViewMac::GetScreenInfo(ScreenInfo* screen_info) const {
+  browser_compositor_->GetRendererScreenInfo(screen_info);
 }
 
 void RenderWidgetHostViewMac::Show() {
@@ -1086,13 +1104,9 @@ void RenderWidgetHostViewMac::SetTooltipText(
   }
 }
 
-void RenderWidgetHostViewMac::OnSynchronizedDisplayPropertiesChanged() {
-  browser_compositor_->OnNSViewWasResized();
-}
-
 void RenderWidgetHostViewMac::ResizeDueToAutoResize(const gfx::Size& new_size,
                                                     uint64_t sequence_number) {
-  browser_compositor_->OnNSViewWillAutoResize(new_size);
+  browser_compositor_->UpdateForAutoResize(new_size);
   RenderWidgetHostViewBase::ResizeDueToAutoResize(new_size, sequence_number);
 }
 
@@ -1725,7 +1739,7 @@ void RenderWidgetHostViewMac::OnDisplayMetricsChanged(
   display::Screen* screen = display::Screen::GetScreen();
   if (display.id() != screen->GetDisplayNearestView(cocoa_view_).id())
     return;
-  UpdateBackingStoreProperties();
+  UpdateNSViewAndDisplayProperties();
 }
 
 Class GetRenderWidgetHostViewCocoaClassForTesting() {
@@ -2791,7 +2805,7 @@ Class GetRenderWidgetHostViewCocoaClassForTesting() {
 }
 
 - (void)updateScreenProperties{
-  renderWidgetHostView_->UpdateBackingStoreProperties();
+  renderWidgetHostView_->UpdateNSViewAndDisplayProperties();
   renderWidgetHostView_->UpdateDisplayLink();
 }
 
@@ -2810,7 +2824,7 @@ Class GetRenderWidgetHostViewCocoaClassForTesting() {
 }
 
 - (void)windowChangedGlobalFrame:(NSNotification*)notification {
-  renderWidgetHostView_->UpdateBackingStoreProperties();
+  renderWidgetHostView_->UpdateNSViewAndDisplayProperties();
 }
 
 - (void)setFrameSize:(NSSize)newSize {
@@ -2820,24 +2834,7 @@ Class GetRenderWidgetHostViewCocoaClassForTesting() {
   // -setFrame: isn't neccessary.
   [super setFrameSize:newSize];
 
-  if (!renderWidgetHostView_->render_widget_host_)
-    return;
-
-  // During auto-resize it is the responsibility of the caller to ensure that
-  // the NSView and RenderWidgetHostImpl are kept in sync.
-  if (renderWidgetHostView_->render_widget_host_->auto_resize_enabled())
-    return;
-
-  if (renderWidgetHostView_->render_widget_host_->delegate())
-    renderWidgetHostView_->render_widget_host_->delegate()->SendScreenRects();
-  else
-    renderWidgetHostView_->render_widget_host_->SendScreenRects();
-
-  // RenderWidgetHostImpl will query BrowserCompositorMac for the dimensions
-  // to send to the renderer, so it is required that BrowserCompositorMac be
-  // updated first.
-  renderWidgetHostView_->browser_compositor_->OnNSViewWasResized();
-  renderWidgetHostView_->render_widget_host_->WasResized();
+  renderWidgetHostView_->UpdateNSViewAndDisplayProperties();
 
   // Wait for the frame that WasResize might have requested. If the view is
   // being made visible at a new size, then this call will have no effect

ui/display/display.cc

@@ -300,4 +300,17 @@ bool Display::HasInternalDisplay() {
   return internal_display_id_ != kInvalidDisplayId;
 }
 
+bool Display::operator==(const Display& rhs) const {
+  return id_ == rhs.id_ && bounds_ == rhs.bounds_ &&
+         size_in_pixels_ == rhs.size_in_pixels_ &&
+         work_area_ == rhs.work_area_ &&
+         device_scale_factor_ == rhs.device_scale_factor_ &&
+         rotation_ == rhs.rotation_ && touch_support_ == rhs.touch_support_ &&
+         accelerometer_support_ == rhs.accelerometer_support_ &&
+         maximum_cursor_size_ == rhs.maximum_cursor_size_ &&
+         color_space_ == rhs.color_space_ && color_depth_ == rhs.color_depth_ &&
+         depth_per_component_ == rhs.depth_per_component_ &&
+         is_monochrome_ == rhs.is_monochrome_;
+}
+
 }  // namespace display

ui/display/display.h

@@ -226,6 +226,9 @@ class DISPLAY_EXPORT Display final {
     is_monochrome_ = is_monochrome;
   }
 
+  bool operator==(const Display& rhs) const;
+  bool operator!=(const Display& rhs) const { return !(*this == rhs); }
+
  private:
   friend struct mojo::StructTraits<mojom::DisplayDataView, Display>;
 

ui/display/test/display_test_util.h

@@ -9,15 +9,6 @@
 
 namespace display {
 
-inline bool operator==(const Display& lhs, const Display& rhs) {
-  return lhs.id() == rhs.id() &&
-         lhs.bounds() == rhs.bounds() &&
-         lhs.work_area() == rhs.work_area() &&
-         lhs.device_scale_factor() == rhs.device_scale_factor() &&
-         lhs.rotation() == rhs.rotation() &&
-         lhs.touch_support() == rhs.touch_support();
-}
-
 void PrintTo(const Display& display, ::std::ostream* os) {
   *os << display.ToString();
 }