Remove PerBrowser launcher

Remove PerBrowser launcher

Remove per-browser.* and related tests.
Remove IsPerAppLauncher() interface in LauncherDelegate.
Remove ash-disable-per-app-launcher switch.
Change class name from ChromeLauncherControllerPerApp to ChromeLauncherController.

In the next CL, BrowserLauncherItemController will be replaced with new browser status monitor only for monitoring browser and tab status.

R=skuhne@chromium.org,jamescook@chromium.org
BUG=169303
TEST=unit_tests, browser_tests

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

[simonhong_, 2013-08-15 15:29:28]

Dear Stefan,
Please take a look.
Thank you.
Simon.

[simonhong_, 2013-08-16 22:12:22]

kindly ping...

[Mr4D (OOO till 08-26), 2013-08-16 22:43:22]

Very good start!

There are more things missing (see comments).

Also - please note in the description that the removal of the
BrowserLauncherItemController will be done with the introduction of the new
state monitor.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc (right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:3: // found
in the LICENSE file.
Note: This entire controller should go. In a separate CL & I know that you have
already a CL created which is a good step towards it. But don't forget!

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:68: } else {
} else if (..) {
}

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:3: // found in the
LICENSE file.
Close...
The "ChromeLauncherControllerPerApp" will become once again
"ChromeLauncherController"!

As such you should remove this file(!), move then
"Chrome_launcher_controller_per_app.cc" to "chrome_launcher_controller.cc" and
rename the class accordingly. Same with the header file.

