Move AppListModel::Users to AppListViewDelegate

Move AppListModel::Users to AppListViewDelegate

This is some minor re-factoring to simplify AppListModel in preparation
for making it associated with a Profile for syncing. While not strictly
necessary, it is confusing to have a list of "Users" associated with a
model that describes the list of apps for a specific user. Also, the
list of Users is only used by AppListMenu and can be maintained by
AppListViewDelegate.

One side effect of this change is that views will not be notified if the
user list changes and therefore can not invalidate the menu. While this
could be addressed, it shoudln't generally be possible to add a user
while the app list UI is visible, so the added complexity did not seem
worth supporting a theoretical edge case.

BUG=315887
For ash/shell/app_list.cc:
TBR=jamescook@chromium.org

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

[xiyuan, 2013-11-06 23:35:06]

While we are at this, should we start to consider http://crbug.com/272321?

I think the reason we had User in AppListModel is to feed it to AppListMenu. The
reason to have AppListMenu inside ui/app_list is because views menu did not
support sub label at the time we add the menu. Since menu sub label is supported
now, we could replace AppListMenu with a
AppListDelegateView::GetAppListMenuModel, and make code simpler.

[stevenjb, 2013-11-06 23:41:34]

On 2013/11/06 23:35:06, xiyuan wrote:
> While we are at this, should we start to consider http://crbug.com/272321?
> 
> I think the reason we had User in AppListModel is to feed it to AppListMenu.
The
> reason to have AppListMenu inside ui/app_list is because views menu did not
> support sub label at the time we add the menu. Since menu sub label is
supported
> now, we could replace AppListMenu with a
> AppListDelegateView::GetAppListMenuModel, and make code simpler.

That *sounds* reasonable, but do you think we should do that first, or is this a
helpful step in the right direction? I'm kind of focused on the model / sync
side of things right now, which is proving complicated enough...

[xiyuan, 2013-11-06 23:49:02]

On 2013/11/06 23:41:34, stevenjb wrote:
> On 2013/11/06 23:35:06, xiyuan wrote:
> > While we are at this, should we start to consider http://crbug.com/272321?
> > 
> > I think the reason we had User in AppListModel is to feed it to AppListMenu.
> The
> > reason to have AppListMenu inside ui/app_list is because views menu did not
> > support sub label at the time we add the menu. Since menu sub label is
> supported
> > now, we could replace AppListMenu with a
> > AppListDelegateView::GetAppListMenuModel, and make code simpler.
> 
> That *sounds* reasonable, but do you think we should do that first, or is this
a
> helpful step in the right direction? I'm kind of focused on the model / sync
> side of things right now, which is proving complicated enough...

Nope. Sync/model change is much more important.

[xiyuan, 2013-11-06 23:49:28]

LGTM

https://codereview.chromium.org/63283003/diff/40001/ui/app_list/test/app_list...
File ui/app_list/test/app_list_test_view_delegate.h (right):

https://codereview.chromium.org/63283003/diff/40001/ui/app_list/test/app_list...
ui/app_list/test/app_list_test_view_delegate.h:59: 
nit: add DISALLOW_COPY_AND_ASSIGN?

[jennyz, 2013-11-07 00:00:36]

lgtm

[commit-bot: I haz the power, 2013-11-08 17:36:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/63283003/280001

[commit-bot: I haz the power, 2013-11-08 17:55:26]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-11-08 18:34:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/63283003/280001

[commit-bot: I haz the power, 2013-11-08 21:27:25]

Change committed as 233996