Fixing problems with constrained windows (i.e. print preview)

Fixing problems with constrained windows (i.e. print preview)


BUG=135103, 135096, 135116
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149497

[Mr4D (OOO till 08-26), 2012-07-31 17:40:17]

Please have a look!

many thanks!
-Stefan

[Ben Goodger (Google), 2012-07-31 17:59:35]

http://codereview.chromium.org/10826085/diff/1/ash/wm/frame_painter.cc
File ash/wm/frame_painter.cc (right):

http://codereview.chromium.org/10826085/diff/1/ash/wm/frame_painter.cc#newcod...
ash/wm/frame_painter.cc:616: (*it)->window_->parent()->layout_manager())) {
hrm... this seems like it could be brittle, for example if we were to add a
layout manager to chrome windows somehow.

It seems like we should have a function somewhere, maybe in ash/wm/window_util.h
that determines if a window is a direct descendent of one of the window
containers (default or always-on-top containers). you could add that function
and call it here instead of doing this.

[Mr4D (OOO till 08-26), 2012-07-31 19:37:26]

Addressed! Have another look!

Thanks!

http://codereview.chromium.org/10826085/diff/1/ash/wm/frame_painter.cc
File ash/wm/frame_painter.cc (right):

http://codereview.chromium.org/10826085/diff/1/ash/wm/frame_painter.cc#newcod...
ash/wm/frame_painter.cc:616: (*it)->window_->parent()->layout_manager())) {
I added a property flag which seems to be a better way to do it. (as discussed).

[Ben Goodger (Google), 2012-08-01 15:40:09]

http://codereview.chromium.org/10826085/diff/4002/ui/aura/client/aura_constan...
File ui/aura/client/aura_constants.h (right):

http://codereview.chromium.org/10826085/diff/4002/ui/aura/client/aura_constan...
ui/aura/client/aura_constants.h:30: AURA_EXPORT extern const
WindowProperty<bool>* const kConstrainedWindowKey;
i'd rather not have this property live in aura. instead, put it in ash.

[Mr4D (OOO till 08-26), 2012-08-01 16:42:28]

Done. Please have another look!

http://codereview.chromium.org/10826085/diff/4002/ui/aura/client/aura_constan...
File ui/aura/client/aura_constants.h (right):

http://codereview.chromium.org/10826085/diff/4002/ui/aura/client/aura_constan...
ui/aura/client/aura_constants.h:30: AURA_EXPORT extern const
WindowProperty<bool>* const kConstrainedWindowKey;
On 2012/08/01 15:40:09, Ben Goodger (Google) wrote:
> i'd rather not have this property live in aura. instead, put it in ash.

Done.

[Ben Goodger (Google), 2012-08-01 19:09:40]

lgtm with one nit

http://codereview.chromium.org/10826085/diff/5006/ash/ash_constants.cc
File ash/ash_constants.cc (right):

http://codereview.chromium.org/10826085/diff/5006/ash/ash_constants.cc#newcode12
ash/ash_constants.cc:12: namespace client {
no client namespace. (namespaces are supposed to follow directories).

[Mr4D (OOO till 08-26), 2012-08-01 20:02:28]

Many thanks!

http://codereview.chromium.org/10826085/diff/5006/ash/ash_constants.cc
File ash/ash_constants.cc (right):

http://codereview.chromium.org/10826085/diff/5006/ash/ash_constants.cc#newcode12
ash/ash_constants.cc:12: namespace client {
Ohhh. I didn't know that. Thanks!
Done.

[commit-bot: I haz the power, 2012-08-01 20:03:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/10826085/3002

[commit-bot: I haz the power, 2012-08-01 21:39:57]

Change committed as 149497