Note: By moving we keep the history which would otherwise be lost.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
And rename to "chrome_launcher_controller.cc".

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
Again -> Rename to "chrome_launcher_controller.h".

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
Rename to chrome_launcher_controller_browsertest.cc

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:267:
DISALLOW_COPY_AND_ASSIGN(LauncherPerAppAppBrowserTest);
Remove the "PerApp" here.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:272:
class LauncherPerAppAppBrowserTestNoDefaultBrowser
Please remove the "PerApp" from this class (and all the functions below).

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:1646:
IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTest, OverflowBubble) {
How did this get into this CL?

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-> chrome_launcher_controller_unittest.cc

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc:1430:

How did this all end up here?

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc (right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:20: class
TestChromeLauncherControllerPerApp :
Remove the "PerApp" here.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:30:
DISALLOW_COPY_AND_ASSIGN(TestChromeLauncherControllerPerApp);
Remove PerApp.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:46: new
TestChromeLauncherControllerPerApp(profile(),
Remove PerApp & indenting.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc:194:
static_cast<ChromeLauncherControllerPerApp*>(launcher_controller()),
This static_cast should not be needed anymore with the renaming of the
controller class.

[simonhong_, 2013-08-19 05:26:10]

Dear Stefan,
Please check again!

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc (right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:3: // found
in the LICENSE file.
On 2013/08/16 22:43:22, Mr4D wrote:
> Note: This entire controller should go. In a separate CL & I know that you
have
> already a CL created which is a good step towards it. But don't forget!

Yep!
I'll do it right after this CL is landing.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:68: } else {
On 2013/08/16 22:43:22, Mr4D wrote:
> } else if (..) {
> }

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:3: // found in the
LICENSE file.
On 2013/08/16 22:43:22, Mr4D wrote:
> Close...
> The "ChromeLauncherControllerPerApp" will become once again
> "ChromeLauncherController"!
> 
> As such you should remove this file(!), move then
> "Chrome_launcher_controller_per_app.cc" to "chrome_launcher_controller.cc" and
> rename the class accordingly. Same with the header file.
> 
> Note: By moving we keep the history which would otherwise be lost.

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2013/08/16 22:43:22, Mr4D wrote:
> And rename to "chrome_launcher_controller.cc".

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2013/08/16 22:43:22, Mr4D wrote:
> Again -> Rename to "chrome_launcher_controller.h".

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2013/08/16 22:43:22, Mr4D wrote:
> Rename to chrome_launcher_controller_browsertest.cc

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:267:
DISALLOW_COPY_AND_ASSIGN(LauncherPerAppAppBrowserTest);
On 2013/08/16 22:43:22, Mr4D wrote:
> Remove the "PerApp" here.

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:272:
class LauncherPerAppAppBrowserTestNoDefaultBrowser
On 2013/08/16 22:43:22, Mr4D wrote:
> Please remove the "PerApp" from this class (and all the functions below).

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_browsertest.cc:1646:
IN_PROC_BROWSER_TEST_F(LauncherPerAppAppBrowserTest, OverflowBubble) {
On 2013/08/16 22:43:22, Mr4D wrote:
> How did this get into this CL?

This test is moved from (previous)chrome_launcher_controlelr_browsertest.cc
All other tests are duplicated except this test.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2013/08/16 22:43:22, Mr4D wrote:
> -> chrome_launcher_controller_unittest.cc

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app_unittest.cc:1430:

On 2013/08/16 22:43:22, Mr4D wrote:
> How did this all end up here?

Most of unit tests in browser_launcher_item_controller_unittest.cc is related
with tabbed launcher item.
All of them I removed and some other unit tests is moved here and browser test.
* PersistLauncherItemPositions and PersistPinned tests are merged with
ChromeLauncherControllerTest.
* FlashTest (renamed to WindowAttentionTest) is merged with
LauncherPlatformAppBrowserTest in browsertest.cc

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc (right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:20: class
TestChromeLauncherControllerPerApp :
On 2013/08/16 22:43:22, Mr4D wrote:
> Remove the "PerApp" here.

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:30:
DISALLOW_COPY_AND_ASSIGN(TestChromeLauncherControllerPerApp);
On 2013/08/16 22:43:22, Mr4D wrote:
> Remove PerApp.

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:46: new
TestChromeLauncherControllerPerApp(profile(),
On 2013/08/16 22:43:22, Mr4D wrote:
> Remove PerApp & indenting.

Done.

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc
(right):

https://codereview.chromium.org/22887015/diff/4001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/shell_window_launcher_item_controller.cc:194:
static_cast<ChromeLauncherControllerPerApp*>(launcher_controller()),
On 2013/08/16 22:43:22, Mr4D wrote:
> This static_cast should not be needed anymore with the renaming of the
> controller class.

Done.

[Mr4D (OOO till 08-26), 2013-08-19 16:13:56]

Only a few more nits - then we are there!

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc
(left):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
Not quite sure if it was a good idea to remove this unit test while the old
BrowserLauncherItemController is still there. That said - I know that the BLIC
will be history soon.

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:5: #include
"chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
Not quite sure what I am looking at here therefore I'd rather ask:

Did you actually MOVE the files with git so that the history gets preserved?

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
(right):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc:434:
const ash::LauncherItem& item = GetLastLauncherItem();
Can you please make this an item1 again (otherwise the item2 / item3 make only
limited sense).

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc:502: 
This line can go.

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc (right):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:44:
&launcher_model_));
Please one line.

[simonhong_, 2013-08-19 21:00:56]

Dear Stefan,
I addressed your comments.
Please check again!

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc
(left):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller_unittest.cc:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2013/08/19 16:13:56, Mr4D wrote:
> Not quite sure if it was a good idea to remove this unit test while the old
> BrowserLauncherItemController is still there. That said - I know that the BLIC
> will be history soon.

I removed this file with per-browser related tests.
And common tests (not only for per-browser) are moved unittest and browsertest.
(PersistLauncherItemPositions, PersistPinned, FlashWindow)
Or, should I merge another CL(that replacing BLIC) to this CL?

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:5: #include
"chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
On 2013/08/19 16:13:56, Mr4D wrote:
> Not quite sure what I am looking at here therefore I'd rather ask:
> 
> Did you actually MOVE the files with git so that the history gets preserved?

Yes, I did below steps.
1) git rm chrome_launcher_controller.[h|cc]
2) git mv chrome_launcher_controller_per_app.[h|cc]
chrome_launcher_controller.[h|cc]

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
(right):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc:434:
const ash::LauncherItem& item = GetLastLauncherItem();
On 2013/08/19 16:13:56, Mr4D wrote:
> Can you please make this an item1 again (otherwise the item2 / item3 make only
> limited sense).

Done.

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc:502: 
On 2013/08/19 16:13:56, Mr4D wrote:
> This line can go.

Done.

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc (right):

https://codereview.chromium.org/22887015/diff/22001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc:44:
&launcher_model_));
On 2013/08/19 16:13:56, Mr4D wrote:
> Please one line.

Done.

[Mr4D (OOO till 08-26), 2013-08-19 21:32:53]

lgtm

[simonhong_, 2013-08-19 21:35:26]

Dear jamescook,
I need owners check in ash/

