Prevents windows in chromeos from resizing bigger than their maximum size. (issue 11366215)

43 views
Skip to first unread message

k...@chromium.org

unread,
Nov 13, 2012, 1:08:38 AM11/13/12
to s...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
Reviewers: sky,

Description:
Prevents windows in chromeos from resizing bigger than their maximum size.


BUG=152065


Please review this at https://codereview.chromium.org/11366215/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M ash/wm/custom_frame_view_ash.h
M ash/wm/custom_frame_view_ash.cc
M ash/wm/frame_painter.h
M ash/wm/frame_painter.cc
M ash/wm/window_resizer.cc
M content/browser/renderer_host/render_widget_host_view_aura.h
M content/browser/renderer_host/render_widget_host_view_aura.cc
M content/browser/web_contents/web_contents_view_aura.h
M content/browser/web_contents/web_contents_view_aura.cc
M ui/aura/window_delegate.h
M ui/views/widget/native_widget_aura.h
M ui/views/widget/native_widget_aura.cc


Index: ash/wm/custom_frame_view_ash.cc
diff --git a/ash/wm/custom_frame_view_ash.cc
b/ash/wm/custom_frame_view_ash.cc
index
a7f7517a9b9a4b1dba6faa2864ef7ffde305932b..5c880b7794070a426d67f6322b02889dd9ced4aa
100644
--- a/ash/wm/custom_frame_view_ash.cc
+++ b/ash/wm/custom_frame_view_ash.cc
@@ -149,6 +149,10 @@ gfx::Size CustomFrameViewAsh::GetMinimumSize() {
return frame_painter_->GetMinimumSize(this);
}

+gfx::Size CustomFrameViewAsh::GetMaximumSize() {
+ return frame_painter_->GetMaximumSize(this);
+}
+

