[win] Change app launcher profile indicator to be a menu.

[win] Change app launcher profile indicator to be a menu.

The new menu shows the currently signed in user, as well as providing
menu items to show the settings app or open a feedback form.

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

[benwells, 2013-03-20 06:44:53]

tapted: please sanity check before I send wider.

[tapted, 2013-03-20 07:44:20]

initial batch

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/resources/s...
File chrome/browser/resources/settings_app/manifest.json (right):

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/resources/s...
chrome/browser/resources/settings_app/manifest.json:24:
"display_in_new_tab_page": false
display_in_new_tab_page: false is implied in this case, maybe better to remove
it to be consistent with cloud print / wallpaper manager?

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

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/ui/app_list...
chrome/browser/ui/app_list/app_list_view_delegate.cc:136: profile_,
chrome::GetActiveDesktop());
Instead of GetActiveDesktop, I think this could be
GetHostDesktopTypeForNativeWindow(controller_->GetAppListWindow())  ref:
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/QR6fbWhGcek/eWm6A...

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/ui/app_list...
File chrome/browser/ui/app_list/search_builder.cc (left):

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/ui/app_list...
chrome/browser/ui/app_list/search_builder.cc:317:
GetImageSkiaNamed(IDR_APP_LIST_USER_INDICATOR));
should this be removed from ui_resources.grd (and git rm the binary?)

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu.cc
File ui/app_list/app_list_menu.cc (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.cc:22: menu_model_.AddItem(CURRENT_USER, L"");
L"" -> string16()

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.cc:41: int command_id,
nit: indenting is off

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.cc:49: break;  // Do nothing
nit: full-stop at end of comment.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu.h
File ui/app_list/app_list_menu.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:14: class AppListMenu :
ui::SimpleMenuModel::Delegate {
Should probably have a class comment

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:14: class AppListMenu :
ui::SimpleMenuModel::Delegate {
Should this be AppListMenuModel ? It also seems common to use multiple
inheritance. E.g.

class CornerSelectionMenuModel : public ui::SimpleMenuModel,
                                 public ui::SimpleMenuModel::Delegate {

+ a ~dozen others

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:17: CURRENT_USER,
It seems common to start these enums from 1 (small sample size, from `git grep
-A5 'enum.*MenuCommands'`

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:23: virtual ~AppListMenu() {}
non-trivial destructor to .cc?

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:25: protected:
private from here?

(I think non-private data members are suggested only for test harnesses (ref:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Access_Control )

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
File ui/app_list/views/app_list_menu_views.cc (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:7: #include
"ui/app_list/views/app_list_menu_views.h"
should be first include, for own header.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:24: explicit CurrentUserView(const
string16& user_name,
I think it's uncommon to use explicit when there are multiple args (?) More
below.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:115: }
nit:  // namespace

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
File ui/app_list/views/app_list_menu_views.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.h:23: class AppListMenuViews :
app_list::AppListMenu {
class comment?

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.h:23: class AppListMenuViews :
app_list::AppListMenu {
AppListMenuViews -> AppListMenuView

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.h:26: virtual ~AppListMenuViews() {}
non-trivial destructor -> .cc

[tapted, 2013-03-20 08:17:56]

a few more minor things, but seems good overall

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/ui/app_list...
File chrome/browser/ui/app_list/search_builder.cc (left):

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/ui/app_list...
chrome/browser/ui/app_list/search_builder.cc:317:
GetImageSkiaNamed(IDR_APP_LIST_USER_INDICATOR));
On 2013/03/20 07:44:20, tapted wrote:
> should this be removed from ui_resources.grd (and git rm the binary?)

don't worry - found it in batch 2 ;)

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_view_...
File ui/app_list/app_list_view_delegate.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_view_...
ui/app_list/app_list_view_delegate.h:22: class APP_LIST_EXPORT
AppListViewDelegate {
note: I think you will need to update ash/shell/app_list.cc:class
ExampleAppListViewDelegate and
ui/app_list/test/app_list_test_view_delegate.h:AppListTestViewDelegate

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
File ui/app_list/views/search_box_view.cc (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.cc:46: menu_button_ = new
views::MenuButton(NULL, L"", this, false);
L"" -> string16()

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.cc:47: menu_button_->set_border(NULL);
hm - this creates a border and then removes it, doesn't look like there's any
way to prevent that though

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
File ui/app_list/views/search_box_view.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.h:37: explicit
SearchBoxView(SearchBoxViewDelegate* delegate,
explicit on multi-arg constructor

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.h:69: View* source, const gfx::Point&
point)OVERRIDE;
nit: space before OVERRIDE

and maybe align with the opening paren, to be consistent with member fns above

[tapted, 2013-03-20 08:24:34]

sorry - forgot a file - I think that's everything..

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
File ui/app_list/views/app_list_menu_views.cc (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:91: delegate_(delegate) {};
stray semicolon

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:92: virtual ~AppListMenuModelAdapter()
{};
stray semicolon

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:94: // Overridden from
views::MenuModelAdapter.
nit: I think these comments usually end in a colon rather than full stop

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:96: if (id ==
AppListMenu::CURRENT_USER) {
maybe early return if id != CURRENT_USER ?

https://codereview.chromium.org/12789010/diff/3001/ui/views/controls/menu/men...
File ui/views/controls/menu/menu_model_adapter.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/views/controls/menu/men...
ui/views/controls/menu/menu_model_adapter.h:42: // Subclasses override this to
allow custom items to be added to the menu.
Does this need a comment about ownership, since it returns a raw pointer?

[benwells, 2013-03-20 08:43:34]

sky: for changes to menu_model_adapter
xiyuan: for the rest

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/resources/s...
File chrome/browser/resources/settings_app/manifest.json (right):

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/resources/s...
chrome/browser/resources/settings_app/manifest.json:24:
"display_in_new_tab_page": false
On 2013/03/20 07:44:20, tapted wrote:
> display_in_new_tab_page: false is implied in this case, maybe better to remove
> it to be consistent with cloud print / wallpaper manager?

Done.

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

https://codereview.chromium.org/12789010/diff/3001/chrome/browser/ui/app_list...
chrome/browser/ui/app_list/app_list_view_delegate.cc:136: profile_,
chrome::GetActiveDesktop());
On 2013/03/20 07:44:20, tapted wrote:
> Instead of GetActiveDesktop, I think this could be
> GetHostDesktopTypeForNativeWindow(controller_->GetAppListWindow())  ref:
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/QR6fbWhGcek/eWm6A...

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu.cc
File ui/app_list/app_list_menu.cc (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.cc:22: menu_model_.AddItem(CURRENT_USER, L"");
On 2013/03/20 07:44:20, tapted wrote:
> L"" -> string16()

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.cc:41: int command_id,
On 2013/03/20 07:44:20, tapted wrote:
> nit: indenting is off

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.cc:49: break;  // Do nothing
On 2013/03/20 07:44:20, tapted wrote:
> nit: full-stop at end of comment.

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu.h
File ui/app_list/app_list_menu.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:14: class AppListMenu :
ui::SimpleMenuModel::Delegate {
On 2013/03/20 07:44:20, tapted wrote:
> Should this be AppListMenuModel ? It also seems common to use multiple
> inheritance. E.g.
> 
> class CornerSelectionMenuModel : public ui::SimpleMenuModel,
>                                  public ui::SimpleMenuModel::Delegate {
> 
> + a ~dozen others

As I don't need to override anything in SimpleMenuModel this feels better.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:14: class AppListMenu :
ui::SimpleMenuModel::Delegate {
On 2013/03/20 07:44:20, tapted wrote:
> Should probably have a class comment

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:17: CURRENT_USER,
On 2013/03/20 07:44:20, tapted wrote:
> It seems common to start these enums from 1 (small sample size, from `git grep
> -A5 'enum.*MenuCommands'`

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:23: virtual ~AppListMenu() {}
On 2013/03/20 07:44:20, tapted wrote:
> non-trivial destructor to .cc?

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_menu....
ui/app_list/app_list_menu.h:25: protected:
On 2013/03/20 07:44:20, tapted wrote:
> private from here?
> 
> (I think non-private data members are suggested only for test harnesses (ref:
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Access_Control
)

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_view_...
File ui/app_list/app_list_view_delegate.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/app_list_view_...
ui/app_list/app_list_view_delegate.h:22: class APP_LIST_EXPORT
AppListViewDelegate {
On 2013/03/20 08:17:56, tapted wrote:
> note: I think you will need to update ash/shell/app_list.cc:class
> ExampleAppListViewDelegate and
> ui/app_list/test/app_list_test_view_delegate.h:AppListTestViewDelegate

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
File ui/app_list/views/app_list_menu_views.cc (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:7: #include
"ui/app_list/views/app_list_menu_views.h"
On 2013/03/20 07:44:20, tapted wrote:
> should be first include, for own header.

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:24: explicit CurrentUserView(const
string16& user_name,
On 2013/03/20 07:44:20, tapted wrote:
> I think it's uncommon to use explicit when there are multiple args (?) More
> below.

Yeap was a relic from an earlier time.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:91: delegate_(delegate) {};
On 2013/03/20 08:24:34, tapted wrote:
> stray semicolon

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:92: virtual ~AppListMenuModelAdapter()
{};
On 2013/03/20 08:24:34, tapted wrote:
> stray semicolon

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:94: // Overridden from
views::MenuModelAdapter.
On 2013/03/20 08:24:34, tapted wrote:
> nit: I think these comments usually end in a colon rather than full stop

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:96: if (id ==
AppListMenu::CURRENT_USER) {
On 2013/03/20 08:24:34, tapted wrote:
> maybe early return if id != CURRENT_USER ?

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.cc:115: }
On 2013/03/20 07:44:20, tapted wrote:
> nit:  // namespace

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
File ui/app_list/views/app_list_menu_views.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.h:23: class AppListMenuViews :
app_list::AppListMenu {
On 2013/03/20 07:44:20, tapted wrote:
> class comment?

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.h:23: class AppListMenuViews :
app_list::AppListMenu {
On 2013/03/20 07:44:20, tapted wrote:
> AppListMenuViews -> AppListMenuView 

It is the Views implementation of the AppListMenu.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/app_list...
ui/app_list/views/app_list_menu_views.h:26: virtual ~AppListMenuViews() {}
On 2013/03/20 07:44:20, tapted wrote:
> non-trivial destructor -> .cc

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
File ui/app_list/views/search_box_view.cc (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.cc:46: menu_button_ = new
views::MenuButton(NULL, L"", this, false);
On 2013/03/20 08:17:56, tapted wrote:
> L"" -> string16()

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.cc:47: menu_button_->set_border(NULL);
On 2013/03/20 08:17:56, tapted wrote:
> hm - this creates a border and then removes it, doesn't look like there's any
> way to prevent that though

Yep. It's like this in toolbar_view.cc.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
File ui/app_list/views/search_box_view.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.h:37: explicit
SearchBoxView(SearchBoxViewDelegate* delegate,
On 2013/03/20 08:17:56, tapted wrote:
> explicit on multi-arg constructor

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/app_list/views/search_b...
ui/app_list/views/search_box_view.h:69: View* source, const gfx::Point&
point)OVERRIDE;
On 2013/03/20 08:17:56, tapted wrote:
> nit: space before OVERRIDE
> 
> and maybe align with the opening paren, to be consistent with member fns above

Done.

https://codereview.chromium.org/12789010/diff/3001/ui/views/controls/menu/men...
File ui/views/controls/menu/menu_model_adapter.h (right):

https://codereview.chromium.org/12789010/diff/3001/ui/views/controls/menu/men...
ui/views/controls/menu/menu_model_adapter.h:42: // Subclasses override this to
allow custom items to be added to the menu.
On 2013/03/20 08:24:34, tapted wrote:
> Does this need a comment about ownership, since it returns a raw pointer?

Done.

[tapted, 2013-03-20 10:39:57]

lgtm for general review

https://codereview.chromium.org/12789010/diff/17002/ui/app_list/views/app_lis...
File ui/app_list/views/app_list_menu_views.h (right):

https://codereview.chromium.org/12789010/diff/17002/ui/app_list/views/app_lis...
ui/app_list/views/app_list_menu_views.h:24: class AppListMenuViews :
app_list::AppListMenu {
public inheritance

[sky, 2013-03-20 14:35:09]

https://codereview.chromium.org/12789010/diff/17002/ui/views/controls/menu/me...
File ui/views/controls/menu/menu_model_adapter.cc (right):

https://codereview.chromium.org/12789010/diff/17002/ui/views/controls/menu/me...
ui/views/controls/menu/menu_model_adapter.cc:190: MenuItemView* item =
CreateCustomItem(menu, model->GetCommandIdAt(i));
Can we rename this CreateMenuItem and have it do the creation? If a subclass
wants other logic it need only override it.

[xiyuan, 2013-03-20 17:20:07]

AppListMenu and AppListMenuViews are too chrome specific. IMHO, the only thing
needed to be in AppList is to invoke the menu. That is, let AppListViewDelegate
have a RunAppListMenu call and do all the work in chrome side. Make sense?

[benwells, 2013-03-20 23:08:30]

sky: please have another look

xiyuan: still thinking about your suggestion, but wanted to get this up to
hopefully get a review from sky before mtv goes to sleep.

https://codereview.chromium.org/12789010/diff/17002/ui/app_list/views/app_lis...
File ui/app_list/views/app_list_menu_views.h (right):

https://codereview.chromium.org/12789010/diff/17002/ui/app_list/views/app_lis...
ui/app_list/views/app_list_menu_views.h:24: class AppListMenuViews :
app_list::AppListMenu {
On 2013/03/20 10:39:57, tapted wrote:
> public inheritance

Done.

https://codereview.chromium.org/12789010/diff/17002/ui/views/controls/menu/me...
File ui/views/controls/menu/menu_model_adapter.cc (right):

https://codereview.chromium.org/12789010/diff/17002/ui/views/controls/menu/me...
ui/views/controls/menu/menu_model_adapter.cc:190: MenuItemView* item =
CreateCustomItem(menu, model->GetCommandIdAt(i));
On 2013/03/20 14:35:09, sky wrote:
> Can we rename this CreateMenuItem and have it do the creation? If a subclass
> wants other logic it need only override it.

Good idea, renamed to AppendMenuItem, which simplifies things nicely.

[xiyuan, 2013-03-20 23:23:21]

LGTM

Please put a TODO to move AppListMenu and AppListMenuViews to chrome. I would
like to track it and make sure a conscious decision being made on it. We could
remove the TODO later if we decide not to do it.

https://codereview.chromium.org/12789010/diff/30001/ui/app_list/views/app_lis...
File ui/app_list/views/app_list_menu_views.cc (right):

https://codereview.chromium.org/12789010/diff/30001/ui/app_list/views/app_lis...
ui/app_list/views/app_list_menu_views.cc:136:
menu_runner_->RunMenuAt(button->GetWidget(), button,
nit: wrap the call with ignore_result(...) beacuse RunMenuAt has
WARN_UNUSED_RESULT.

[sky, 2013-03-20 23:37:28]

MenuModelAdapter changes LGTM

[commit-bot: I haz the power, 2013-03-21 04:25:59]

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

[commit-bot: I haz the power, 2013-03-21 05:00:58]

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-03-21 06:15:52]

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

[commit-bot: I haz the power, 2013-03-21 11:19:24]

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

[commit-bot: I haz the power, 2013-03-21 19:49:36]

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

[commit-bot: I haz the power, 2013-03-21 19:49:45]

Failed to apply patch for chrome/browser/ui/app_list/app_list_view_delegate.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/app_list/app_list_view_delegate.cc
  Hunk #1 FAILED at 4.
  Hunk #2 succeeded at 104 (offset 3 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/ui/app_list/app_list_view_delegate.cc.rej

Patch:       chrome/browser/ui/app_list/app_list_view_delegate.cc
Index: chrome/browser/ui/app_list/app_list_view_delegate.cc
diff --git a/chrome/browser/ui/app_list/app_list_view_delegate.cc
b/chrome/browser/ui/app_list/app_list_view_delegate.cc
index
b4631aa62a20a2af0221c747d238bf6be116c1c2..0f165e79436a9f3c686fd02a12a90f58f3232c41
100644
--- a/chrome/browser/ui/app_list/app_list_view_delegate.cc
+++ b/chrome/browser/ui/app_list/app_list_view_delegate.cc
@@ -4,12 +4,18 @@
 
 #include "chrome/browser/ui/app_list/app_list_view_delegate.h"
 
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/apps_model_builder.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 #include "chrome/browser/ui/app_list/chrome_signin_delegate.h"
 #include "chrome/browser/ui/app_list/search_builder.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/host_desktop.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "content/public/browser/user_metrics.h"
 
 #if defined(USE_ASH)
@@ -101,3 +107,34 @@ void AppListViewDelegate::ViewActivationChanged(bool
active) {
 gfx::ImageSkia AppListViewDelegate::GetWindowIcon() {
   return controller_->GetWindowIcon();
 }
+
+string16 AppListViewDelegate::GetCurrentUserName() {
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+  int profile_index = cache.GetIndexOfProfileWithPath(profile_->GetPath());
+  return cache.GetNameOfProfileAtIndex(profile_index);
+}
+
+string16 AppListViewDelegate::GetCurrentUserEmail()  {
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+  int profile_index = cache.GetIndexOfProfileWithPath(profile_->GetPath());
+  return cache.GetUserNameOfProfileAtIndex(profile_index);
+}
+
+void AppListViewDelegate::OpenSettings() {
+  ExtensionService* service = profile_->GetExtensionService();
+  DCHECK(service);
+  const extensions::Extension* extension = service->GetInstalledExtension(
+      extension_misc::kSettingsAppId);
+  DCHECK(extension);
+  controller_->ActivateApp(profile_, extension, 0);
+}
+
+void AppListViewDelegate::OpenFeedback() {
+  chrome::HostDesktopType desktop = chrome::GetHostDesktopTypeForNativeWindow(
+      controller_->GetAppListWindow());
+  Browser* browser = chrome::FindOrCreateTabbedBrowser(
+      profile_, desktop);
+  chrome::ShowFeedbackPage(browser, std::string(), std::string());
+}

[commit-bot: I haz the power, 2013-03-21 23:08:22]

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

[commit-bot: I haz the power, 2013-03-22 02:31:37]

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

[commit-bot: I haz the power, 2013-03-22 18:42:39]

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

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

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

[commit-bot: I haz the power, 2013-03-24 22:45:13]

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

[commit-bot: I haz the power, 2013-03-24 22:45:20]

Failed to apply patch for chrome/browser/ui/app_list/app_list_view_delegate.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/app_list/app_list_view_delegate.cc
  Hunk #1 FAILED at 4.
  Hunk #2 succeeded at 101 (offset -3 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/ui/app_list/app_list_view_delegate.cc.rej

Patch:       chrome/browser/ui/app_list/app_list_view_delegate.cc
Index: chrome/browser/ui/app_list/app_list_view_delegate.cc
diff --git a/chrome/browser/ui/app_list/app_list_view_delegate.cc
b/chrome/browser/ui/app_list/app_list_view_delegate.cc
index
2c1902ea5d9884bb197aa770870311dee1d1497c..0ec2b4832d785fc3de08c8e39b931efa28bff40f
100644
--- a/chrome/browser/ui/app_list/app_list_view_delegate.cc
+++ b/chrome/browser/ui/app_list/app_list_view_delegate.cc
@@ -4,13 +4,19 @@
 
 #include "chrome/browser/ui/app_list/app_list_view_delegate.h"
 
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/apps_model_builder.h"
 #include "chrome/browser/ui/app_list/chrome_app_list_item.h"
 #include "chrome/browser/ui/app_list/chrome_signin_delegate.h"
 #include "chrome/browser/ui/app_list/search_builder.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/webui/ntp/app_launcher_handler.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "content/public/browser/user_metrics.h"
 
 #if defined(USE_ASH)
@@ -104,3 +110,34 @@ void AppListViewDelegate::ViewActivationChanged(bool
active) {
 gfx::ImageSkia AppListViewDelegate::GetWindowIcon() {
   return controller_->GetWindowIcon();
 }
+
+string16 AppListViewDelegate::GetCurrentUserName() {
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+  int profile_index = cache.GetIndexOfProfileWithPath(profile_->GetPath());
+  return cache.GetNameOfProfileAtIndex(profile_index);
+}
+
+string16 AppListViewDelegate::GetCurrentUserEmail()  {
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+  int profile_index = cache.GetIndexOfProfileWithPath(profile_->GetPath());
+  return cache.GetUserNameOfProfileAtIndex(profile_index);
+}
+
+void AppListViewDelegate::OpenSettings() {
+  ExtensionService* service = profile_->GetExtensionService();
+  DCHECK(service);
+  const extensions::Extension* extension = service->GetInstalledExtension(
+      extension_misc::kSettingsAppId);
+  DCHECK(extension);
+  controller_->ActivateApp(profile_, extension, 0);
+}
+
+void AppListViewDelegate::OpenFeedback() {
+  chrome::HostDesktopType desktop = chrome::GetHostDesktopTypeForNativeWindow(
+      controller_->GetAppListWindow());
+  Browser* browser = chrome::FindOrCreateTabbedBrowser(
+      profile_, desktop);
+  chrome::ShowFeedbackPage(browser, std::string(), std::string());
+}

[commit-bot: I haz the power, 2013-03-24 23:44:00]

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

[commit-bot: I haz the power, 2013-03-25 06:09:44]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-03-25 10:42:25]

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

[commit-bot: I haz the power, 2013-03-25 11:09:00]

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-03-25 16:03:26]

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

[commit-bot: I haz the power, 2013-03-25 18:03:30]

Change committed as 190434