[James Cook, 2013-08-19 22:32:38]

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc (right):

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:247: return;
nit: no "return" needed

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:251: return;
nit: no "return" needed

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.h (right):

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.h:67: // content
windows. Launcher items have a type, an optional app id, and a
nit: "content content" - remove one word :-)

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.h:119: virtual void
Init() OVERRIDE;
Do you need virtual?  OVERRIDE?  I suspect you do not, and there are also
probably lots of places below where you don't need them.

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/launcher_item_controller.h (right):

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/launcher_item_controller.h:48:
ChromeLauncherController* launcher_controller() const {
we don't usually use "const" for accessor functions like this that return
pointers, since the caller can modify the data it points to

[simonhong_, 2013-08-20 01:11:19]

Dear jamescook,
I removed useless virtual & OVERRIDE.
Also I added a const version of launcher_controller().
Please check below messages!

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc (right):

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:247: return;
On 2013/08/19 22:32:38, James Cook wrote:
> nit: no "return" needed

Done.

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/browser_launcher_item_controller.cc:251: return;
On 2013/08/19 22:32:38, James Cook wrote:
> nit: no "return" needed

Done.

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.h (right):

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.h:67: // content
windows. Launcher items have a type, an optional app id, and a
On 2013/08/19 22:32:38, James Cook wrote:
> nit: "content content" - remove one word :-)

Done.

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.h:119: virtual void
Init() OVERRIDE;
On 2013/08/19 22:32:38, James Cook wrote:
> Do you need virtual?  OVERRIDE?  I suspect you do not, and there are also
> probably lots of places below where you don't need them.

Yes, there are so many unwanted virtual & OVERRIDE.
Done.

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
File chrome/browser/ui/ash/launcher/launcher_item_controller.h (right):

https://codereview.chromium.org/22887015/diff/31001/chrome/browser/ui/ash/lau...
chrome/browser/ui/ash/launcher/launcher_item_controller.h:48:
ChromeLauncherController* launcher_controller() const {
On 2013/08/19 22:32:38, James Cook wrote:
> we don't usually use "const" for accessor functions like this that return
> pointers, since the caller can modify the data it points to

How about adding a const version of launcher_controller()?
This function is needed in some const functions.

[simonhong_, 2013-08-20 02:02:16]

Instead of using launcher_controller(), chrome_launcher_controller_  is added to
access in const function of AppShortcutLauncherItemController.

[James Cook, 2013-08-20 16:42:22]

LGTM. Thanks for cleaning this up!

[commit-bot: I haz the power, 2013-08-20 17:34:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simon.hong81@gmail.com/22887015/46001

[commit-bot: I haz the power, 2013-08-20 20:58:57]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
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.

[simonhong_, 2013-08-20 21:24:21]

Dear jamescook,
I changed ChromeLauncherController::IsLoggedInAsGuest() to virtual function.
LauncherContextMenuTest overrides it for test.
Please check again.

[James Cook, 2013-08-20 21:26:03]

lgtm

[commit-bot: I haz the power, 2013-08-20 21:31:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simon.hong81@gmail.com/22887015/76001

[commit-bot: I haz the power, 2013-08-21 00:38:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simon.hong81@gmail.com/22887015/82001

[commit-bot: I haz the power, 2013-08-21 08:14:20]

Change committed as 218674

[James Cook, 2013-08-21 20:03:27]

This change lost the revision history of chrome_launcher_controller_per_app.h/cc
due to a delete and rename on top of each other. I reverted it with
https://src.chromium.org/viewvc/chrome?view=rev&revision=218808

I'll re-land the parts separately

[simonhong_, 2013-08-21 20:19:43]

On 2013/08/21 20:03:27, James Cook wrote:
> This change lost the revision history of
chrome_launcher_controller_per_app.h/cc
> due to a delete and rename on top of each other. I reverted it with
> https://src.chromium.org/viewvc/chrome?view=rev&revision=218808
> 
> I'll re-land the parts separately

Thank you, James.

[James Cook, 2013-08-21 21:26:41]

On 2013/08/21 20:03:27, James Cook wrote:
> This change lost the revision history of
chrome_launcher_controller_per_app.h/cc
> due to a delete and rename on top of each other. I reverted it with
> https://src.chromium.org/viewvc/chrome?view=rev&revision=218808
> 
> I'll re-land the parts separately

Reland part 1: https://codereview.chromium.org/23068021/