Fix shelf alignment and auto-hide initialization.

Fix shelf alignment and auto-hide initialization.

Leave the shelf locked and hidden until user prefs are loaded.
(otherwise, a left/right shelf may be placed incorrectly on login)

Make TestShelfDelegate init the shelf prefs, like Chrome would.
Use TestShelfDelegate as the base class in shelf_view_unittest.cc.

To fix mash_unittests:
-Use a TestShellDelegate (and its test sub-delegates).
-Remove TestShelfDelegate's ShelfModel param (WmShell not ready). 
-Fix an expectation in AcceleratorControllerTest.GlobalAccelerators.

BUG=681883
TEST=automated; snapped windows don't shrink across login w/left-shelf; no shelf behavior changes.
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2688353005
Cr-Commit-Position: refs/heads/master@{#450378}
Committed: https://chromium.googlesource.com/chromium/src/+/0bc6a07e9b04a8885f9e206f035e7c2e011b132c

[msw, 2017-02-13 23:17:55]

Description was changed from

==========
Fix shelf alignment and auto-hide initialization.

BUG=681883
TEST=automated; snapped windows retain sizes across login w/left-shelf.
R=jamescook@chromium.org
==========

to

==========
Fix shelf alignment and auto-hide initialization.

Leave the shelf locked and hidden until user prefs are loaded.
(otherwise, a left/right shelf may be placed incorrectly on login)

Make TestShelfDelegate init the shelf prefs, like Chrome would.
Use TestShelfDelegate as the base class in shelf_view_unittest.cc.

BUG=681883
TEST=automated; snapped windows don't shrink across login w/left-shelf; no shelf
behavior changes.
R=jamescook@chromium.org
==========

[msw, 2017-02-13 23:19:07]

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

[commit-bot: I haz the power, 2017-02-13 23:20:17]

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

[msw, 2017-02-13 23:22:42]

Hey James, please take a look; thanks!

[James Cook, 2017-02-14 01:38:03]

LGTM with nits.

https://codereview.chromium.org/2688353005/diff/20001/ash/common/shelf/wm_she...
File ash/common/shelf/wm_shelf.h (right):

https://codereview.chromium.org/2688353005/diff/20001/ash/common/shelf/wm_she...
ash/common/shelf/wm_shelf.h:167: ShelfAutoHideBehavior auto_hide_behavior_ =
SHELF_AUTO_HIDE_ALWAYS_HIDDEN;
This makes me a bit nervous. It's probably fine, but could you make sure you've
tested a couple cases manually before landing this CL? Specifically I'm worried
about:
* OOBE (run --login-manager with empty profile dir)
* Supervised user creation (using the ... in the fake shelf at login)

If those are OK then I'm good with this.

https://codereview.chromium.org/2688353005/diff/20001/ash/common/test/test_sh...
File ash/common/test/test_shelf_delegate.cc (right):

https://codereview.chromium.org/2688353005/diff/20001/ash/common/test/test_sh...
ash/common/test/test_shelf_delegate.cc:50: shelf_initializer_ =
base::MakeUnique<ShelfInitializer>();
Can this go in the initializer list? If not, comment why.

[James Cook, 2017-02-14 01:38:41]

Hrm, you have some mash_unittest failures.

[msw, 2017-02-14 03:47:41]

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

[commit-bot: I haz the power, 2017-02-14 03:48:13]

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

[msw, 2017-02-14 03:53:27]

Description was changed from

==========
Fix shelf alignment and auto-hide initialization.

Leave the shelf locked and hidden until user prefs are loaded.
(otherwise, a left/right shelf may be placed incorrectly on login)

Make TestShelfDelegate init the shelf prefs, like Chrome would.
Use TestShelfDelegate as the base class in shelf_view_unittest.cc.

BUG=681883
TEST=automated; snapped windows don't shrink across login w/left-shelf; no shelf
behavior changes.
R=jamescook@chromium.org
==========

to

==========
Fix shelf alignment and auto-hide initialization.

Leave the shelf locked and hidden until user prefs are loaded.
(otherwise, a left/right shelf may be placed incorrectly on login)

Make TestShelfDelegate init the shelf prefs, like Chrome would.
Use TestShelfDelegate as the base class in shelf_view_unittest.cc.

To fix mash_unittests:
-Use a TestShellDelegate (and its test sub-delegates).
-Remove TestShelfDelegate's ShelfModel param (WmShell not ready). 
-Fix an expectation in AcceleratorControllerTest.GlobalAccelerators.

BUG=681883
TEST=automated; snapped windows don't shrink across login w/left-shelf; no shelf
behavior changes.
R=jamescook@chromium.org
==========

[commit-bot: I haz the power, 2017-02-14 04:50:41]

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

[commit-bot: I haz the power, 2017-02-14 04:50:42]

Dry run: This issue passed the CQ dry run.

[msw, 2017-02-14 05:13:18]

Please take another look, thanks! (I also fixed mash_unittest failures)

https://codereview.chromium.org/2688353005/diff/20001/ash/common/shelf/wm_she...
File ash/common/shelf/wm_shelf.h (right):

https://codereview.chromium.org/2688353005/diff/20001/ash/common/shelf/wm_she...
ash/common/shelf/wm_shelf.h:167: ShelfAutoHideBehavior auto_hide_behavior_ =
SHELF_AUTO_HIDE_ALWAYS_HIDDEN;
On 2017/02/14 01:38:03, James Cook wrote:
> This makes me a bit nervous. It's probably fine, but could you make sure
you've
> tested a couple cases manually before landing this CL? Specifically I'm
worried
> about:
> * OOBE (run --login-manager with empty profile dir)
> * Supervised user creation (using the ... in the fake shelf at login)
> 
> If those are OK then I'm good with this.

Done. I had already run OOBE with an existing account; I also tried a new
account. (when signing in to an existing account with a non-bottom shelf pref,
we use the bottom alignment (and blank wallpaper) for a short time, before the
cloud prefs are loaded).

Hmm, supervised users don't have the option of changing the shelf alignment, but
that also seems to be the case on ToT. Perhaps that's intentional, or perhaps
they're already getting stuck in SHELF_ALIGNMENT_BOTTOM_LOCKED after login.
Relatedly, I wasn't able to test "Browse as Guest" manually, because it has an
unrelated login crash. Do you think I should dig in more here for this CL?

https://codereview.chromium.org/2688353005/diff/20001/ash/common/test/test_sh...
File ash/common/test/test_shelf_delegate.cc (right):

https://codereview.chromium.org/2688353005/diff/20001/ash/common/test/test_sh...
ash/common/test/test_shelf_delegate.cc:50: shelf_initializer_ =
base::MakeUnique<ShelfInitializer>();
On 2017/02/14 01:38:03, James Cook wrote:
> Can this go in the initializer list? If not, comment why.

Done.

[James Cook, 2017-02-14 15:12:07]

Still LGTM

For supervised users I was thinking:
* Login to an account, set shelf position left
* Do supervised user addition flow, logging in as that user
* Make sure shelf looks reasonable (I don't really care if it's left or bottom,
just that it doesn't seem broken)

It's possible the very last stage of supervised user creation will either be
really slow or time out when you run on Linux desktop. That's OK.

Supervised users are weird in that you're signed in during the create flow but
the shelf is in a weird state until the end.

https://codereview.chromium.org/2688353005/diff/60001/ash/mus/window_manager.h
File ash/mus/window_manager.h (right):

https://codereview.chromium.org/2688353005/diff/60001/ash/mus/window_manager....
ash/mus/window_manager.h:198: std::unique_ptr<ShellDelegate>
shell_delegate_for_test_;
Woo hoo, thanks for doing this!

[msw, 2017-02-14 16:26:18]

On 2017/02/14 15:12:07, James Cook wrote:
> For supervised users I was thinking:
> * Login to an account, set shelf position left
> * Do supervised user addition flow, logging in as that user
> * Make sure shelf looks reasonable (I don't really care if it's left or
bottom,
> just that it doesn't seem broken)
> 
> It's possible the very last stage of supervised user creation will either be
> really slow or time out when you run on Linux desktop. That's OK.
> 
> Supervised users are weird in that you're signed in during the create flow but
> the shelf is in a weird state until the end.

The fake shelf is bottom-aligned during supervised user creation, it seems ok.
Oddly, WmShelf::CanChangeShelfAlignment returns false for supervised users.
(they can enable auto-hide and change wallpaper, but not alignment...)

https://codereview.chromium.org/2688353005/diff/60001/ash/mus/window_manager.h
File ash/mus/window_manager.h (right):

https://codereview.chromium.org/2688353005/diff/60001/ash/mus/window_manager....
ash/mus/window_manager.h:198: std::unique_ptr<ShellDelegate>
shell_delegate_for_test_;
On 2017/02/14 15:12:07, James Cook wrote:
> Woo hoo, thanks for doing this!

You're welcome :)

[msw, 2017-02-14 16:26:30]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2017-02-14 16:26:56]

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-02-14 16:33:42]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487089590762360,
"parent_rev": "1ac98dca91bdf6347a4470f803ba7913160c84ab", "commit_rev":
"0bc6a07e9b04a8885f9e206f035e7c2e011b132c"}