////////////////////////////////////////////////////////////////////////////////
// views::ButtonListener overrides:
void CustomFrameViewAsh::ButtonPressed(views::Button* sender,
Index: ash/wm/custom_frame_view_ash.h
diff --git a/ash/wm/custom_frame_view_ash.h b/ash/wm/custom_frame_view_ash.h
index
c09710f66229f4626c4cb5e3cfc318ebbb91f0af..5c3f0c397122cf90ce2ae45c18bd5d20542aef74
100644
--- a/ash/wm/custom_frame_view_ash.h
+++ b/ash/wm/custom_frame_view_ash.h
@@ -71,6 +71,7 @@ class ASH_EXPORT CustomFrameViewAsh : public
views::NonClientFrameView,
virtual std::string GetClassName() const OVERRIDE;
virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
virtual gfx::Size GetMinimumSize() OVERRIDE;
+ virtual gfx::Size GetMaximumSize() OVERRIDE;

// views::ButtonListener overrides:
virtual void ButtonPressed(views::Button* sender,
Index: ash/wm/frame_painter.cc
diff --git a/ash/wm/frame_painter.cc b/ash/wm/frame_painter.cc
index
6ed11275b8fd382bda28117f2c6d9677c9924949..cf3656402ad60b8cd65966f3b6b9c20ec128b45f
100644
--- a/ash/wm/frame_painter.cc
+++ b/ash/wm/frame_painter.cc
@@ -314,6 +314,10 @@ gfx::Size
FramePainter::GetMinimumSize(views::NonClientFrameView* view) {
return min_size;
}

+gfx::Size FramePainter::GetMaximumSize(views::NonClientFrameView* view) {
+ return frame_->client_view()->GetMaximumSize();
+}
+
int FramePainter::GetRightInset() const {
gfx::Size close_size = close_button_->GetPreferredSize();
gfx::Size size_button_size = size_button_->GetPreferredSize();
Index: ash/wm/frame_painter.h
diff --git a/ash/wm/frame_painter.h b/ash/wm/frame_painter.h
index
977fd7e9f4c45a41db47d4bb536ce1cf7c9a6521..af5089959158cfc8eb4208037bc5e33df0a15dbf
100644
--- a/ash/wm/frame_painter.h
+++ b/ash/wm/frame_painter.h
@@ -84,6 +84,7 @@ class ASH_EXPORT FramePainter : public
aura::WindowObserver,
int NonClientHitTest(views::NonClientFrameView* view,
const gfx::Point& point);
gfx::Size GetMinimumSize(views::NonClientFrameView* view);
+ gfx::Size GetMaximumSize(views::NonClientFrameView* view);

// Returns the inset from the right edge.
int GetRightInset() const;
Index: ash/wm/window_resizer.cc
diff --git a/ash/wm/window_resizer.cc b/ash/wm/window_resizer.cc
index
16070e7eaa21b9bb2048cdb2642a593b9cbcf283..51fca5b07fd6fd0589592c794241ca657a47c558
100644
--- a/ash/wm/window_resizer.cc
+++ b/ash/wm/window_resizer.cc
@@ -324,6 +324,9 @@ int WindowResizer::GetWidthForDrag(const Details&
details,
// And don't let the window go bigger than the display.
int max_width = Shell::GetScreen()->GetDisplayNearestWindow(
details.window).bounds().width();
+ gfx::Size max_size = details.window->delegate()->GetMaximumSize();
+ if (!max_size.IsEmpty())
+ max_width = std::min(max_width, max_size.width());
if (width > max_width) {
width = max_width;
*delta_x = -x_multiplier * (details.initial_bounds_in_parent.width()
-
@@ -354,6 +357,9 @@ int WindowResizer::GetHeightForDrag(const Details&
details,
// And don't let the window go bigger than the display.
int max_height = Shell::GetScreen()->GetDisplayNearestWindow(
details.window).bounds().height();
+ gfx::Size max_size = details.window->delegate()->GetMaximumSize();
+ if (!max_size.IsEmpty())
+ max_height = std::min(max_height, max_size.height());
if (height > max_height) {
height = max_height;
*delta_y = -y_multiplier *
(details.initial_bounds_in_parent.height() -
Index: content/browser/renderer_host/render_widget_host_view_aura.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc
b/content/browser/renderer_host/render_widget_host_view_aura.cc
index
9ffe30b8b3b2d6cf325b803dc3297246ae167aa3..db7f4c9bf2fd23aee95a9770ff2fbcef4af82a14
100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -1327,6 +1327,10 @@ gfx::Size RenderWidgetHostViewAura::GetMinimumSize()
const {
return gfx::Size();
}

+gfx::Size RenderWidgetHostViewAura::GetMaximumSize() const {
+ return gfx::Size();
+}
+
void RenderWidgetHostViewAura::OnBoundsChanged(const gfx::Rect& old_bounds,
const gfx::Rect&
new_bounds) {
// We care about this only in fullscreen mode, where there is no
Index: content/browser/renderer_host/render_widget_host_view_aura.h
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.h
b/content/browser/renderer_host/render_widget_host_view_aura.h
index
e01e0bbdd4b449e27947c5f901c848dc9717f60a..2c701c5608c56bc3d942dc54f1698a3c1837824f
100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.h
+++ b/content/browser/renderer_host/render_widget_host_view_aura.h
@@ -169,6 +169,7 @@ class RenderWidgetHostViewAura

// Overridden from aura::WindowDelegate:
virtual gfx::Size GetMinimumSize() const OVERRIDE;
+ virtual gfx::Size GetMaximumSize() const OVERRIDE;
virtual void OnBoundsChanged(const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) OVERRIDE;
virtual void OnFocus(aura::Window* old_focused_window) OVERRIDE;
Index: content/browser/web_contents/web_contents_view_aura.cc
diff --git a/content/browser/web_contents/web_contents_view_aura.cc
b/content/browser/web_contents/web_contents_view_aura.cc
index
a18411e141c9e830723343dfa1dae2df364892b2..46cd6e9946b3053104ee1fdd8f166c39811b5d24
100644
--- a/content/browser/web_contents/web_contents_view_aura.cc
+++ b/content/browser/web_contents/web_contents_view_aura.cc
@@ -502,6 +502,10 @@ gfx::Size WebContentsViewAura::GetMinimumSize() const {
return gfx::Size();
}

+gfx::Size WebContentsViewAura::GetMaximumSize() const {
+ return gfx::Size();
+}
+
void WebContentsViewAura::OnBoundsChanged(const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) {
SizeChangedCommon(new_bounds.size());
Index: content/browser/web_contents/web_contents_view_aura.h
diff --git a/content/browser/web_contents/web_contents_view_aura.h
b/content/browser/web_contents/web_contents_view_aura.h
index
6fc2ae3f77a391ab90f61575c98674258276b4bc..3ccf09637fae8c376954fa95ecc6d5121b4ccb92
100644
--- a/content/browser/web_contents/web_contents_view_aura.h
+++ b/content/browser/web_contents/web_contents_view_aura.h
@@ -88,6 +88,7 @@ class CONTENT_EXPORT WebContentsViewAura

// Overridden from aura::WindowDelegate:
virtual gfx::Size GetMinimumSize() const OVERRIDE;
+ virtual gfx::Size GetMaximumSize() const OVERRIDE;
virtual void OnBoundsChanged(const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) OVERRIDE;
virtual void OnFocus(aura::Window* old_focused_window) OVERRIDE;
Index: ui/aura/window_delegate.h
diff --git a/ui/aura/window_delegate.h b/ui/aura/window_delegate.h
index
fb98de77f720ed919675d765d261d48fbc882f43..b14a0c6308661ff2134ab307c8d6e6757bd47a60
100644
--- a/ui/aura/window_delegate.h
+++ b/ui/aura/window_delegate.h
@@ -36,6 +36,9 @@ class AURA_EXPORT WindowDelegate : public
ui::EventHandler {
// Returns the window's minimum size, or size 0,0 if there is no limit.
virtual gfx::Size GetMinimumSize() const = 0;

+ // Returns the window's maximum size, or size 0,0 if there is no limit.
+ virtual gfx::Size GetMaximumSize() const = 0;
+
// Called when the Window's position and/or size changes.
virtual void OnBoundsChanged(const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) = 0;
Index: ui/views/widget/native_widget_aura.cc
diff --git a/ui/views/widget/native_widget_aura.cc
b/ui/views/widget/native_widget_aura.cc
index
807c185d6d73cff299e7602f91907e00b36cf032..227c91bd2a1e863d1a79911920af908887cf4fd2
100644
--- a/ui/views/widget/native_widget_aura.cc
+++ b/ui/views/widget/native_widget_aura.cc
@@ -645,6 +645,10 @@ gfx::Size NativeWidgetAura::GetMinimumSize() const {
return delegate_->GetMinimumSize();
}

+gfx::Size NativeWidgetAura::GetMaximumSize() const {
+ return delegate_->GetMaximumSize();
+}
+
void NativeWidgetAura::OnBoundsChanged(const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) {
if (old_bounds.origin() != new_bounds.origin())
Index: ui/views/widget/native_widget_aura.h
diff --git a/ui/views/widget/native_widget_aura.h
b/ui/views/widget/native_widget_aura.h
index
a2e0d7de081676aa8f1ca62d5f008eddd49543db..99497fb6655ee40ea6e0d706296650ff196a9906
100644
--- a/ui/views/widget/native_widget_aura.h
+++ b/ui/views/widget/native_widget_aura.h
@@ -132,6 +132,7 @@ class VIEWS_EXPORT NativeWidgetAura : public
internal::NativeWidgetPrivate,

// Overridden from aura::WindowDelegate:
virtual gfx::Size GetMinimumSize() const OVERRIDE;
+ virtual gfx::Size GetMaximumSize() const OVERRIDE;
virtual void OnBoundsChanged(const gfx::Rect& old_bounds,
const gfx::Rect& new_bounds) OVERRIDE;
virtual void OnFocus(aura::Window* old_focused_window) OVERRIDE;


s...@chromium.org

unread,
Nov 13, 2012, 12:29:41 PM11/13/12
to k...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org, sku...@chromium.org
You also need to deal with window snapping and layout of attached windows.

I'm adding Stefan on the cc list for this as he's adding support for
properly
dealing with windows that aren't resizable at all.

https://codereview.chromium.org/11366215/

k...@chromium.org

unread,
Nov 13, 2012, 7:42:38 PM11/13/12
to s...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org, sku...@chromium.org
On 2012/11/13 17:29:40, sky wrote:
> You also need to deal with window snapping and layout of attached windows.

> I'm adding Stefan on the cc list for this as he's adding support for
> properly
> dealing with windows that aren't resizable at all.

Cool, thanks. I've changed the definition of
ShellWindowViews::CanMaximize() to
disable maximise / snap to left / snap to right when a window has a max size
defined. Is this what you meant by window snapping?

Stefan, if you're going to add support for windows that aren't resizable at
all,
could you also consider the case where windows have constraints on their
min/max
size?

Scott, in either case, is this okay to land this patch without handling the
attached windows case?

https://codereview.chromium.org/11366215/

sku...@chromium.org

unread,
Nov 13, 2012, 8:09:33 PM11/13/12
to k...@chromium.org, s...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
My CL has landed now.

I have seen that a constraint for the minimum size - however I have not
seen a
constrained for the maximum size.

If you want I can add that of course. :)

https://codereview.chromium.org/11366215/

s...@chromium.org

unread,
Nov 13, 2012, 9:40:27 PM11/13/12
to k...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
If you're going to fix the attached case and snapping code soon after, I'm
ok
with that landing in another patch. Is that the case?

https://codereview.chromium.org/11366215/

s...@chromium.org

unread,
Nov 13, 2012, 9:40:48 PM11/13/12
to k...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
That said, you do need some test coverage in this patch.

https://codereview.chromium.org/11366215/

k...@chromium.org

unread,
Nov 14, 2012, 1:24:27 AM11/14/12
to s...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
On 2012/11/14 02:40:48, sky wrote:
> That said, you do need some test coverage in this patch.

Okay, I've added some unit tests and fixed snapping (see the updated
description
for more).

Stefan, if you're keen to handle min/max sizes for attached windows that'd
be
awesome :-)

https://codereview.chromium.org/11366215/

s...@chromium.org

unread,
Nov 14, 2012, 9:55:10 AM11/14/12
to k...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
LGTM - follow up with Stefan to make sure one of you deal with the attached
case.

https://codereview.chromium.org/11366215/

jer...@chromium.org

unread,
Nov 14, 2012, 8:12:43 PM11/14/12
to k...@chromium.org, s...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, benw...@chromium.org, tap...@chromium.org

https://codereview.chromium.org/11366215/diff/6014/ash/wm/window_resizer.cc
File ash/wm/window_resizer.cc (right):

https://codereview.chromium.org/11366215/diff/6014/ash/wm/window_resizer.cc#newcode328
ash/wm/window_resizer.cc:328: if (!max_size.IsEmpty())
Perhaps you only want to check the max width here? A window can have a
max width without having a max height, and size.IsEmpty() returns true
if *either* of the width/height are 0.

https://codereview.chromium.org/11366215/diff/6014/ash/wm/workspace/workspace_window_resizer.cc
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/11366215/diff/6014/ash/wm/workspace/workspace_window_resizer.cc#newcode758
ash/wm/workspace/workspace_window_resizer.cc:758: if
(!window()->delegate()->GetMaximumSize().IsEmpty())
Maybe abstract this into ash::wm::CanSnapWindow() (in
ash/wm/window_util.h)?

https://codereview.chromium.org/11366215/

k...@chromium.org

unread,
Nov 14, 2012, 9:46:13 PM11/14/12
to s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
On 2012/11/15 01:12:43, jeremya wrote:
> Perhaps you only want to check the max width here? A window can have a
max width
> without having a max height, and size.IsEmpty() returns true if
*either* of the
> width/height are 0.

Good catch. Fixed here and below.

https://codereview.chromium.org/11366215/diff/6014/ash/wm/workspace/workspace_window_resizer.cc
File ash/wm/workspace/workspace_window_resizer.cc (right):

https://codereview.chromium.org/11366215/diff/6014/ash/wm/workspace/workspace_window_resizer.cc#newcode758
ash/wm/workspace/workspace_window_resizer.cc:758: if
(!window()->delegate()->GetMaximumSize().IsEmpty())
On 2012/11/15 01:12:43, jeremya wrote:
> Maybe abstract this into ash::wm::CanSnapWindow() (in
ash/wm/window_util.h)?

Done.

https://codereview.chromium.org/11366215/

commi...@chromium.org

unread,
Nov 14, 2012, 9:58:41 PM11/14/12
to k...@chromium.org, s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org

commi...@chromium.org

unread,
Nov 14, 2012, 10:39:59 PM11/14/12
to k...@chromium.org, s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=58691
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

https://chromiumcodereview.appspot.com/11366215/

commi...@chromium.org

unread,
Nov 15, 2012, 6:24:57 PM11/15/12
to k...@chromium.org, s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org

commi...@chromium.org

unread,
Nov 15, 2012, 7:04:31 PM11/15/12
to k...@chromium.org, s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chromeos&number=59043

commi...@chromium.org

unread,
Nov 15, 2012, 8:26:17 PM11/15/12
to k...@chromium.org, s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org

commi...@chromium.org

unread,
Nov 17, 2012, 9:14:40 AM11/17/12
to k...@chromium.org, s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org

commi...@chromium.org

unread,
Nov 17, 2012, 10:57:22 AM11/17/12
to k...@chromium.org, s...@chromium.org, jer...@chromium.org, chromium...@chromium.org, sad...@chromium.org, yusuke...@chromium.org, ben+...@chromium.org, tfa...@chromium.org, j...@chromium.org, penghua...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, su...@chromium.org, jer...@chromium.org, benw...@chromium.org, tap...@chromium.org
Reply all
Reply to author
Forward
0 new messages