wip: Refactoring Ash's AppListController, moving the bulk of the logic to chrome/browser/ui/ash/app…

wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
  manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
  (in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
  AppListLayoutDelegate contains ash-y type of functionality, such as updating
  the shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
  app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
  uses AppListShowerAsh directly.
Also see a rough design doc here: https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Known issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
  AppListServiceAsh and AppListControllerDelegateAsh. There is
  src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
  different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
  AppListController and AppListServiceAsh depend on AppListShowerAsh for
  manipulating the app list. This leads to an awkward PostTask in
  AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app list
  on a UI event outside the app list window. 

BUG=557408

[mfomitchev, 2016-03-07 21:02:04]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

BUG=557408
==========

[mfomitchev, 2016-03-07 21:17:27]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring (also see rough design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q):
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

BUG=557408
==========

[mfomitchev, 2016-03-07 21:18:01]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring (also see rough design doc:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q):
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q
In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

BUG=557408
==========

[mfomitchev, 2016-03-07 21:29:48]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q
In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q
In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

[mfomitchev, 2016-03-07 21:33:37]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q
In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q
In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

[mfomitchev, 2016-03-07 21:33:59]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q
In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

[mfomitchev, 2016-03-07 21:34:56]

mfomitchev@chromium.org changed reviewers:
+ mgiuca@chromium.org, sky@chromium.org

[mfomitchev, 2016-03-07 21:34:57]

PTAL:  Looking for general feedback about the design at this point.

[mfomitchev, 2016-03-07 21:51:04]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Known issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

[mfomitchev, 2016-03-07 21:53:40]

Description was changed from

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
(in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
AppListLayoutDelegate contains ash-y type of functionality, such as updating the
shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Known issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
AppListServiceAsh and AppListControllerDelegateAsh. There is
src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
AppListController and AppListServiceAsh depend on AppListShowerAsh for
manipulating the app list. This leads to an awkward PostTask in
AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list on a UI event outside the app list window. 

BUG=557408
==========

to

==========
wip: Refactoring Ash's AppListController, moving the bulk of the logic to
chrome/browser/ui/ash/app_list/

The CL is incomplete. Unit tests are not done, and configurations other than
chrome+ash (no mus) are not taken care of. Looking only for general feedback
about the design at this point.

This is in preparation for the Mus implementation of the app list.

Here is the gist of the refactoring:
- a new interface app_list::AppListController is introduced for ash to
  manipulate the app list.
- ash::AppListController (formerly in ash/wm) is split up into AppListShowerAsh
  (in chrome/browser/ui/ash/app_list) and AppListLayoutDelegate (in ash/wm).
  AppListLayoutDelegate contains ash-y type of functionality, such as updating
  the shelf launcher icon.
- AppListShowerAsh communicates with AppListLayoutDelegate via
  app_list::AppListLayoutDelegate.
- Chrome no longer calls into ash::Shell for app-list functionality. Instead it
  uses AppListShowerAsh directly.
Also see a rough design doc here:
https://docs.google.com/document/d/1M9vqTTuprMssRXs8UIVaFjQUq3lUxSzwrWoPjPvca6Q

In Mus+ash world, there will be a Mojo interface for app_list::AppListController
and a different implementation of app_list::AppListLayoutDelegate, which will
(at least initially) do less and will not communicate with Ash directly.

Known issues with the current implementation:
- AppListShowerAsh is a bit of an awkward class in the presence of
  AppListServiceAsh and AppListControllerDelegateAsh. There is
  src/chrome/browser/ui/app_list/app_list_shower_views.h, which is similar, but
  different.
- AppListShowerAsh depends on AppListController for AppListViewDelegate, but
  AppListController and AppListServiceAsh depend on AppListShowerAsh for
  manipulating the app list. This leads to an awkward PostTask in
  AppListServiceAsh constructor.
- ash::AppListLayoutDelegate does more than layout: e.g. it dismisses the app
list
  on a UI event outside the app list window. 

BUG=557408
==========

[sky, 2016-03-07 23:49:59]

I poked at your review a bit, mostly thinking about AppListController.
AppListController seems roughly like what I would expect to see. 

AppListShowerAsh is a bit wordy, Did you consider ChromeAppListController or
AshAppListController?

[mfomitchev, 2016-03-08 03:32:57]

On 2016/03/07 23:49:59, sky wrote:
> I poked at your review a bit, mostly thinking about AppListController.
> AppListController seems roughly like what I would expect to see. 
> 
> AppListShowerAsh is a bit wordy, Did you consider ChromeAppListController or
> AshAppListController?

There is a somewhat similar construct for Linux with AppListShower
(app_list_shower_views.h) and AppListServiceViews.
We already have AppListControllerDelegateAsh, and it is referred to as just
"controller" in a bunch of places, so I think having another "controller" would
be confusing. We could just merge the functionality into
AppListControllerDelegateAsh, but I thought it would be better to keep it in a
separate class.

[sky, 2016-03-08 16:46:13]

Ok

On Mon, Mar 7, 2016 at 7:32 PM,  <mfomitchev@chromium.org> wrote:
> On 2016/03/07 23:49:59, sky wrote:
>> I poked at your review a bit, mostly thinking about AppListController.
>> AppListController seems roughly like what I would expect to see.
>>
>> AppListShowerAsh is a bit wordy, Did you consider ChromeAppListController
>> or
>> AshAppListController?
>
> There is a somewhat similar construct for Linux with AppListShower
> (app_list_shower_views.h) and AppListServiceViews.
> We already have AppListControllerDelegateAsh, and it is referred to as just
> "controller" in a bunch of places, so I think having another "controller"
> would
> be confusing. We could just merge the functionality into
> AppListControllerDelegateAsh, but I thought it would be better to keep it in
> a
> separate class.
>
> https://codereview.chromium.org/1770993002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mfomitchev, 2016-03-08 19:21:09]

On 2016/03/08 16:46:13, sky wrote:
> Ok
> 
> On Mon, Mar 7, 2016 at 7:32 PM,  <mailto:mfomitchev@chromium.org> wrote:
> > On 2016/03/07 23:49:59, sky wrote:
> >> I poked at your review a bit, mostly thinking about AppListController.
> >> AppListController seems roughly like what I would expect to see.
> >>
> >> AppListShowerAsh is a bit wordy, Did you consider ChromeAppListController
> >> or
> >> AshAppListController?
> >
> > There is a somewhat similar construct for Linux with AppListShower
> > (app_list_shower_views.h) and AppListServiceViews.
> > We already have AppListControllerDelegateAsh, and it is referred to as just
> > "controller" in a bunch of places, so I think having another "controller"
> > would
> > be confusing. We could just merge the functionality into
> > AppListControllerDelegateAsh, but I thought it would be better to keep it in
> > a
> > separate class.
> >
> > https://codereview.chromium.org/1770993002/
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.
> 

Do you mind taking a look at ui/app_list/app_list_layout_delegate.h and
ash/wm/app_list_layout_delegate.h? 
ash::AppListController was essentially broken up into AppListShowerAsh and
ash::AppListLayoutDelegate. I'd like to confirm that the way I broke it apart
looks reasonable to you. Thanks!

[sky, 2016-03-08 22:49:18]

Where is AppListLayoutDelegate used? I see the ash implementation of
it, but I don't see the code that takes an AppListLayoutDelegate* and
will call into it.

  -Scott

On Tue, Mar 8, 2016 at 11:21 AM,  <mfomitchev@chromium.org> wrote:
> On 2016/03/08 16:46:13, sky wrote:
>> Ok
>>
>> On Mon, Mar 7, 2016 at 7:32 PM, <mailto:mfomitchev@chromium.org> wrote:
>> > On 2016/03/07 23:49:59, sky wrote:
>> >> I poked at your review a bit, mostly thinking about AppListController.
>> >> AppListController seems roughly like what I would expect to see.
>> >>
>> >> AppListShowerAsh is a bit wordy, Did you consider
>> >> ChromeAppListController
>> >> or
>> >> AshAppListController?
>> >
>> > There is a somewhat similar construct for Linux with AppListShower
>> > (app_list_shower_views.h) and AppListServiceViews.
>> > We already have AppListControllerDelegateAsh, and it is referred to as
>> > just
>> > "controller" in a bunch of places, so I think having another
>> > "controller"
>> > would
>> > be confusing. We could just merge the functionality into
>> > AppListControllerDelegateAsh, but I thought it would be better to keep
>> > it in
>> > a
>> > separate class.
>> >
>> > https://codereview.chromium.org/1770993002/
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Chromium-reviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
> email
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> Do you mind taking a look at ui/app_list/app_list_layout_delegate.h and
> ash/wm/app_list_layout_delegate.h?
> ash::AppListController was essentially broken up into AppListShowerAsh and
> ash::AppListLayoutDelegate. I'd like to confirm that the way I broke it
> apart
> looks reasonable to you. Thanks!
>
> https://codereview.chromium.org/1770993002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mfomitchev, 2016-03-08 23:08:36]

On 2016/03/08 22:49:18, sky wrote:
> Where is AppListLayoutDelegate used? I see the ash implementation of
> it, but I don't see the code that takes an AppListLayoutDelegate* and
> will call into it.
 
AppListShowerAsh uses AppListLayoutDelegate:
// Responsible for laying out the app list UI.
scoped_ptr<app_list::AppListLayoutDelegate> layout_delegate_;

[sky, 2016-03-08 23:33:12]

sky@chromium.org changed reviewers:
+ xiyuan@chromium.org

[sky, 2016-03-08 23:38:25]

On 2016/03/08 23:08:36, mfomitchev wrote:
> On 2016/03/08 22:49:18, sky wrote:
> > Where is AppListLayoutDelegate used? I see the ash implementation of
> > it, but I don't see the code that takes an AppListLayoutDelegate* and
> > will call into it.
>  
> AppListShowerAsh uses AppListLayoutDelegate:
> // Responsible for laying out the app list UI.
> scoped_ptr<app_list::AppListLayoutDelegate> layout_delegate_;

Thanks. I searched for AppListLayoutDelegate* and didn't find it.
I would have to study this code more to understand if AppListLayoutDelegate is
the right level of abstraction. My hope is that you could create and show the
widget the same way on all platforms, and platforms that need some level of
customization of the widget could inject customization in appropriate ways
(widget subclass, customization of properties on the widget, WidgetObserver...).
Is that possible? If not, why?

I'm adding xiyuan as a reviewer. He's in your timezone, and owner and familiar
with this code.

[mfomitchev, 2016-03-08 23:57:24]

On 2016/03/08 23:38:25, sky wrote:
> On 2016/03/08 23:08:36, mfomitchev wrote:
> > On 2016/03/08 22:49:18, sky wrote:
> > > Where is AppListLayoutDelegate used? I see the ash implementation of
> > > it, but I don't see the code that takes an AppListLayoutDelegate* and
> > > will call into it.
> >  
> > AppListShowerAsh uses AppListLayoutDelegate:
> > // Responsible for laying out the app list UI.
> > scoped_ptr<app_list::AppListLayoutDelegate> layout_delegate_;
> 
> Thanks. I searched for AppListLayoutDelegate* and didn't find it.
> I would have to study this code more to understand if AppListLayoutDelegate is
> the right level of abstraction. My hope is that you could create and show the
> widget the same way on all platforms, and platforms that need some level of
> customization of the widget could inject customization in appropriate ways
> (widget subclass, customization of properties on the widget,
WidgetObserver...).
> Is that possible? If not, why?
> 
> I'm adding xiyuan as a reviewer. He's in your timezone, and owner and familiar
> with this code.

AppListLayoutDelegate basically contains the code that has dependencies on
various
Ash pieces. For example the code that updates the launcher icon in the shelf,
the
code that listens to various events from the shell (MaximizeMode, shelf
alignment)
that impact the app list, etc.
In Mus+ash we can't just call into ash::Shell from chrome, so I decided to
factor
all that ash stuff out into a separate class, put it behind an interface, and
provide a different implementation in Mus.

[mfomitchev, 2016-03-09 00:31:33]

> AppListLayoutDelegate basically contains the code that has dependencies on
> various
> Ash pieces. For example the code that updates the launcher icon in the shelf,
> the
> code that listens to various events from the shell (MaximizeMode, shelf
> alignment)
> that impact the app list, etc.
> In Mus+ash we can't just call into ash::Shell from chrome, so I decided to
> factor
> all that ash stuff out into a separate class, put it behind an interface, and
> provide a different implementation in Mus.

Your next question might be "what does this have to do with layout?" The
layout is dependent on ash::Shell too (IsMaximizeModeWindowManagerEnabled(),
GetShelfDisplayBoundsInRoot(), etc). The delegate also decides whether to show
the app list as a window or as a bubble.

[sky, 2016-03-09 00:46:55]

Ok, that makes sense. I find the name mildly confusing as the
implication is that AppListLayoutDelegate is a delegate of
AppListLayout, when that isn't the case. There is no AppListLayout.
Seems like this class is intended as the delegate of
AppListController. Did you consider AppListControllerDelegate?

I better understand the split and what you have here makes sense to me.

  -Scott

On Tue, Mar 8, 2016 at 4:31 PM,  <mfomitchev@chromium.org> wrote:
>> AppListLayoutDelegate basically contains the code that has dependencies on
>> various
>> Ash pieces. For example the code that updates the launcher icon in the
>> shelf,
>> the
>> code that listens to various events from the shell (MaximizeMode, shelf
>> alignment)
>> that impact the app list, etc.
>> In Mus+ash we can't just call into ash::Shell from chrome, so I decided to
>> factor
>> all that ash stuff out into a separate class, put it behind an interface,
>> and
>> provide a different implementation in Mus.
>
> Your next question might be "what does this have to do with layout?" The
> layout is dependent on ash::Shell too (IsMaximizeModeWindowManagerEnabled(),
> GetShelfDisplayBoundsInRoot(), etc). The delegate also decides whether to
> show
> the app list as a window or as a bubble.
>
> https://codereview.chromium.org/1770993002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mfomitchev, 2016-03-09 17:57:24]

Yeah, I am not a fan of the name either. Problem is, we already have
AppListControllerDelegate: browser/ui/app_list/app_list_controller_delegate.h,
and we have AppListControllerDelegateAsh, which is one of the classes
affected by this CL.

How about we rename app_list::AppListController to app_list::AppListShower,
and then the delegate could be called app_list::AppListShowerDelegate? 

(It's still not great, because we have AppListShower here: 
chrome/browser/ui/app_list/app_list_shower_views.h. We can rename that to
AppListShowerViews. It won't extend app_list::AppListShower, which is
a bit confusing, but perhaps okay, especially considering that code should
eventually go away.)


On 2016/03/09 00:46:55, sky wrote:
> Ok, that makes sense. I find the name mildly confusing as the
> implication is that AppListLayoutDelegate is a delegate of
> AppListLayout, when that isn't the case. There is no AppListLayout.
> Seems like this class is intended as the delegate of
> AppListController. Did you consider AppListControllerDelegate?

[sky, 2016-03-09 18:35:56]

Fine by me.

On Wed, Mar 9, 2016 at 9:57 AM,  <mfomitchev@chromium.org> wrote:
> Yeah, I am not a fan of the name either. Problem is, we already have
> AppListControllerDelegate:
> browser/ui/app_list/app_list_controller_delegate.h,
> and we have AppListControllerDelegateAsh, which is one of the classes
> affected by this CL.
>
> How about we rename app_list::AppListController to app_list::AppListShower,
> and then the delegate could be called app_list::AppListShowerDelegate?
>
> (It's still not great, because we have AppListShower here:
> chrome/browser/ui/app_list/app_list_shower_views.h. We can rename that to
> AppListShowerViews. It won't extend app_list::AppListShower, which is
> a bit confusing, but perhaps okay, especially considering that code should
> eventually go away.)
>
>
> On 2016/03/09 00:46:55, sky wrote:
>> Ok, that makes sense. I find the name mildly confusing as the
>> implication is that AppListLayoutDelegate is a delegate of
>> AppListLayout, when that isn't the case. There is no AppListLayout.
>> Seems like this class is intended as the delegate of
>> AppListController. Did you consider AppListControllerDelegate?
>
> https://codereview.chromium.org/1770993002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[xiyuan, 2016-03-11 18:36:00]

The design looks reasonable to me. Hopefully, after the app list clean up work,
we have less confusing names.

I did not look very closely since this is WIP. Let me know when I should take
another look.

https://codereview.chromium.org/1770993002/diff/80001/ash/wm/app_list_layout_...
File ash/wm/app_list_layout_delegate.h (right):

https://codereview.chromium.org/1770993002/diff/80001/ash/wm/app_list_layout_...
ash/wm/app_list_layout_delegate.h:42: //
app_list::AppListController::LayoutDelegate:
nit: update

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
File chrome/browser/ui/ash/app_list/app_list_controller_ash.cc (right):

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
chrome/browser/ui/ash/app_list/app_list_controller_ash.cc:24:
ash::Shell::GetInstance()->DismissAppList();
Call app_list_shower_->Dismiss(). It looks like we could get rid of Shell
reference in this file.

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
File chrome/browser/ui/ash/app_list/app_list_service_ash.cc (right):

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
chrome/browser/ui/ash/app_list/app_list_service_ash.cc:37:
base::Unretained(this)));
We should figure out a way to get rid of this PostTask. How about deferring the
work here to AppListServiceAsh::ShowForProfile or
AppListServiceAsh::GetAppListController?

