Mac/Linux: Ensure window size constraints propagate to the window server during Init

Currently fixed-size views dialogs on Mac don't initialize properly.
(And, e.g., WebDialogBrowserTest.SizeWindow fails because of it).
Desktop Linux has a similar issue.

On desktop Linux, NativeWidgetPrivate::OnRootViewLayout triggers a call
to UpdateMinAndMaxSize. OnRootViewLayout happens during Widget::Init()
but the X11 window is not yet mapped, so this was resulting in a no-op.
For Linux, fix by explicitly calling UpdateMinAndMaxSize after the
window is mapped.

Mac needs to do a similar thing during Init. However, Mac was calling
OnSizeConstraintsChanged() before Widget::Init() had set the non-client
view, so Widget::GetMinimumSize() returned invalid sizes. For Mac, move
the OnSizeConstraintsChanged() call to OnRootViewLayout(), similar to
Linux.

Then, add a cross-platform test. Mac was not clamping to the size
constraints in SetBounds(), so do that.

For the test itself, collapse the testing WidgetDelegates in
widget_unittest.cc into one helper class rather than adding yet another
WidgetDelegate. Then, provide a way to query the OS for the minimum
window size to fill the gaps required for testing the propagation of
size constraints to the window server.

BUG=454698, 447086

Review URL: https://codereview.chromium.org/1059853007

Cr-Commit-Position: refs/heads/master@{#325538}

ui/views/cocoa/bridged_native_widget.h

@@ -50,6 +50,11 @@ class VIEWS_EXPORT BridgedNativeWidget : public ui::LayerDelegate,
                                // the window above its parent if it has one.
   };
 
+  // Return the size that |window| will take for the given client area |size|,
+  // based on its current style mask.
+  static gfx::Size GetWindowSizeForClientSize(NSWindow* window,
+                                              const gfx::Size& size);
+
   // Creates one side of the bridge. |parent| must not be NULL.
   explicit BridgedNativeWidget(NativeWidgetMac* parent);
   ~BridgedNativeWidget() override;

ui/views/cocoa/bridged_native_widget.mm

@@ -64,10 +64,31 @@ bool PositionWindowInScreenCoordinates(views::Widget* widget,
   return widget && widget->is_top_level();
 }
 
