mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Use ChromeLauncherControllerImpl in mash with a fake ShelfModel.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior and bad interaction in mash.

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2835063003
Cr-Commit-Position: refs/heads/master@{#467228}
Committed: https://chromium.googlesource.com/chromium/src/+/6fd672f43f11c93d78c93f39c9d0fb9a50d98375

[msw, 2017-04-25 00:34:14]

Description was changed from

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.



BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=sky@chromium.org
==========

to

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior in mash.
TODO:

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=sky@chromium.org
==========

[msw, 2017-04-25 01:49:33]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 01:50:01]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2017-04-25 01:50:16]

Description was changed from

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior in mash.
TODO:

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=sky@chromium.org
==========

to

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior and bad interaction in mash.

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=jamescook@chromium.org
==========

[msw, 2017-04-25 01:50:17]

msw@chromium.org changed reviewers:
+ jamescook@chromium.org
- sky@chromium.org

[msw, 2017-04-25 01:50:35]

Description was changed from

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior and bad interaction in mash.

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=jamescook@chromium.org
==========

to

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Use ChromeLauncherControllerImpl in mash with a fake ShelfModel.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior and bad interaction in mash.

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=jamescook@chromium.org
==========

[commit-bot: I haz the power, 2017-04-25 03:56:01]

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

[commit-bot: I haz the power, 2017-04-25 03:56:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msw, 2017-04-25 22:05:12]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 22:06:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2017-04-25 22:37:29]

Hey James, please take a look; thanks! Note: if this still has any
mash_browser_test failures, hopefully they'll be easy to address.

[commit-bot: I haz the power, 2017-04-26 00:08:50]

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

[commit-bot: I haz the power, 2017-04-26 00:08:51]

Dry run: This issue passed the CQ dry run.

[James Cook, 2017-04-26 01:03:26]

Code looks good but one question.

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h (right):

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h:107:
ash::ShelfModel* shelf_model() const { return model_; }
Why not provide a GetShelfModel() on ChromeLauncherController instead? Then
there would be the possibility of returning a fake/stub one in unit tests.

It feels kinda of weird to have a relatively nice abstract base class (CLC) then
tie all the other classes to the concrete CLCImpl.

Is your long-term plan to collapse CLC and CLCImpl? Am I missing something?

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h (right):

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h:51:
std::unique_ptr<ash::ShelfModel> chrome_shelf_model_;
nit: This and the CLCImpl above need a comment that they are only used in mus. I
would probably put the two members next to each other in the list.

[msw, 2017-04-26 01:35:15]

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h (right):

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.h:107:
ash::ShelfModel* shelf_model() const { return model_; }
On 2017/04/26 01:03:26, James Cook wrote:
> Why not provide a GetShelfModel() on ChromeLauncherController instead? Then
> there would be the possibility of returning a fake/stub one in unit tests.
> 
> It feels kinda of weird to have a relatively nice abstract base class (CLC)
then
> tie all the other classes to the concrete CLCImpl.
> 
> Is your long-term plan to collapse CLC and CLCImpl? Am I missing something?

Yes, I plan to merge CLC and CLCImpl in an imminent followup CL. I thought this
might be easier, otherwise I'd have to go back and switch GetShelfModel() to
shelf_model() at all these call sites anyway.

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h (right):

https://codereview.chromium.org/2835063003/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h:51:
std::unique_ptr<ash::ShelfModel> chrome_shelf_model_;
On 2017/04/26 01:03:26, James Cook wrote:
> nit: This and the CLCImpl above need a comment that they are only used in mus.
I
> would probably put the two members next to each other in the list.

Done.

[msw, 2017-04-26 01:35:23]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-26 01:36:09]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2017-04-26 01:52:59]

Got it. LGTM

[commit-bot: I haz the power, 2017-04-26 02:42:31]

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

[commit-bot: I haz the power, 2017-04-26 02:42:31]

Dry run: This issue passed the CQ dry run.

[msw, 2017-04-26 03:16:02]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2017-04-26 03:16:38]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 04:00:38]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493176562853900,
"parent_rev": "2330a79389bf0976eea62788b434a366b0e72c5b", "commit_rev":
"6fd672f43f11c93d78c93f39c9d0fb9a50d98375"}

[commit-bot: I haz the power, 2017-04-26 04:00:47]

Description was changed from

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Use ChromeLauncherControllerImpl in mash with a fake ShelfModel.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior and bad interaction in mash.

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=jamescook@chromium.org
==========

to

==========
mash: Use ChromeLauncherControllerImpl, remove Mus subclass stub.

Use ChromeLauncherControllerImpl in mash with a fake ShelfModel.

Update BrowserShortcutLauncherItemController to not crash in mash:
-Use BrowserList::GetLastActive instead of ash::wm::GetActiveWindow.
-Use the locally cached |shelf_model_| instead of Shell's accessor.

Avoid crashes by disabling some behavior in mash (no Shell):
-Skip BrowserStatusMonitor's activation client observation.
-Skip ChromeLauncherController's SystemTrayDelegate & Keyboard use.
-Use a stub MultiUserWindowManager instance.

Remove unused ChromeLauncherControllerMus stub.

TODO: Fix disabled behavior and bad interaction in mash.

BUG=557406
TEST=No classic Chrome OS or chrome --mash behavior changes.
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2835063003
Cr-Commit-Position: refs/heads/master@{#467228}
Committed:
https://chromium.googlesource.com/chromium/src/+/6fd672f43f11c93d78c93f39c9d0...
==========

[commit-bot: I haz the power, 2017-04-26 04:00:50]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/6fd672f43f11c93d78c93f39c9d0...