[ash-md] Properly scales windows with transient parents in overview

[ash-md] Properly scales windows with transient parents in overview

Previously to this CL the code was only considering the focused window
when calculating scale for the overview mode transform. It needs to
consider the bounding rectangle of the whole transient parent's tree.

BUG=632694
Committed: https://crrev.com/0e4a6217e4694b15ba76a51b406254e9a4f22e1e
Cr-Commit-Position: refs/heads/master@{#408763}

[varkha, 2016-07-29 19:52:48]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 19:53:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[varkha, 2016-07-29 19:53:58]

varkha@chromium.org changed reviewers:
+ tdanderson@chromium.org

[varkha, 2016-07-29 19:53:59]

tdanderson@, can you please take a look. This is a stable blocker in M-53.
Thanks!

[tdanderson, 2016-07-29 20:20:02]

lgtm with a few comments

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
File ash/common/wm/overview/scoped_transform_overview_window.cc (right):

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.cc:349: int top_inset =
hide_header ? GetTopInset() : 0;
nit: const

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.cc:383: // selector
target item.
Did you mean to have this comment here? I think something like "// Return 0
if..." would be more helpful.

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.cc:384: if (window ==
window_ || (window->GetType() != ui::wm::WINDOW_TYPE_NORMAL &&
Perhaps restructure as this instead:

for (...) {
  if (!...)  // negation of the current boolean expression
    return 0;
}

and/or consider moving this expression to a helper function which can also be
called at lines 354-356.

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
File ash/common/wm/overview/scoped_transform_overview_window.h (right):

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.h:93: // space. The
returned Rect is in virtual screen coordinates.
nit: include comment about the purpose of |hide_header|. Also mention that the
returned Rect can be the union of window bounds along the transient ancestor
path

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/wind...
File ash/common/wm/overview/window_selector_item.h (right):

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector_item.h:87: // Returns the original target
bounds of all transformed windows.
nit: explain in a bit more detail what you mean by "all transformed windows" in
this context

[commit-bot: I haz the power, 2016-07-29 20:34:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 20:34:24]

Dry run: This issue passed the CQ dry run.

[varkha, 2016-07-29 20:44:31]

Patchset #2 (id:20001) has been deleted

[varkha, 2016-07-29 20:45:12]

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
File ash/common/wm/overview/scoped_transform_overview_window.cc (right):

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.cc:349: int top_inset =
hide_header ? GetTopInset() : 0;
On 2016/07/29 20:20:02, tdanderson wrote:
> nit: const

Done.

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.cc:383: // selector
target item.
On 2016/07/29 20:20:02, tdanderson wrote:
> Did you mean to have this comment here? I think something like "// Return 0
> if..." would be more helpful.

Done.

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.cc:384: if (window ==
window_ || (window->GetType() != ui::wm::WINDOW_TYPE_NORMAL &&
On 2016/07/29 20:20:02, tdanderson wrote:
> Perhaps restructure as this instead:
> 
> for (...) {
>   if (!...)  // negation of the current boolean expression
>     return 0;
> }
> 
> and/or consider moving this expression to a helper function which can also be
> called at lines 354-356.

Done.

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
File ash/common/wm/overview/scoped_transform_overview_window.h (right):

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/scop...
ash/common/wm/overview/scoped_transform_overview_window.h:93: // space. The
returned Rect is in virtual screen coordinates.
On 2016/07/29 20:20:02, tdanderson wrote:
> nit: include comment about the purpose of |hide_header|. Also mention that the
> returned Rect can be the union of window bounds along the transient ancestor
> path

Done.

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/wind...
File ash/common/wm/overview/window_selector_item.h (right):

https://codereview.chromium.org/2197773002/diff/1/ash/common/wm/overview/wind...
ash/common/wm/overview/window_selector_item.h:87: // Returns the original target
bounds of all transformed windows.
On 2016/07/29 20:20:02, tdanderson wrote:
> nit: explain in a bit more detail what you mean by "all transformed windows"
in
> this context

Done.

[varkha, 2016-07-29 20:45:20]

The CQ bit was checked by varkha@chromium.org

[varkha, 2016-07-29 20:45:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org
Link to the patchset: https://codereview.chromium.org/2197773002/#ps40001
(title: "[ash-md] Disables rounded corners by default in overview mode (nits)")

[commit-bot: I haz the power, 2016-07-29 20:45:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 21:23:42]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-07-29 21:26:49]

Description was changed from

==========
[ash-md] Properly scales windows with transient parents in overview

Previously to this CL the code was only considering the focused window
when calculating scale for the overview mode transform. It needs to
consider the bounding rectangle of the whole transient parent's tree.

BUG=632694
==========

to

==========
[ash-md] Properly scales windows with transient parents in overview

Previously to this CL the code was only considering the focused window
when calculating scale for the overview mode transform. It needs to
consider the bounding rectangle of the whole transient parent's tree.

BUG=632694

Committed: https://crrev.com/0e4a6217e4694b15ba76a51b406254e9a4f22e1e
Cr-Commit-Position: refs/heads/master@{#408763}
==========

[commit-bot: I haz the power, 2016-07-29 21:26:51]

Patchset 2 (id:??) landed as
https://crrev.com/0e4a6217e4694b15ba76a51b406254e9a4f22e1e
Cr-Commit-Position: refs/heads/master@{#408763}