chromeos: Promotes more from WmShell to Shell

chromeos: Promotes more from WmShell to Shell

Specifically ShellDelegate, PrefClientStore and WallpaperDelegate.

I also added Shell::Get() and deprecated GetInstance(). Will rename
closer to branch.

BUG=687660
TEST=covered by tests
R=elijahtaylor@chromium.org, jamescook@chromium.org, rockot@chromium.org
TBR=rockot@chromium.org

Review-Url: https://codereview.chromium.org/2741273002
Cr-Commit-Position: refs/heads/master@{#456403}
Committed: https://chromium.googlesource.com/chromium/src/+/b073203afbe9bd42ecb403983e04a947f5023f5e

[sky, 2017-03-10 23:55:00]

elijahtaylor: arg
jamescook: the rest

[sky, 2017-03-10 23:55:06]

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

[commit-bot: I haz the power, 2017-03-10 23:55:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[elijahtaylor1, 2017-03-11 00:14:23]

arc lgtm rubberstamp

[commit-bot: I haz the power, 2017-03-11 00:26:33]

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

[commit-bot: I haz the power, 2017-03-11 00:26:34]

Dry run: This issue passed the CQ dry run.

[James Cook, 2017-03-11 00:40:43]

LGTM with nits. I'm liking Shell::Get() and I like the approach of gradually
converting.

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.cc (right):

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:121: : WmShell(),
nit: I don't think this line is needed.

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/window_manager.cc
File ash/mus/window_manager.cc (right):

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:200: init_params.delegate = shell_delegate_for_test_
Can you add a comment by ShellInitParams::delegate that Shell takes ownership?
Or make it a unique_ptr<>? It took me a while to re-understand ownership here.

https://codereview.chromium.org/2741273002/diff/20001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2741273002/diff/20001/ash/shell.h#newcode305
ash/shell.h:305: ShellDelegate* delegate() { return delegate_.get(); }
optional: Since you're moving it, and there are already a bunch of foo_delegates
here, WDYT of renaming this to shell_delegate_?

[sky, 2017-03-13 15:23:19]

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/bridge/wm_shell...
File ash/mus/bridge/wm_shell_mus.cc (right):

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/bridge/wm_shell...
ash/mus/bridge/wm_shell_mus.cc:121: : WmShell(),
On 2017/03/11 00:40:43, James Cook wrote:
> nit: I don't think this line is needed.

Done.

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/window_manager.cc
File ash/mus/window_manager.cc (right):

https://codereview.chromium.org/2741273002/diff/20001/ash/mus/window_manager....
ash/mus/window_manager.cc:200: init_params.delegate = shell_delegate_for_test_
On 2017/03/11 00:40:43, James Cook wrote:
> Can you add a comment by ShellInitParams::delegate that Shell takes ownership?
> Or make it a unique_ptr<>? It took me a while to re-understand ownership here.
> 

Done.

https://codereview.chromium.org/2741273002/diff/20001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2741273002/diff/20001/ash/shell.h#newcode305
ash/shell.h:305: ShellDelegate* delegate() { return delegate_.get(); }
On 2017/03/11 00:40:43, James Cook wrote:
> optional: Since you're moving it, and there are already a bunch of
foo_delegates
> here, WDYT of renaming this to shell_delegate_?

Done.

[sky, 2017-03-13 15:23:20]

The CQ bit was checked by sky@chromium.org

[sky, 2017-03-13 15:23:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
elijahtaylor@chromium.org, jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2741273002/#ps40001
(title: "feedback")

[commit-bot: I haz the power, 2017-03-13 15:23:41]

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-03-13 15:31:07]

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

[commit-bot: I haz the power, 2017-03-13 15:31:08]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sky, 2017-03-13 15:54:48]

Description was changed from

==========
chromeos: Promotes more from WmShell to Shell

Specifically ShellDelegate, PrefClientStore and WallpaperDelegate.

I also added Shell::Get() and deprecated GetInstance(). Will rename
closer to branch.

BUG=687660
TEST=covered by tests
R=elijahtaylor@chromium.org, jamescook@chromium.org
==========

to

==========
chromeos: Promotes more from WmShell to Shell

Specifically ShellDelegate, PrefClientStore and WallpaperDelegate.

I also added Shell::Get() and deprecated GetInstance(). Will rename
closer to branch.

BUG=687660
TEST=covered by tests
R=elijahtaylor@chromium.org, jamescook@chromium.org, rockot@chromium.org
==========

[sky, 2017-03-13 15:54:48]

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

[sky, 2017-03-13 15:55:21]

TBR rockot for DEPS change (moved services/preferences/public dep from
ash/common/DEPS to ash/DEPS).

[sky, 2017-03-13 15:55:26]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-03-13 15:55:47]

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-03-13 16:05:30]

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

[commit-bot: I haz the power, 2017-03-13 16:05:31]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sky, 2017-03-13 16:51:57]

Description was changed from

==========
chromeos: Promotes more from WmShell to Shell

Specifically ShellDelegate, PrefClientStore and WallpaperDelegate.

I also added Shell::Get() and deprecated GetInstance(). Will rename
closer to branch.

BUG=687660
TEST=covered by tests
R=elijahtaylor@chromium.org, jamescook@chromium.org, rockot@chromium.org
==========

to

==========
chromeos: Promotes more from WmShell to Shell

Specifically ShellDelegate, PrefClientStore and WallpaperDelegate.

I also added Shell::Get() and deprecated GetInstance(). Will rename
closer to branch.

BUG=687660
TEST=covered by tests
R=elijahtaylor@chromium.org, jamescook@chromium.org, rockot@chromium.org
TBR=rockot@chromium.org
==========

[sky, 2017-03-13 16:52:03]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-03-13 16:52:44]

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-03-13 16:59:20]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489423923295010,
"parent_rev": "08583e375717564923404ed4b6fd6660ce259a97", "commit_rev":
"b073203afbe9bd42ecb403983e04a947f5023f5e"}

[commit-bot: I haz the power, 2017-03-13 16:59:59]

Description was changed from

==========
chromeos: Promotes more from WmShell to Shell

Specifically ShellDelegate, PrefClientStore and WallpaperDelegate.

I also added Shell::Get() and deprecated GetInstance(). Will rename
closer to branch.

BUG=687660
TEST=covered by tests
R=elijahtaylor@chromium.org, jamescook@chromium.org, rockot@chromium.org
TBR=rockot@chromium.org
==========

to

==========
chromeos: Promotes more from WmShell to Shell

Specifically ShellDelegate, PrefClientStore and WallpaperDelegate.

I also added Shell::Get() and deprecated GetInstance(). Will rename
closer to branch.

BUG=687660
TEST=covered by tests
R=elijahtaylor@chromium.org, jamescook@chromium.org, rockot@chromium.org
TBR=rockot@chromium.org

Review-Url: https://codereview.chromium.org/2741273002
Cr-Commit-Position: refs/heads/master@{#456403}
Committed:
https://chromium.googlesource.com/chromium/src/+/b073203afbe9bd42ecb403983e04...
==========

[commit-bot: I haz the power, 2017-03-13 17:00:00]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b073203afbe9bd42ecb403983e04...