RemoteMacViews: Remove browser dependencies of SetBounds

The function BridgedNativeWidget::SetBounds calls back into the
NativeWidgetMac to determine
- the minimum content size
- if the widget's position should be relative to the parent or the
  screen (and the offset to do that computation)
- if the widget is a modal sheet
This call will eventually be running in the app shim process, where
these values will not be available (without adding a synchronous IPC).

Change NativeWidgetMac::SetBounds to call BridgedNativeWidgetHostImpl::
SetBounds, and have that function pre-compute the required parameters,
- minimum content size
- offset necessary to be applied to compensate for position being
  relative to the parent or the screen
- (but not if the widget is a modal sheet, that's coming later)
and pass these as parameters to BridgedNativeWidget::SetBounds.

Split out the SetBounds calls made during initialization from
being within BridgedNativeWidget::Init to being made by its caller,
BridgedNativeWidgetHostImpl::InitWindow, to allow the additional
parameters to be passed in.

This is a re-land of crrev.com/584327 (reverted in crrev.com/584339)
with the logic for initial bounds setting separated out into the
BridgedNativeWidget::SetInitialBounds (merging the behaviors was too
ambitious).

TBR=ellyjones (original reviewer)

Bug: 859152
Change-Id: I98b6a5d419e586fb2697ef3b45fdc19c73f9c619
Reviewed-on: https://chromium-review.googlesource.com/1180715Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584363}

ui/views/cocoa/bridged_native_widget.h

@@ -51,6 +51,22 @@ class VIEWS_EXPORT BridgedNativeWidgetPublic {
   // BridgedNativeWidgetHost.
   virtual void InitCompositorView() = 0;
 
+  // Specify initial bounds for the window via |new_bounds| in screen
+  // coordinates. It is invalid for |new_bounds| to have an empty size and
+  // non-zero position. The size of the window will be expanded so that the
+  // content size will be at least |minimum_content_size|. The bounds are offset
+  // by |parent_offset| (this isn't incorporated directly into |new_bounds| for
+  // the aforementioned checks of |new_bounds|' position).
+  virtual void SetInitialBounds(const gfx::Rect& new_bounds,
+                                const gfx::Size& minimum_content_size,
+                                const gfx::Vector2d& parent_offset) = 0;
+
+  // Specify new bounds for the window via |new_bounds| in screen coordinates.
+  // The size of the window will be expanded so that the content size will be
+  // at least |minimum_content_size|.
+  virtual void SetBounds(const gfx::Rect& new_bounds,
+                         const gfx::Size& minimum_content_size) = 0;
+
   // Specify the content to draw in the NSView.
   virtual void SetCALayerParams(const gfx::CALayerParams& ca_layer_params) = 0;
 
@@ -238,6 +254,11 @@ class VIEWS_EXPORT BridgedNativeWidget
 
   // views::BridgedNativeWidgetPublic:
   void InitCompositorView() override;
+  void SetInitialBounds(const gfx::Rect& new_bounds,
+                        const gfx::Size& minimum_content_size,
+                        const gfx::Vector2d& parent_offset) override;
+  void SetBounds(const gfx::Rect& new_bounds,
+                 const gfx::Size& minimum_content_size) override;
   void SetCALayerParams(const gfx::CALayerParams& ca_layer_params) override;
   void ClearTouchBar() override;
 

ui/views/cocoa/bridged_native_widget.mm

@@ -38,7 +38,6 @@
 #include "ui/views/view.h"
 #include "ui/views/widget/native_widget_mac.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/widget/widget_aura_utils.h"
 #include "ui/views/widget/widget_delegate.h"
 
 namespace {
@@ -120,18 +119,6 @@ using NSViewComparatorValue = __kindof NSView*;
 
 int kWindowPropertiesKey;
 
-// Returns true if bounds passed to window in SetBounds should be treated as
-// though they are in screen coordinates.
-bool PositionWindowInScreenCoordinates(views::Widget* widget,
-                                       views::Widget::InitParams::Type type) {
-  // Replicate the logic in desktop_aura/desktop_screen_position_client.cc.
-  if (views::GetAuraWindowTypeForWidgetType(type) ==
-      aura::client::WINDOW_TYPE_POPUP)
-    return true;
-
-  return widget && widget->is_top_level();
-}
-
 // Returns true if the content_view is reparented.
 bool PositionWindowInNativeViewParent(NSView* content_view) {
   return [[content_view window] contentView] != content_view;
@@ -326,18 +313,21 @@ void BridgedNativeWidget::Init(const Widget::InitParams& params) {
       break;
   }  // No default case, to pick up new types.
 
-  // Set a meaningful initial bounds. Note that except for frameless widgets
-  // with no WidgetDelegate, the bounds will be set again by Widget after
-  // initializing the non-client view. In the former case, if bounds were not
-  // set at all, the creator of the Widget is expected to call SetBounds()
-  // before calling Widget::Show() to avoid a kWindowSizeDeterminedLater-sized
-  // (i.e. 1x1) window appearing.
-  if (!params.bounds.IsEmpty()) {
-    SetBounds(params.bounds);
-  } else {
+  // Tooltip Widgets shouldn't have their own tooltip manager, but tooltips are
+  // native on Mac, so nothing should ever want one in Widget form.
+  DCHECK_NE(params.type, Widget::InitParams::TYPE_TOOLTIP);
+  tooltip_manager_.reset(new TooltipManagerMac(this));
+}
+
+void BridgedNativeWidget::SetInitialBounds(
+    const gfx::Rect& new_bounds,
+    const gfx::Size& minimum_content_size,
+    const gfx::Vector2d& bounds_offset_for_parent) {
+  gfx::Rect adjusted_bounds = new_bounds;
+  if (new_bounds.IsEmpty()) {
     // If a position is set, but no size, complain. Otherwise, a 1x1 window
     // would appear there, which might be unexpected.
-    DCHECK(params.bounds.origin().IsOrigin())
+    DCHECK(new_bounds.origin().IsOrigin())
         << "Zero-sized windows not supported on Mac.";
 
     // Otherwise, bounds is all zeroes. Cocoa will currently have the window at
@@ -346,28 +336,21 @@ void BridgedNativeWidget::Init(const Widget::InitParams& params) {
     // Read back the current frame: it will be a 1x1 context rect but the frame
     // size also depends on the window style.
     NSRect frame_rect = [window_ frame];
-    SetBounds(gfx::Rect(gfx::Point(),
-                        gfx::Size(NSWidth(frame_rect), NSHeight(frame_rect))));
+    adjusted_bounds = gfx::Rect(
+        gfx::Point(), gfx::Size(NSWidth(frame_rect), NSHeight(frame_rect)));
   }
-
-  // Widgets for UI controls (usually layered above web contents) start visible.
-  if (params.type == Widget::InitParams::TYPE_CONTROL)
-    SetVisibilityState(SHOW_INACTIVE);
-
-  // Tooltip Widgets shouldn't have their own tooltip manager, but tooltips are
-  // native on Mac, so nothing should ever want one in Widget form.
-  DCHECK_NE(params.type, Widget::InitParams::TYPE_TOOLTIP);
-  tooltip_manager_.reset(new TooltipManagerMac(this));
+  adjusted_bounds.Offset(bounds_offset_for_parent);
+  SetBounds(adjusted_bounds, minimum_content_size);
 }
 
-void BridgedNativeWidget::SetBounds(const gfx::Rect& new_bounds) {
-  Widget* widget = native_widget_mac_->GetWidget();
+void BridgedNativeWidget::SetBounds(const gfx::Rect& new_bounds,
+                                    const gfx::Size& minimum_content_size) {
   // -[NSWindow contentMinSize] is only checked by Cocoa for user-initiated
   // resizes. This is not what toolkit-views expects, so clamp. Note there is
   // no check for maximum size (consistent with aura::Window::SetBounds()).
   gfx::Size clamped_content_size =
       GetClientSizeForWindowSize(window_, new_bounds.size());
-  clamped_content_size.SetToMax(widget->GetMinimumSize());
+  clamped_content_size.SetToMax(minimum_content_size);
 
   // A contentRect with zero width or height is a banned practice in ChromeMac,
   // due to unpredictable OSX treatment.
@@ -385,9 +368,6 @@ void BridgedNativeWidget::SetBounds(const gfx::Rect& new_bounds) {
       new_bounds.origin(),
       GetWindowSizeForClientSize(window_, clamped_content_size));
 
-  if (parent_ && !PositionWindowInScreenCoordinates(widget, widget_type_))
-    actual_new_bounds.Offset(parent_->GetChildWindowOffset());
-
   if (PositionWindowInNativeViewParent(bridged_view_))
     actual_new_bounds.Offset(GetNativeViewParentOffset(bridged_view_));
 

ui/views/cocoa/bridged_native_widget_host_impl.h

@@ -49,6 +49,16 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   BridgedNativeWidget* bridge_impl() const { return bridge_impl_.get(); }
   BridgedNativeWidgetPublic* bridge() const;
 
+  void InitWindow(const Widget::InitParams& params);
+
+  // Changes the bounds of the window and the hosted layer if present. The
+  // origin is a location in screen coordinates except for "child" windows,
+  // which are positioned relative to their parent. SetBounds() considers a
+  // "child" window to be one initialized with InitParams specifying all of:
+  // a |parent| NSWindow, the |child| attribute, and a |type| that
+  // views::GetAuraWindowTypeForWidgetType does not consider a "popup" type.
+  void SetBounds(const gfx::Rect& bounds);
+
   // Set the root view (set during initialization and un-set during teardown).
   void SetRootView(views::View* root_view);
 
@@ -82,6 +92,7 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   const display::Display& GetCurrentDisplay() const { return display_; }
 
  private:
+  gfx::Vector2d GetBoundsOffsetForParent() const;
   void DestroyCompositor();
 
   // views::BridgedNativeWidgetHost:
@@ -127,6 +138,8 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
 
   views::NativeWidgetMac* const native_widget_mac_;  // Weak. Owns |this_|.
 
+  Widget::InitParams::Type widget_type_ = Widget::InitParams::TYPE_WINDOW;
+
   views::View* root_view_ = nullptr;  // Weak. Owned by |native_widget_mac_|.
 
   // TODO(ccameron): Rather than instantiate a BridgedNativeWidget here,

ui/views/cocoa/bridged_native_widget_host_impl.mm

@@ -13,12 +13,28 @@
 #include "ui/views/cocoa/bridged_native_widget.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/native_widget_mac.h"
+#include "ui/views/widget/widget_aura_utils.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/window/dialog_delegate.h"
 #include "ui/views/word_lookup_client.h"
 
 namespace views {
 
+namespace {
+
+// Returns true if bounds passed to window in SetBounds should be treated as
+// though they are in screen coordinates.
+bool PositionWindowInScreenCoordinates(Widget* widget,
+                                       Widget::InitParams::Type type) {
+  // Replicate the logic in desktop_aura/desktop_screen_position_client.cc.
+  if (GetAuraWindowTypeForWidgetType(type) == aura::client::WINDOW_TYPE_POPUP)
+    return true;
+
+  return widget && widget->is_top_level();
+}
+
+}  // namespace
+
 BridgedNativeWidgetHostImpl::BridgedNativeWidgetHostImpl(
     NativeWidgetMac* parent)
     : native_widget_mac_(parent),
@@ -38,6 +54,41 @@ BridgedNativeWidgetPublic* BridgedNativeWidgetHostImpl::bridge() const {
   return bridge_impl_.get();
 }
 
+void BridgedNativeWidgetHostImpl::InitWindow(const Widget::InitParams& params) {
+  widget_type_ = params.type;
+  bridge_impl_->Init(params);
+
+  // Set a meaningful initial bounds. Note that except for frameless widgets
+  // with no WidgetDelegate, the bounds will be set again by Widget after
+  // initializing the non-client view. In the former case, if bounds were not
+  // set at all, the creator of the Widget is expected to call SetBounds()
+  // before calling Widget::Show() to avoid a kWindowSizeDeterminedLater-sized
+  // (i.e. 1x1) window appearing.
+  bridge()->SetInitialBounds(params.bounds,
+                             native_widget_mac_->GetWidget()->GetMinimumSize(),
+                             GetBoundsOffsetForParent());
+
+  // Widgets for UI controls (usually layered above web contents) start visible.
+  if (widget_type_ == Widget::InitParams::TYPE_CONTROL)
+    bridge_impl_->SetVisibilityState(BridgedNativeWidget::SHOW_INACTIVE);
+}
+
+void BridgedNativeWidgetHostImpl::SetBounds(const gfx::Rect& bounds) {
+  gfx::Rect adjusted_bounds = bounds;
+  adjusted_bounds.Offset(GetBoundsOffsetForParent());
+  bridge()->SetBounds(adjusted_bounds,
+                      native_widget_mac_->GetWidget()->GetMinimumSize());
+}
+
+gfx::Vector2d BridgedNativeWidgetHostImpl::GetBoundsOffsetForParent() const {
+  gfx::Vector2d offset;
+  Widget* widget = native_widget_mac_->GetWidget();
+  BridgedNativeWidgetOwner* parent = bridge_impl_->parent();
+  if (parent && !PositionWindowInScreenCoordinates(widget, widget_type_))
+    offset = parent->GetChildWindowOffset();
+  return offset;
+}
+
 void BridgedNativeWidgetHostImpl::SetRootView(views::View* root_view) {
   root_view_ = root_view;
   // TODO(ccameron): The BridgedNativeWidget should not need to know its root

ui/views/cocoa/bridged_native_widget_unittest.mm

@@ -309,7 +309,7 @@ class MockNativeWidgetMac : public NativeWidgetMac {
                         backing:NSBackingStoreBuffered
                           defer:NO]);
     bridge()->SetWindow(window);
-    bridge()->Init(params);
+    bridge_host_for_testing()->InitWindow(params);
 
     // Usually the bridge gets initialized here. It is skipped to run extra
     // checks in tests, and so that a second window isn't created.

ui/views/widget/native_widget_mac.mm

@@ -132,7 +132,7 @@ void NativeWidgetMac::InitNativeWidget(const Widget::InitParams& params) {
   base::scoped_nsobject<NativeWidgetMacNSWindow> window(
       [CreateNSWindow(params) retain]);
   bridge()->SetWindow(window);
-  bridge()->Init(params);
+  bridge_host_->InitWindow(params);
 
   // Only set always-on-top here if it is true since setting it may affect how
   // the window is treated by Expose.
@@ -328,8 +328,8 @@ std::string NativeWidgetMac::GetWorkspace() const {
 }
 
 void NativeWidgetMac::SetBounds(const gfx::Rect& bounds) {
-  if (bridge())
-    bridge()->SetBounds(bounds);
+  if (bridge_host_)
+    bridge_host_->SetBounds(bounds);
 }
 
 void NativeWidgetMac::SetBoundsConstrained(const gfx::Rect& bounds) {