If not possible, then how about using a similar construct as the current
AppListShower/AppListShowerDelegate (old code, not the shower in this CL) and
make the new AppListShower to call into a delegate interface to get
AppListViewDelegate before it needs to create AppListView.

https://codereview.chromium.org/1770993002/diff/80001/ui/app_list/app_list_la...
File ui/app_list/app_list_layout_delegate.h (right):

https://codereview.chromium.org/1770993002/diff/80001/ui/app_list/app_list_la...
ui/app_list/app_list_layout_delegate.h:8: #include
"ui/app_list/app_list_controller.h"
nit: unused?

https://codereview.chromium.org/1770993002/diff/80001/ui/app_list/app_list_la...
ui/app_list/app_list_layout_delegate.h:28: void virtual
Init(app_list::AppListView* view,
nit: "app_list::" not necessary since it is already in the namespace

[mfomitchev, 2016-03-24 17:47:29]

https://codereview.chromium.org/1770993002/diff/80001/ash/wm/app_list_layout_...
File ash/wm/app_list_layout_delegate.h (right):

https://codereview.chromium.org/1770993002/diff/80001/ash/wm/app_list_layout_...
ash/wm/app_list_layout_delegate.h:42: //
app_list::AppListController::LayoutDelegate:
On 2016/03/11 18:36:00, xiyuan wrote:
> nit: update

Done.

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
File chrome/browser/ui/ash/app_list/app_list_controller_ash.cc (right):

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
chrome/browser/ui/ash/app_list/app_list_controller_ash.cc:24:
ash::Shell::GetInstance()->DismissAppList();
On 2016/03/11 18:36:00, xiyuan wrote:
> Call app_list_shower_->Dismiss(). It looks like we could get rid of Shell
> reference in this file.

Done. Shell is still used in one place unfortunately to access metrics
(task_switch_metrics_recorder).

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
File chrome/browser/ui/ash/app_list/app_list_service_ash.cc (right):

https://codereview.chromium.org/1770993002/diff/80001/chrome/browser/ui/ash/a...
chrome/browser/ui/ash/app_list/app_list_service_ash.cc:37:
base::Unretained(this)));
On 2016/03/11 18:36:00, xiyuan wrote:
> We should figure out a way to get rid of this PostTask. How about deferring
the
> work here to AppListServiceAsh::ShowForProfile or
> AppListServiceAsh::GetAppListController?
> 
> If not possible, then how about using a similar construct as the current
> AppListShower/AppListShowerDelegate (old code, not the shower in this CL) and
> make the new AppListShower to call into a delegate interface to get
> AppListViewDelegate before it needs to create AppListView.

Done.

https://codereview.chromium.org/1770993002/diff/80001/ui/app_list/app_list_la...
File ui/app_list/app_list_layout_delegate.h (right):

https://codereview.chromium.org/1770993002/diff/80001/ui/app_list/app_list_la...
ui/app_list/app_list_layout_delegate.h:8: #include
"ui/app_list/app_list_controller.h"
On 2016/03/11 18:36:00, xiyuan wrote:
> nit: unused?

Done.

https://codereview.chromium.org/1770993002/diff/80001/ui/app_list/app_list_la...
ui/app_list/app_list_layout_delegate.h:28: void virtual
Init(app_list::AppListView* view,
On 2016/03/11 18:36:00, xiyuan wrote:
> nit: "app_list::" not necessary since it is already in the namespace

Done.

[mfomitchev, 2016-03-24 17:48:01]

I will break up this CL and land in smaller pieces.

[mfomitchev, 2016-03-24 17:58:58]

Patchset #11 (id:200001) has been deleted