[commit-bot: I haz the power, 2017-02-14 16:34:14]

Description was changed from

==========
Fix shelf alignment and auto-hide initialization.

Leave the shelf locked and hidden until user prefs are loaded.
(otherwise, a left/right shelf may be placed incorrectly on login)

Make TestShelfDelegate init the shelf prefs, like Chrome would.
Use TestShelfDelegate as the base class in shelf_view_unittest.cc.

To fix mash_unittests:
-Use a TestShellDelegate (and its test sub-delegates).
-Remove TestShelfDelegate's ShelfModel param (WmShell not ready). 
-Fix an expectation in AcceleratorControllerTest.GlobalAccelerators.

BUG=681883
TEST=automated; snapped windows don't shrink across login w/left-shelf; no shelf
behavior changes.
R=jamescook@chromium.org
==========

to

==========
Fix shelf alignment and auto-hide initialization.

Leave the shelf locked and hidden until user prefs are loaded.
(otherwise, a left/right shelf may be placed incorrectly on login)

Make TestShelfDelegate init the shelf prefs, like Chrome would.
Use TestShelfDelegate as the base class in shelf_view_unittest.cc.

To fix mash_unittests:
-Use a TestShellDelegate (and its test sub-delegates).
-Remove TestShelfDelegate's ShelfModel param (WmShell not ready). 
-Fix an expectation in AcceleratorControllerTest.GlobalAccelerators.

BUG=681883
TEST=automated; snapped windows don't shrink across login w/left-shelf; no shelf
behavior changes.
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2688353005
Cr-Commit-Position: refs/heads/master@{#450378}
Committed:
https://chromium.googlesource.com/chromium/src/+/0bc6a07e9b04a8885f9e206f035e...
==========

[commit-bot: I haz the power, 2017-02-14 16:34:15]

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