Elim ActivateAppListItem, ChromeAppListItem

Elim ActivateAppListItem, ChromeAppListItem

This should simplify the view/model structure some by having the View
call ItemModel->Activate() directly, rather than going through the
delegate (which continues to manage the model and handle non item
specific events). This will allow us to create folder items as part
of the non Chrome specific model implementation, without having to
make AppListViewDelegate a concrete class, or adding special folder
handling to the view classes.

BUG=303839
For ash/shell:
TBR=jamescook@chromium.org

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

[stevenjb, 2013-10-03 20:12:15]

Patchset 1 should be relatively straightforward, with an ugly cast in
AppsModelBuilder::GetAppAt (which was there before).

Patchest 2 adds AppType and makes AppsModelBuilder less ExtensionAppItem
specific. We might want to hold off on this for now (I think we need to
generalize how app list items are sorted), but I thought it a useful example of
where this is heading and how we might identify item types.

Please comment freely.

[jennyz, 2013-10-03 20:40:27]

https://codereview.chromium.org/25859005/diff/3001/chrome/browser/ui/app_list...
File chrome/browser/ui/app_list/apps_model_builder.h (right):

https://codereview.chromium.org/25859005/diff/3001/chrome/browser/ui/app_list...
chrome/browser/ui/app_list/apps_model_builder.h:78: void AddApps(const
ExtensionSet* extensions, ExtensionApps* apps);
Should this be renamed as AddExtensionApps?

[xiyuan, 2013-10-03 20:42:22]

PS#1 looks good.

In PS#2, adding AppType() and Update() to AppListItemModel is probably not
necessary. Let's create a new model builder instead of trying to make old
AppsModelBuilder happy.

[jennyz, 2013-10-03 20:45:50]

https://codereview.chromium.org/25859005/diff/3001/chrome/browser/ui/app_list...
File chrome/browser/ui/app_list/apps_model_builder.cc (right):

https://codereview.chromium.org/25859005/diff/3001/chrome/browser/ui/app_list...
chrome/browser/ui/app_list/apps_model_builder.cc:35: return true;  //
ExtensionAppItem preceeds non-ExtensionAppItem
Do you mean we should pass null for non-ExtensionAppItem for sorting?

[stevenjb, 2013-10-03 23:33:41]

Per feedback, I reverted patchset #2 and will address the static cast in
AppsModelBuilder separately with a new sorting model that is independent of the
extension app order (which will only be used for initial ordering).

[jennyz, 2013-10-04 00:37:46]

lgtm

[xiyuan, 2013-10-04 00:59:53]

LGTM

[koz (OOO until 15th September), 2013-10-04 01:42:17]

lgtm

[benwells, 2013-10-04 04:23:09]

On 2013/10/04 01:42:17, koz wrote:
> lgtm

stamp lgtm

[commit-bot: I haz the power, 2013-10-04 16:38:38]

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

[commit-bot: I haz the power, 2013-10-04 17:12:49]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-04 18:42:54]

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

[commit-bot: I haz the power, 2013-10-04 22:04:03]

Change committed as 227108