Auto hide app list on Windows when it loses focus.

Auto hide app list on Windows when it loses focus.

This is a bit tricky as the app list should not hide when the taskbar gets focus. Handling this involves using a timer, when the app list does not have focus, to check whether either the taskbar or the app list has focus. Once one of these is not true the app list is closed.

This change also ensures that the app list is never opened multiple times.

Also includes is a bit of cleanup:
- AppListController has been renamed to AppListControllerDelegate
- a singleton AppListController has been introduced for Windows, similar to ash
- default implementations have been added to AppListControllerDelegate where it makes sense.

BUG=152847, 152846
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=161474

[xiyuan, 2012-10-09 17:43:15]

https://chromiumcodereview.appspot.com/11094019/diff/12002/chrome/browser/ui/...
File chrome/browser/ui/views/app_list/app_list_controller_win.cc (right):

https://chromiumcodereview.appspot.com/11094019/diff/12002/chrome/browser/ui/...
chrome/browser/ui/views/app_list/app_list_controller_win.cc:62: bool
timer_running_;
Where is |timer_running_| used?

https://chromiumcodereview.appspot.com/11094019/diff/12002/chrome/browser/ui/...
chrome/browser/ui/views/app_list/app_list_controller_win.cc:98: // Weak pointer.
The view manageds its own lifetime.
nit: manageds -> manages

https://chromiumcodereview.appspot.com/11094019/diff/12002/chrome/browser/ui/...
chrome/browser/ui/views/app_list/app_list_controller_win.cc:339: if
(focused_hwnd == jump_list_hwnd || focused_hwnd == taskbar_hwnd ||
nit: one condition per line

https://chromiumcodereview.appspot.com/11094019/diff/12002/ui/app_list/app_li...
File ui/app_list/app_list_view.cc (right):

https://chromiumcodereview.appspot.com/11094019/diff/12002/ui/app_list/app_li...
ui/app_list/app_list_view.cc:203: void
AppListView::OnWidgetClosing(views::Widget* widget) {
Call BubbleDelegateView::OnWidgetClosing or add a comment to explain why we
don't need to call it.

https://chromiumcodereview.appspot.com/11094019/diff/12002/ui/app_list/app_li...
ui/app_list/app_list_view.cc:209: bool active) {
Add a comment to explain why we don't want to call
BubbleDelegateView::OnWidgetActivationChanged.

https://chromiumcodereview.appspot.com/11094019/diff/12002/ui/app_list/app_li...
File ui/app_list/app_list_view.h (right):

https://chromiumcodereview.appspot.com/11094019/diff/12002/ui/app_list/app_li...
ui/app_list/app_list_view.h:80: // Overridden from
views::WidgetDelegateObserver:
nit: views::WidgetObserver?

[benwells, 2012-10-10 00:55:21]

http://codereview.chromium.org/11094019/diff/12002/chrome/browser/ui/views/ap...
File chrome/browser/ui/views/app_list/app_list_controller_win.cc (right):

http://codereview.chromium.org/11094019/diff/12002/chrome/browser/ui/views/ap...
chrome/browser/ui/views/app_list/app_list_controller_win.cc:62: bool
timer_running_;
On 2012/10/09 17:43:15, xiyuan wrote:
> Where is |timer_running_| used?

Ah, that's a relic. Gone now!

http://codereview.chromium.org/11094019/diff/12002/chrome/browser/ui/views/ap...
chrome/browser/ui/views/app_list/app_list_controller_win.cc:98: // Weak pointer.
The view manageds its own lifetime.
On 2012/10/09 17:43:15, xiyuan wrote:
> nit: manageds -> manages

Done.

http://codereview.chromium.org/11094019/diff/12002/chrome/browser/ui/views/ap...
chrome/browser/ui/views/app_list/app_list_controller_win.cc:339: if
(focused_hwnd == jump_list_hwnd || focused_hwnd == taskbar_hwnd ||
On 2012/10/09 17:43:15, xiyuan wrote:
> nit: one condition per line

Done.

http://codereview.chromium.org/11094019/diff/12002/ui/app_list/app_list_view.cc
File ui/app_list/app_list_view.cc (right):

http://codereview.chromium.org/11094019/diff/12002/ui/app_list/app_list_view....
ui/app_list/app_list_view.cc:203: void
AppListView::OnWidgetClosing(views::Widget* widget) {
On 2012/10/09 17:43:15, xiyuan wrote:
> Call BubbleDelegateView::OnWidgetClosing or add a comment to explain why we
> don't need to call it.

Done (and also check widget).

http://codereview.chromium.org/11094019/diff/12002/ui/app_list/app_list_view....
ui/app_list/app_list_view.cc:209: bool active) {
On 2012/10/09 17:43:15, xiyuan wrote:
> Add a comment to explain why we don't want to call
> BubbleDelegateView::OnWidgetActivationChanged.

Done (and also check widget).

http://codereview.chromium.org/11094019/diff/12002/ui/app_list/app_list_view.h
File ui/app_list/app_list_view.h (right):

http://codereview.chromium.org/11094019/diff/12002/ui/app_list/app_list_view....
ui/app_list/app_list_view.h:80: // Overridden from
views::WidgetDelegateObserver:
On 2012/10/09 17:43:15, xiyuan wrote:
> nit: views::WidgetObserver?

Done.

[benwells, 2012-10-10 00:58:11]

sky: I need owners review from you for
chrome/browser/ui/ash/chrome_shell_delegate.cc

Also if you want to look at the way I'm using the timer and checking HWNDs I'd
appreciate it (app_controller_win.cc)

[xiyuan, 2012-10-10 03:15:59]

LGTM

[sky, 2012-10-10 04:31:19]

I added Carlos to the review. He might have ideas on finding the taskbar.

http://codereview.chromium.org/11094019/diff/7002/chrome/browser/ui/views/app...
File chrome/browser/ui/views/app_list/app_list_controller_win.cc (right):

http://codereview.chromium.org/11094019/diff/7002/chrome/browser/ui/views/app...
chrome/browser/ui/views/app_list/app_list_controller_win.cc:325: HWND
jump_list_hwnd = FindWindow(L"DV2ControlHost", NULL);
This seems incredibly hacky and fragiel. Is there no better way to do this?

[cpu_(ooo_6.6-7.5), 2012-10-11 17:37:40]

lgtm

I gave up, I don't see a better way to do this. At least the class names seems
stable from XP to Win7. No idea if this works on win8.

[sky, 2012-10-11 17:43:41]

LGTM

[commit-bot: I haz the power, 2012-10-11 23:17:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/11094019/7002

[commit-bot: I haz the power, 2012-10-12 01:54:19]

Change committed as 161474