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}
diff --git a/ui/views/cocoa/bridged_native_widget.h b/ui/views/cocoa/bridged_native_widget.h
index e3bec4ae..149c537 100644
--- a/ui/views/cocoa/bridged_native_widget.h
+++ b/ui/views/cocoa/bridged_native_widget.h
@@ -50,6 +50,11 @@
// 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;
diff --git a/ui/views/cocoa/bridged_native_widget.mm b/ui/views/cocoa/bridged_native_widget.mm
index efe1785..2427578 100644
--- a/ui/views/cocoa/bridged_native_widget.mm
+++ b/ui/views/cocoa/bridged_native_widget.mm
@@ -64,10 +64,31 @@
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::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";
+ DCHECK(!clamped_content_size.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())];
+ 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)
diff --git a/ui/views/test/widget_test.h b/ui/views/test/widget_test.h
index e820191..6d1169e 100644
--- a/ui/views/test/widget_test.h
+++ b/ui/views/test/widget_test.h
@@ -115,6 +115,10 @@
// 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.
diff --git a/ui/views/test/widget_test_aura.cc b/ui/views/test/widget_test_aura.cc
index 822ea98e..778f1f0c 100644
--- a/ui/views/test/widget_test_aura.cc
+++ b/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 {
@@ -66,6 +71,27 @@
}
// 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();
}
diff --git a/ui/views/test/widget_test_mac.mm b/ui/views/test/widget_test_mac.mm
index 523e31d..2f1698d 100644
--- a/ui/views/test/widget_test_mac.mm
+++ b/ui/views/test/widget_test_mac.mm
@@ -73,6 +73,11 @@
}
// 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());
}
diff --git a/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc b/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
index 1316b8b..125e8f0c 100644
--- a/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
+++ b/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
@@ -1612,6 +1612,8 @@
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_) {
diff --git a/ui/views/widget/native_widget_mac.mm b/ui/views/widget/native_widget_mac.mm
index 8c3daaf4..ca8fcb64 100644
--- a/ui/views/widget/native_widget_mac.mm
+++ b/ui/views/widget/native_widget_mac.mm
@@ -34,12 +34,6 @@
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 @@
delegate_->OnNativeWidgetCreated(true);
- OnSizeConstraintsChanged();
-
bridge_->SetFocusManager(GetWidget()->GetFocusManager());
DCHECK(GetWidget()->GetRootView());
@@ -206,7 +198,8 @@
}
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 @@
}
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 {
diff --git a/ui/views/widget/widget_unittest.cc b/ui/views/widget/widget_unittest.cc
index be4c1df..b564366f 100644
--- a/ui/views/widget/widget_unittest.cc
+++ b/ui/views/widget/widget_unittest.cc
@@ -209,6 +209,67 @@
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 @@
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 @@
#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 @@
}
#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 @@
// 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 @@
EXPECT_EQ(activate_result, MA_ACTIVATE);
modal_dialog_widget->CloseNow();
- top_level_widget.CloseNow();
}
#endif // defined(OS_WIN)
#endif // !defined(OS_CHROMEOS)
