Revert "RemoteMacViews: Remove browser dependencies of SetBounds"

This reverts commit 5f99a2c0.

Reason for revert: WebDialogBrowserTest.SizeWindow fails on max 10.11, 10.12 (10.10 and 10.13 seem happy):

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/28636
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/14984

Original change's description:
> 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.
> 
> Bug: 859152
> Change-Id: I2f33ab08f60d2ed1bc3b6af5ecf3402e3715ffd4
> Reviewed-on: https://chromium-review.googlesource.com/1178813
> Commit-Queue: ccameron <ccameron@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584327}

TBR=ellyjones@chromium.org,ccameron@chromium.org

Change-Id: I1300db812416881fb539b586e7d7fab2597d60c5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 859152
Reviewed-on: https://chromium-review.googlesource.com/1180761Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584339}

ui/views/cocoa/bridged_native_widget.h

@@ -51,15 +51,6 @@ class VIEWS_EXPORT BridgedNativeWidgetPublic {
   // BridgedNativeWidgetHost.
   virtual void InitCompositorView() = 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|. The bounds are offset by |parent_offset|
-  // (this isn't incorporated directly into |new_bounds| for initialization edge
-  // cases).
-  virtual void SetBounds(const gfx::Rect& new_bounds,
-                         const gfx::Size& minimum_content_size,
-                         const gfx::Vector2d& parent_offset) = 0;
-
   // Specify the content to draw in the NSView.
   virtual void SetCALayerParams(const gfx::CALayerParams& ca_layer_params) = 0;
 
@@ -248,9 +239,6 @@ class VIEWS_EXPORT BridgedNativeWidget
 
   // views::BridgedNativeWidgetPublic:
   void InitCompositorView() override;
-  void SetBounds(const gfx::Rect& new_bounds,
-                 const gfx::Size& minimum_content_size,
-                 const gfx::Vector2d& parent_offset) override;
   void SetCALayerParams(const gfx::CALayerParams& ca_layer_params) override;
   void ClearTouchBar() override;
 

ui/views/cocoa/bridged_native_widget.mm

@@ -38,6 +38,7 @@
 #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 {
@@ -119,6 +120,18 @@ 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;
@@ -315,20 +328,18 @@ void BridgedNativeWidget::Init(const Widget::InitParams& params) {
       break;
   }  // No default case, to pick up new types.
 
-  // 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::SetBounds(
-    const gfx::Rect& new_bounds,
-    const gfx::Size& minimum_content_size,
-    const gfx::Vector2d& bounds_offset_for_parent) {
-  if (new_bounds.IsEmpty()) {
+  // 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 {
     // If a position is set, but no size, complain. Otherwise, a 1x1 window
     // would appear there, which might be unexpected.
-    DCHECK(new_bounds.origin().IsOrigin())
+    DCHECK(params.bounds.origin().IsOrigin())
         << "Zero-sized windows not supported on Mac.";
 
     // Otherwise, bounds is all zeroes. Cocoa will currently have the window at
@@ -338,17 +349,27 @@ void BridgedNativeWidget::SetBounds(
     // 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))),
-              minimum_content_size, bounds_offset_for_parent);
-    return;
+                        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));
+}
+
+void BridgedNativeWidget::SetBounds(const gfx::Rect& new_bounds) {
+  Widget* widget = native_widget_mac_->GetWidget();
   // -[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(minimum_content_size);
+  clamped_content_size.SetToMax(widget->GetMinimumSize());
 
   // A contentRect with zero width or height is a banned practice in ChromeMac,
   // due to unpredictable OSX treatment.
@@ -366,7 +387,8 @@ void BridgedNativeWidget::SetBounds(
       new_bounds.origin(),
       GetWindowSizeForClientSize(window_, clamped_content_size));
 
-  actual_new_bounds.Offset(bounds_offset_for_parent);
+  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,16 +49,6 @@ 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);
 
@@ -92,7 +82,6 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   const display::Display& GetCurrentDisplay() const { return display_; }
 
  private:
-  gfx::Vector2d GetBoundsOffsetForParent() const;
   void DestroyCompositor();
 
   // views::BridgedNativeWidgetHost:
@@ -138,8 +127,6 @@ 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,28 +13,12 @@
 #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),
@@ -54,37 +38,6 @@ 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.
-  SetBounds(params.bounds);
-
-  // 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) {
-  bridge()->SetBounds(bounds, native_widget_mac_->GetWidget()->GetMinimumSize(),
-                      GetBoundsOffsetForParent());
-}
-
-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_host_for_testing()->InitWindow(params);
+    bridge()->Init(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);
   bridge()->SetWindow(window);
-  bridge_host_->InitWindow(params);
+  bridge()->Init(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_host_)
-    bridge_host_->SetBounds(bounds);
+  if (bridge())
+    bridge()->SetBounds(bounds);
 }
 
 void NativeWidgetMac::SetBoundsConstrained(const gfx::Rect& bounds) {