+// Return the content size for a minimum or maximum widget size.
+gfx::Size GetClientSizeForWindowSize(NSWindow* window,
+                                     const gfx::Size& window_size) {
+  NSRect frame_rect =
+      NSMakeRect(0, 0, window_size.width(), window_size.height());
+  // Note gfx::Size will prevent dimensions going negative. They are allowed to
+  // be zero at this point, because Widget::GetMinimumSize() may later increase
+  // the size.
+  return gfx::Size([window contentRectForFrameRect:frame_rect].size);
+}
+
 }  // namespace
 
 namespace views {
 
+// static
+gfx::Size BridgedNativeWidget::GetWindowSizeForClientSize(
+    NSWindow* window,
+    const gfx::Size& content_size) {
+  NSRect content_rect =
+      NSMakeRect(0, 0, content_size.width(), content_size.height());
+  NSRect frame_rect = [window frameRectForContentRect:content_rect];
+  return gfx::Size(NSWidth(frame_rect), NSHeight(frame_rect));
+}
+
 BridgedNativeWidget::BridgedNativeWidget(NativeWidgetMac* parent)
     : native_widget_mac_(parent),
       focus_manager_(nullptr),
@@ -187,22 +208,31 @@ void BridgedNativeWidget::SetFocusManager(FocusManager* focus_manager) {
 }
 
 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(widget->GetMinimumSize());
+
   // A contentRect with zero width or height is a banned practice in ChromeMac,
   // due to unpredictable OSX treatment.
-  DCHECK(!new_bounds.IsEmpty()) << "Zero-sized windows not supported on Mac";
-
-  if (native_widget_mac_->GetWidget()->IsModal()) {
-    // Modal dialogs are positioned by Cocoa. Just update the size.
-    [window_
-        setContentSize:NSMakeSize(new_bounds.width(), new_bounds.height())];
+  DCHECK(!clamped_content_size.IsEmpty())
+      << "Zero-sized windows not supported on Mac";
+
+  if (!window_visible_ && widget->IsModal()) {
+    // Window-Modal dialogs (i.e. sheets) are positioned by Cocoa when shown for
+    // the first time. They also have no frame, so just update the content size.
+    [window_ setContentSize:NSMakeSize(clamped_content_size.width(),
+                                       clamped_content_size.height())];
     return;
   }
+  gfx::Rect actual_new_bounds(
+      new_bounds.origin(),
+      GetWindowSizeForClientSize(window_, clamped_content_size));
 
-  gfx::Rect actual_new_bounds(new_bounds);
-
-  if (parent_ &&
-      !PositionWindowInScreenCoordinates(native_widget_mac_->GetWidget(),
-                                         widget_type_))
+  if (parent_ && !PositionWindowInScreenCoordinates(widget, widget_type_))
     actual_new_bounds.Offset(parent_->GetRestoredBounds().OffsetFromOrigin());
 
   [window_ setFrame:gfx::ScreenRectToNSRect(actual_new_bounds)

ui/views/test/widget_test.h

@@ -115,6 +115,10 @@ class WidgetTest : public ViewsTestBase {
   // Both windows must be visible.
   static bool IsWindowStackedAbove(Widget* above, Widget* below);
 
+  // Query the native window system for the minimum size configured for user
+  // initiated window resizes.
+  static gfx::Size GetNativeWidgetMinimumContentSize(Widget* widget);
+
   // Return the event processor for |widget|. On aura platforms, this is an
   // aura::WindowEventDispatcher. Otherwise, it is a bridge to the OS event
   // processor.

ui/views/test/widget_test_aura.cc

@@ -8,6 +8,11 @@
 #include "ui/aura/window_tree_host.h"
 #include "ui/views/widget/widget.h"
 
+#if defined(USE_X11)
+#include <X11/Xutil.h>
+#include "ui/gfx/x/x11_types.h"
+#endif
+
 namespace views {
 namespace test {
 
@@ -65,6 +70,27 @@ bool WidgetTest::IsWindowStackedAbove(Widget* above, Widget* below) {
   return FindLayersInOrder(root_layer->children(), &first, &second);
 }
 
+// static
+gfx::Size WidgetTest::GetNativeWidgetMinimumContentSize(Widget* widget) {
+  // On Windows, HWNDMessageHandler receives a WM_GETMINMAXINFO message whenever
+  // the window manager is interested in knowing the size constraints. On
+  // ChromeOS, it's handled internally. Elsewhere, the size constraints need to
+  // be pushed to the window server when they change.
+#if defined(OS_CHROMEOS) || defined(OS_WIN)
+  return widget->GetNativeWindow()->delegate()->GetMinimumSize();
+#elif defined(USE_X11)
+  XSizeHints hints;
+  long supplied_return;
+  XGetWMNormalHints(
+      gfx::GetXDisplay(),
+      widget->GetNativeWindow()->GetHost()->GetAcceleratedWidget(), &hints,
+      &supplied_return);
+  return gfx::Size(hints.min_width, hints.min_height);
+#else
+  NOTREACHED();
+#endif
+}
+
 // static
 ui::EventProcessor* WidgetTest::GetEventProcessor(Widget* widget) {
   return widget->GetNativeWindow()->GetHost()->event_processor();

ui/views/test/widget_test_mac.mm

@@ -72,6 +72,11 @@ bool WidgetTest::IsWindowStackedAbove(Widget* above, Widget* below) {
   return false;
 }
 
+// static
+gfx::Size WidgetTest::GetNativeWidgetMinimumContentSize(Widget* widget) {
+  return gfx::Size([widget->GetNativeWindow() contentMinSize]);
+}
+
 // static
 ui::EventProcessor* WidgetTest::GetEventProcessor(Widget* widget) {
   return static_cast<internal::RootView*>(widget->GetRootView());

ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc

@@ -1612,6 +1612,8 @@ void DesktopWindowTreeHostX11::MapWindow(ui::WindowShowState show_state) {
     ui::X11EventSource::GetInstance()->BlockUntilWindowMapped(xwindow_);
   window_mapped_ = true;
 
+  UpdateMinAndMaxSize();
+
   // Some WMs only respect maximize hints after the window has been mapped.
   // Check whether we need to re-do a maximization.
   if (should_maximize_after_map_) {

ui/views/widget/native_widget_mac.mm

@@ -34,12 +34,6 @@ NSInteger StyleMaskForParams(const Widget::InitParams& params) {
   return NSBorderlessWindowMask;
 }
 
-gfx::Size WindowSizeForClientAreaSize(NSWindow* window, const gfx::Size& size) {
-  NSRect content_rect = NSMakeRect(0, 0, size.width(), size.height());
-  NSRect frame_rect = [window frameRectForContentRect:content_rect];
-  return gfx::Size(NSWidth(frame_rect), NSHeight(frame_rect));
-}
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -91,8 +85,6 @@ void NativeWidgetMac::InitNativeWidget(const Widget::InitParams& params) {
 
   delegate_->OnNativeWidgetCreated(true);
 
-  OnSizeConstraintsChanged();
-
   bridge_->SetFocusManager(GetWidget()->GetFocusManager());
 
   DCHECK(GetWidget()->GetRootView());
@@ -206,7 +198,8 @@ ui::InputMethod* NativeWidgetMac::GetHostInputMethod() {
 }
 
 void NativeWidgetMac::CenterWindow(const gfx::Size& size) {
-  SetSize(WindowSizeForClientAreaSize(GetNativeWindow(), size));
+  SetSize(
+      BridgedNativeWidget::GetWindowSizeForClientSize(GetNativeWindow(), size));
   // Note that this is not the precise center of screen, but it is the standard
   // location for windows like dialogs to appear on screen for Mac.
   // TODO(tapted): If there is a parent window, center in that instead.
@@ -515,7 +508,9 @@ ui::NativeTheme* NativeWidgetMac::GetNativeTheme() const {
 }
 
 void NativeWidgetMac::OnRootViewLayout() {
-  NOTIMPLEMENTED();
+  // Ensure possible changes to the non-client view (e.g. Minimum/Maximum size)
+  // propagate through to the NSWindow properties.
+  OnSizeConstraintsChanged();
 }
 
 bool NativeWidgetMac::IsTranslucentWindowOpacitySupported() const {

ui/views/widget/widget_unittest.cc

@@ -209,6 +209,67 @@ class EventCountHandler : public ui::EventHandler {
   DISALLOW_COPY_AND_ASSIGN(EventCountHandler);
 };
 
+// A helper WidgetDelegate for tests that require hooks into WidgetDelegate
+// calls, and removes some of the boilerplate for initializing a Widget. Calls
+// Widget::CloseNow() when destroyed if it hasn't already been done.
+class TestDesktopWidgetDelegate : public WidgetDelegate {
+ public:
+  TestDesktopWidgetDelegate() : widget_(new Widget) {}
+
+  ~TestDesktopWidgetDelegate() override {
+    if (widget_)
+      widget_->CloseNow();
+    EXPECT_FALSE(widget_);
+  }
+
+  // Initialize the Widget, adding some meaningful default InitParams.
+  void InitWidget(Widget::InitParams init_params) {
+    init_params.delegate = this;
+#if !defined(OS_CHROMEOS)
+    init_params.native_widget = new PlatformDesktopNativeWidget(widget_);
+#endif
+    init_params.bounds = initial_bounds_;
+    widget_->Init(init_params);
+  }
+
+  // Set the contents view to be used during Widget initialization. For Widgets
+  // that use non-client views, this will be the contents_view used to
+  // initialize the ClientView in WidgetDelegate::CreateClientView(). Otherwise,
+  // it is the ContentsView of the Widget's RootView. Ownership passes to the
+  // view hierarchy during InitWidget().
+  void set_contents_view(View* contents_view) {
+    contents_view_ = contents_view;
+  }
+
+  int window_closing_count() const { return window_closing_count_; }
+  const gfx::Rect& initial_bounds() { return initial_bounds_; }
+
+  // WidgetDelegate overrides:
+  void WindowClosing() override {
+    window_closing_count_++;
+    widget_ = nullptr;
+  }
+
+  Widget* GetWidget() override { return widget_; }
+  const Widget* GetWidget() const override { return widget_; }
+
+  View* GetContentsView() override {
+    return contents_view_ ? contents_view_ : WidgetDelegate::GetContentsView();
+  }
+
+  bool ShouldAdvanceFocusToTopLevelWidget() const override {
+    return true;  // Same default as DefaultWidgetDelegate in widget.cc.
+  }
+
+ private:
+  Widget* widget_;
+  View* contents_view_ = nullptr;
+  int window_closing_count_ = 0;
+  gfx::Rect initial_bounds_ = gfx::Rect(100, 100, 200, 200);
+
+  DISALLOW_COPY_AND_ASSIGN(TestDesktopWidgetDelegate);
+};
+
 TEST_F(WidgetTest, WidgetInitParams) {
   // Widgets are not transparent by default.
   Widget::InitParams init1;
@@ -1117,6 +1178,54 @@ TEST_F(WidgetTest, GetWindowPlacement) {
   widget->CloseNow();
 }
 
+// Test that widget size constraints are properly applied immediately after
+// Init(), and that SetBounds() calls are appropriately clamped.
+TEST_F(WidgetTest, MinimumSizeConstraints) {
+  TestDesktopWidgetDelegate delegate;
+  gfx::Size minimum_size(100, 100);
+  const gfx::Size smaller_size(90, 90);
+
+  delegate.set_contents_view(new StaticSizedView(minimum_size));
+  delegate.InitWidget(CreateParams(Widget::InitParams::TYPE_WINDOW));
+  Widget* widget = delegate.GetWidget();
+
+  // On desktop Linux, the Widget must be shown to ensure the window is mapped.
+  // On other platforms this line is optional.
+  widget->Show();
+
+  // Sanity checks.
+  EXPECT_GT(delegate.initial_bounds().width(), minimum_size.width());
+  EXPECT_GT(delegate.initial_bounds().height(), minimum_size.height());
+  EXPECT_EQ(delegate.initial_bounds().size(),
+            widget->GetWindowBoundsInScreen().size());
+  // Note: StaticSizedView doesn't currently provide a maximum size.
+  EXPECT_EQ(gfx::Size(), widget->GetMaximumSize());
+
+  if (!widget->ShouldUseNativeFrame()) {
+    // The test environment may have dwm disabled on Windows. In this case,
+    // CustomFrameView is used instead of the NativeFrameView, which will
+    // provide a minimum size that includes frame decorations.
+    minimum_size = widget->non_client_view()->GetWindowBoundsForClientBounds(
+        gfx::Rect(minimum_size)).size();
+  }
+
+  EXPECT_EQ(minimum_size, widget->GetMinimumSize());
+  EXPECT_EQ(minimum_size, GetNativeWidgetMinimumContentSize(widget));
+
+  // Trying to resize smaller than the minimum size should restrict the content
+  // size to the minimum size.
+  widget->SetBounds(gfx::Rect(smaller_size));
+  EXPECT_EQ(minimum_size, widget->GetClientAreaBoundsInScreen().size());
+
+  widget->SetSize(smaller_size);
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+  // TODO(tapted): Desktop Linux ignores size constraints for SetSize. Fix it.
+  EXPECT_EQ(smaller_size, widget->GetClientAreaBoundsInScreen().size());
+#else
+  EXPECT_EQ(minimum_size, widget->GetClientAreaBoundsInScreen().size());
+#endif
+}
+
 // Tests that SetBounds() and GetWindowBoundsInScreen() is symmetric when the
 // widget is visible and not maximized or fullscreen.
 TEST_F(WidgetTest, GetWindowBoundsInScreen) {
@@ -1768,44 +1877,14 @@ TEST_F(WidgetTest, MouseEventDispatchWhileTouchIsDown) {
 
 #endif  // !defined(OS_MACOSX) || defined(USE_AURA)
 
-// Used by SingleWindowClosing to count number of times WindowClosing() has
-// been invoked.
-class ClosingDelegate : public WidgetDelegate {
- public:
-  ClosingDelegate() : count_(0), widget_(NULL) {}
-
-  int count() const { return count_; }
-
-  void set_widget(views::Widget* widget) { widget_ = widget; }
-
-  // WidgetDelegate overrides:
-  Widget* GetWidget() override { return widget_; }
-  const Widget* GetWidget() const override { return widget_; }
-  void WindowClosing() override { count_++; }
-
- private:
-  int count_;
-  views::Widget* widget_;
-
-  DISALLOW_COPY_AND_ASSIGN(ClosingDelegate);
-};
-
 // Verifies WindowClosing() is invoked correctly on the delegate when a Widget
 // is closed.
 TEST_F(WidgetTest, SingleWindowClosing) {
-  scoped_ptr<ClosingDelegate> delegate(new ClosingDelegate());
-  Widget* widget = new Widget();  // Destroyed by CloseNow() below.
-  Widget::InitParams init_params =
-      CreateParams(Widget::InitParams::TYPE_WINDOW);
-  init_params.bounds = gfx::Rect(0, 0, 200, 200);
-  init_params.delegate = delegate.get();
-#if !defined(OS_CHROMEOS)
-  init_params.native_widget = new PlatformDesktopNativeWidget(widget);
-#endif
-  widget->Init(init_params);
-  EXPECT_EQ(0, delegate->count());
-  widget->CloseNow();
-  EXPECT_EQ(1, delegate->count());
+  TestDesktopWidgetDelegate delegate;
+  delegate.InitWidget(CreateParams(Widget::InitParams::TYPE_WINDOW));
+  EXPECT_EQ(0, delegate.window_closing_count());
+  delegate.GetWidget()->CloseNow();
+  EXPECT_EQ(1, delegate.window_closing_count());
 }
 
 class WidgetWindowTitleTest : public WidgetTest {
@@ -3093,70 +3172,17 @@ TEST_F(WidgetTest, FullscreenStatePropagated) {
 }
 #if defined(OS_WIN)
 
-// Provides functionality to test widget activation via an activation flag
-// which can be set by an accessor.
-class ModalWindowTestWidgetDelegate : public WidgetDelegate {
- public:
-  ModalWindowTestWidgetDelegate()
-      : widget_(NULL),
-        can_activate_(true) {}
-
-  virtual ~ModalWindowTestWidgetDelegate() {}
-
-  // Overridden from WidgetDelegate:
-  virtual void DeleteDelegate() override {
-    delete this;
-  }
-  virtual Widget* GetWidget() override {
-    return widget_;
-  }
-  virtual const Widget* GetWidget() const override {
-    return widget_;
-  }
-  virtual bool CanActivate() const override {
-    return can_activate_;
-  }
-  virtual bool ShouldAdvanceFocusToTopLevelWidget() const override {
-    return true;
-  }
-
-  void set_can_activate(bool can_activate) {
-    can_activate_ = can_activate;
-  }
-
-  void set_widget(Widget* widget) {
-    widget_ = widget;
-  }
-
- private:
-  Widget* widget_;
-  bool can_activate_;
-
-  DISALLOW_COPY_AND_ASSIGN(ModalWindowTestWidgetDelegate);
-};
-
 // Tests whether we can activate the top level widget when a modal dialog is
 // active.
 TEST_F(WidgetTest, WindowModalityActivationTest) {
-  // Destroyed when the top level widget created below is destroyed.
-  ModalWindowTestWidgetDelegate* widget_delegate =
-      new ModalWindowTestWidgetDelegate;
-  // Create a top level widget.
-  Widget top_level_widget;
-  Widget::InitParams init_params =
-      CreateParams(Widget::InitParams::TYPE_WINDOW);
-  init_params.show_state = ui::SHOW_STATE_NORMAL;
-  gfx::Rect initial_bounds(0, 0, 500, 500);
-  init_params.bounds = initial_bounds;
-  init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
-  init_params.native_widget = new DesktopNativeWidgetAura(&top_level_widget);
-  init_params.delegate = widget_delegate;
-  top_level_widget.Init(init_params);
-  widget_delegate->set_widget(&top_level_widget);
-  top_level_widget.Show();
-  EXPECT_TRUE(top_level_widget.IsVisible());
+  TestDesktopWidgetDelegate widget_delegate;
+  widget_delegate.InitWidget(CreateParams(Widget::InitParams::TYPE_WINDOW));
 
-  HWND win32_window = views::HWNDForWidget(&top_level_widget);
+  Widget* top_level_widget = widget_delegate.GetWidget();
+  top_level_widget->Show();
+  EXPECT_TRUE(top_level_widget->IsVisible());
+
+  HWND win32_window = views::HWNDForWidget(top_level_widget);
   EXPECT_TRUE(::IsWindow(win32_window));
 
   // This instance will be destroyed when the dialog is destroyed.
@@ -3164,10 +3190,10 @@ TEST_F(WidgetTest, WindowModalityActivationTest) {
 
   // We should be able to activate the window even if the WidgetDelegate
   // says no, when a modal dialog is active.
-  widget_delegate->set_can_activate(false);
+  widget_delegate.set_can_activate(false);
 
   Widget* modal_dialog_widget = views::DialogDelegate::CreateDialogWidget(
-      dialog_delegate, NULL, top_level_widget.GetNativeWindow());
+      dialog_delegate, NULL, top_level_widget->GetNativeView());
   modal_dialog_widget->SetBounds(gfx::Rect(100, 100, 200, 200));
   modal_dialog_widget->Show();
   EXPECT_TRUE(modal_dialog_widget->IsVisible());
@@ -3180,7 +3206,6 @@ TEST_F(WidgetTest, WindowModalityActivationTest) {
   EXPECT_EQ(activate_result, MA_ACTIVATE);
 
   modal_dialog_widget->CloseNow();
-  top_level_widget.CloseNow();
 }
 #endif  // defined(OS_WIN)
 #endif  // !defined(OS_CHROMEOS)