Aura: Add non-browser windows into the list of "Alt + Tab" cycle list.

Aura: Add non-browser windows into the list of "Alt + Tab" cycle.

This CL changes to use the child window list of the default container as the list of "Alt + Tab" cycle, instead of the browsers' list. But the list excludes the window which is not forcusable and has modal window as a child.

This CL can enable us to focus task manager window with "Alt + Tab" switcher.

BUG=118611
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=129443

[yoshiki, 2012-03-21 15:01:12]

sky: Could you take a look and approve to check-in?

[sky, 2012-03-21 15:26:18]

You'll want to add some tests too.

http://codereview.chromium.org/9719037/diff/6002/chrome/browser/ui/views/ash/...
File chrome/browser/ui/views/ash/chrome_shell_delegate.cc (right):

http://codereview.chromium.org/9719037/diff/6002/chrome/browser/ui/views/ash/...
chrome/browser/ui/views/ash/chrome_shell_delegate.cc:5: #include <algorithm>
This should be between 7 and 8, eg:

#include "chrome_shel..

#include <alogrith>

#include the rest

http://codereview.chromium.org/9719037/diff/6002/chrome/browser/ui/views/ash/...
chrome/browser/ui/views/ash/chrome_shell_delegate.cc:33: bool
ShouldSkipOnCycle(aura::Window* window) {
The ActivationClient is ultimately the place that decides if a window can be
activated. You should expose a method there for this test.

http://codereview.chromium.org/9719037/diff/6002/chrome/browser/ui/views/ash/...
chrome/browser/ui/views/ash/chrome_shell_delegate.cc:98: std::remove_if(
indent this line 4.

[yoshiki, 2012-03-27 04:20:24]

sky: PTAL. I've fixed as the review and written the test. I wonder if
"CanActivateWindowItself" is prefer name or not. If you have a better name idea,
let me know.

[sky, 2012-03-27 14:59:58]

http://codereview.chromium.org/9719037/diff/16003/ash/wm/activation_controlle...
File ash/wm/activation_controller.cc (right):

http://codereview.chromium.org/9719037/diff/16003/ash/wm/activation_controlle...
ash/wm/activation_controller.cc:76: !event ||
Why are you adding the '!event' clause? It is legal to pass NULL to
CanActivateWindow.

http://codereview.chromium.org/9719037/diff/16003/ash/wm/activation_controlle...
ash/wm/activation_controller.cc:78: SupportsChildActivation(window->parent());
fix indentation.

http://codereview.chromium.org/9719037/diff/16003/ash/wm/activation_controller.h
File ash/wm/activation_controller.h (right):

http://codereview.chromium.org/9719037/diff/16003/ash/wm/activation_controlle...
ash/wm/activation_controller.h:37: // Determines if the given window itself can
be activated or not. Returns
How about:
Returns true if |window| can be activated, false otherwise. If |window| has a
modal child it can not be activated.

http://codereview.chromium.org/9719037/diff/16003/ash/wm/activation_controlle...
ash/wm/activation_controller.h:41: static bool
CanActivateWindowItself(aura::Window* window);
Just CanActivateWindow, and this should be a method on ActivationClient.

[yoshiki, 2012-03-28 15:08:41]

sky: PTAL. I've changed the name of the anonymous function from
(anonymous)::CanActivateWindow to (anonymous)::CanActivateWindowWithEvent in
activation_controller.cc, because it conflicts with
ActivationController::CanActivateWindow. May it be better to use
internal::CanActivateWindow insterd?

https://chromiumcodereview.appspot.com/9719037/diff/16003/ash/wm/activation_c...
File ash/wm/activation_controller.cc (right):

https://chromiumcodereview.appspot.com/9719037/diff/16003/ash/wm/activation_c...
ash/wm/activation_controller.cc:76: !event ||
On 2012/03/27 14:59:58, sky wrote:
> Why are you adding the '!event' clause? It is legal to pass NULL to
> CanActivateWindow.

Done. Un-changed.

https://chromiumcodereview.appspot.com/9719037/diff/16003/ash/wm/activation_c...
ash/wm/activation_controller.cc:78: SupportsChildActivation(window->parent());
On 2012/03/27 14:59:58, sky wrote:
> fix indentation.

Done.

https://chromiumcodereview.appspot.com/9719037/diff/16003/ash/wm/activation_c...
File ash/wm/activation_controller.h (right):

https://chromiumcodereview.appspot.com/9719037/diff/16003/ash/wm/activation_c...
ash/wm/activation_controller.h:37: // Determines if the given window itself can
be activated or not. Returns
On 2012/03/27 14:59:58, sky wrote:
> How about:
> Returns true if |window| can be activated, false otherwise. If |window| has a
> modal child it can not be activated.

Done. Thanks, I've used it. And the comment has been moved to
activation_client.h.

https://chromiumcodereview.appspot.com/9719037/diff/16003/ash/wm/activation_c...
ash/wm/activation_controller.h:41: static bool
CanActivateWindowItself(aura::Window* window);
On 2012/03/27 14:59:58, sky wrote:
> Just CanActivateWindow, and this should be a method on ActivationClient.

Done.

[sky, 2012-03-28 15:57:42]

LGTM

[commit-bot: I haz the power, 2012-03-28 16:09:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/9719037/20002

[commit-bot: I haz the power, 2012-03-28 18:08:23]

Change committed as 129443