Getting rid of ChromeLauncherController::model().

Getting rid of ChromeLauncherController::model().

Also updating comments/error output:
BrowserLauncherItemController -> BrowserShortcutLauncherItemController.

BUG=NONE

Committed: https://crrev.com/258f2a662ce1832a109bd780048dd07b26907243
Cr-Commit-Position: refs/heads/master@{#397848}

[mfomitchev, 2016-06-03 17:48:12]

mfomitchev@chromium.org changed reviewers:
+ msw@chromium.org

[msw, 2016-06-03 17:55:27]

lgtm

https://codereview.chromium.org/2039543002/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/2039543002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:1704: << "There
should be always a BrowserShortcutLauncherItemController.";
optional nit: "always be"

[mfomitchev, 2016-06-03 18:09:12]

The CQ bit was checked by mfomitchev@chromium.org

[mfomitchev, 2016-06-03 18:09:17]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2039543002/#ps20001
(title: "Addressing nit.")

[commit-bot: I haz the power, 2016-06-03 18:09:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-03 18:09:28]

This CL has an open dependency (Issue 2039543002 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[mfomitchev, 2016-06-03 18:09:45]

https://codereview.chromium.org/2039543002/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/2039543002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:1704: << "There
should be always a BrowserShortcutLauncherItemController.";
On 2016/06/03 17:55:27, msw wrote:
> optional nit: "always be"

Done.

[mfomitchev, 2016-06-03 20:36:18]

msw: Had to change this a bit to make the tests pass. Does it still look ok to
you?

[msw, 2016-06-03 20:54:11]

sure, it's uglier, but still lgtm

https://codereview.chromium.org/2039543002/diff/60001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/extension_launcher_context_menu.cc (right):

https://codereview.chromium.org/2039543002/diff/60001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/extension_launcher_context_menu.cc:88: if
(!BrowserShortcutLauncherItemController(controller(), shelf_model_)
aside: yuck!

[mfomitchev, 2016-06-03 20:57:07]

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

[mfomitchev, 2016-06-03 20:57:07]

sky@chromium.org: Please review changes in 

ash/

[sky, 2016-06-03 22:31:27]

LGTM

[mfomitchev, 2016-06-03 23:48:03]

The CQ bit was checked by mfomitchev@chromium.org

[commit-bot: I haz the power, 2016-06-03 23:48:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2039543002/60001

[commit-bot: I haz the power, 2016-06-03 23:55:18]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-03 23:57:45]

Description was changed from

==========
Getting rid of ChromeLauncherController::model().

Also updating comments/error output:
BrowserLauncherItemController -> BrowserShortcutLauncherItemController.

BUG=NONE
==========

to

==========
Getting rid of ChromeLauncherController::model().

Also updating comments/error output:
BrowserLauncherItemController -> BrowserShortcutLauncherItemController.

BUG=NONE

Committed: https://crrev.com/258f2a662ce1832a109bd780048dd07b26907243
Cr-Commit-Position: refs/heads/master@{#397848}
==========

[commit-bot: I haz the power, 2016-06-03 23:57:46]

Patchset 4 (id:??) landed as
https://crrev.com/258f2a662ce1832a109bd780048dd07b26907243
Cr-Commit-Position: refs/heads/master@{#397848}

[James Cook, 2016-06-04 04:47:57]

jamescook@chromium.org changed reviewers:
+ jamescook@chromium.org

[James Cook, 2016-06-04 04:47:58]

Drive-by, since I just hit a merge conflict on this, and I'm actively working in
the shelf code.

https://codereview.chromium.org/2039543002/diff/60001/ash/shelf/shelf.h
File ash/shelf/shelf.h (right):

https://codereview.chromium.org/2039543002/diff/60001/ash/shelf/shelf.h#newco...
ash/shelf/shelf.h:141: ShelfModel* shelf_model() { return shelf_view_->model();
}
I don't understand why you're exposing this here, rather than calling
ash::Shell::shelf_model(). There is only one model for the shelf, shared among
multiple instances of this class. Doing it this way means every user of this
class directly depends on ShelfView.

If it has to be exposed here for some reason, maybe it could be a non-inline
function GetShelfModel() and we could sort out later whether it should get the
model directly from ash::Shell or inject a pointer or get it from ShelfView.

[mfomitchev, 2016-06-06 15:46:50]

On 2016/06/04 04:47:58, James Cook wrote:
> Drive-by, since I just hit a merge conflict on this, and I'm actively working
in
> the shelf code.
> 
> https://codereview.chromium.org/2039543002/diff/60001/ash/shelf/shelf.h
> File ash/shelf/shelf.h (right):
> 
>
https://codereview.chromium.org/2039543002/diff/60001/ash/shelf/shelf.h#newco...
> ash/shelf/shelf.h:141: ShelfModel* shelf_model() { return
shelf_view_->model();
> }
> I don't understand why you're exposing this here, rather than calling
> ash::Shell::shelf_model(). There is only one model for the shelf, shared among
> multiple instances of this class. Doing it this way means every user of this
> class directly depends on ShelfView.
> 
> If it has to be exposed here for some reason, maybe it could be a non-inline
> function GetShelfModel() and we could sort out later whether it should get the
> model directly from ash::Shell or inject a pointer or get it from ShelfView.

Thanks for the drive by. You are absolutely right, there is no reason to do
this. Unlike ChromeLauncherController, there's no unit tests that injects
model into the shelf.
Fixed in crrev.com/2039253002