Avoids centering docked windows, Unsnaps when centering.

Avoids centering docked windows, Unsnaps when centering.

BUG=322196
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=237143

[varkha, 2013-11-21 21:30:42]

PTAL. This is a small patch liberated from
https://codereview.chromium.org/68033003/.

[varkha, 2013-11-21 22:53:55]

+oshima, PTAL. This is correcting another implication of starting to treat
snapped show types as states. It should be orthogonal to
https://codereview.chromium.org/68033003/. Thanks!

https://codereview.chromium.org/79023009/diff/190001/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/79023009/diff/190001/ash/wm/window_util.cc#ne...
ash/wm/window_util.cc:70: window_state->Restore();
Restore is needed here to properly reset window_show_type_ to SHOW_TYPE_NORMAL
if a window was snapped since SetBoundsInScreen() does not change it.

[oshima, 2013-11-21 23:27:16]

change lg. can you add test?

[pkotwicz, 2013-11-21 23:46:48]

LGTM with nits

https://codereview.chromium.org/79023009/diff/190001/ash/accelerators/acceler...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/79023009/diff/190001/ash/accelerators/acceler...
ash/accelerators/accelerator_controller.cc:875: // notifications are not sent.
Disable centering for docked windows.
I find it hard to understand the comment. This may be clearer:
"DockedWindowLayoutManager does not support windows leaving the dock as a result
of a bounds change outside the context of a drag. Disable centering for docked
windows."

https://codereview.chromium.org/79023009/diff/190001/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/79023009/diff/190001/ash/wm/window_util.cc#ne...
ash/wm/window_util.cc:60: if (!window_state->IsNormalShowState())
I have trouble keeping show type and show state straight.
oshima@, can you please comment? I suggest using show type for any new ash code
(with the goal of removing all uses of show state in ash code)

[pkotwicz, 2013-11-21 23:46:50]

LGTM with nits

https://codereview.chromium.org/79023009/diff/190001/ash/accelerators/acceler...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/79023009/diff/190001/ash/accelerators/acceler...
ash/accelerators/accelerator_controller.cc:875: // notifications are not sent.
Disable centering for docked windows.
I find it hard to understand the comment. This may be clearer:
"DockedWindowLayoutManager does not support windows leaving the dock as a result
of a bounds change outside the context of a drag. Disable centering for docked
windows."

https://codereview.chromium.org/79023009/diff/190001/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/79023009/diff/190001/ash/wm/window_util.cc#ne...
ash/wm/window_util.cc:60: if (!window_state->IsNormalShowState())
I have trouble keeping show type and show state straight.
oshima@, can you please comment? I suggest using show type for any new ash code
(with the goal of removing all uses of show state in ash code)

[pkotwicz, 2013-11-21 23:48:51]

Can you please add a UMA stat? (I want it so that we can evaluate whether we can
remove the accelerator in a future milestone)

[varkha, 2013-11-22 03:43:37]

I've added test. Please take a look.

https://codereview.chromium.org/79023009/diff/190001/ash/accelerators/acceler...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/79023009/diff/190001/ash/accelerators/acceler...
ash/accelerators/accelerator_controller.cc:875: // notifications are not sent.
Disable centering for docked windows.
On 2013/11/21 23:46:50, pkotwicz wrote:
> I find it hard to understand the comment. This may be clearer:
> "DockedWindowLayoutManager does not support windows leaving the dock as a
result
> of a bounds change outside the context of a drag. Disable centering for docked
> windows."

Done.

https://codereview.chromium.org/79023009/diff/420001/ash/accelerators/acceler...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/79023009/diff/420001/ash/accelerators/acceler...
ash/accelerators/accelerator_controller.cc:873:
content::RecordAction(content::UserMetricsAction("Accel_Center"));
I have already added UMA count for this. I moved it a bit above to make
invocation location more logical.

[oshima, 2013-11-25 18:49:12]

lgtm

[commit-bot: I haz the power, 2013-11-25 19:01:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/varkha@chromium.org/79023009/420001

[commit-bot: I haz the power, 2013-11-25 21:20:01]

Change committed as 237143