Cleanup shelf initialization and observation.

Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
R=sky@chromium.org

Committed: https://crrev.com/174379b507800b1fd819bb8907644682e9a6302a
Cr-Commit-Position: refs/heads/master@{#385060}

[msw, 2016-04-01 18:56:46]

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

[commit-bot: I haz the power, 2016-04-01 18:57:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1851883002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1851883002/1

[commit-bot: I haz the power, 2016-04-01 19:52:31]

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

[commit-bot: I haz the power, 2016-04-01 19:52:32]

Dry run: This issue passed the CQ dry run.

[msw, 2016-04-02 00:32:33]

Description was changed from

==========
Cleanup shelf observation.

BUG=
==========

to

==========
Cleanup shelf initialization and observation.

BUG=
==========

[msw, 2016-04-04 19:30:19]

Description was changed from

==========
Cleanup shelf initialization and observation.

BUG=
==========

to

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
Add Shell::OnShelfAutoHideBehaviorChanged (like alignment)
Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.

Move alignment and auto-hide properties to Shelf class.
(lets us reduce dependence on ShelfLayoutManager)

Simplify Shell functions to access shelf properties.
Simplify ChromeLauncherControlller shelf observation.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

Init Shell's ShelfModel earlier.
Call ShelfDelegate::OnShelfCreated after ShelfWidget::shelf_ init.

BUG=
==========

[msw, 2016-04-04 19:30:58]

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

[commit-bot: I haz the power, 2016-04-04 19:31:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1851883002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1851883002/100001

[commit-bot: I haz the power, 2016-04-04 19:52:05]

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

[commit-bot: I haz the power, 2016-04-04 19:52:06]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[msw, 2016-04-04 20:20:58]

Description was changed from

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
Add Shell::OnShelfAutoHideBehaviorChanged (like alignment)
Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.

Move alignment and auto-hide properties to Shelf class.
(lets us reduce dependence on ShelfLayoutManager)

Simplify Shell functions to access shelf properties.
Simplify ChromeLauncherControlller shelf observation.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

Init Shell's ShelfModel earlier.
Call ShelfDelegate::OnShelfCreated after ShelfWidget::shelf_ init.

BUG=
==========

to

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
==========

[msw, 2016-04-04 20:49:50]

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

[msw, 2016-04-04 20:50:18]

Description was changed from

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
==========

to

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
R=sky@chromium.org
==========

[msw, 2016-04-04 20:50:18]

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

[commit-bot: I haz the power, 2016-04-04 20:50:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1851883002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1851883002/120001

[msw, 2016-04-04 20:50:47]

Hey Scott, please take a look; thanks! +CC James for FYI.

[commit-bot: I haz the power, 2016-04-04 21:32:36]

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

[commit-bot: I haz the power, 2016-04-04 21:32:37]

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

[msw, 2016-04-04 21:56:36]

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

[commit-bot: I haz the power, 2016-04-04 21:57:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1851883002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1851883002/140001

[sky, 2016-04-04 22:44:32]

https://codereview.chromium.org/1851883002/diff/140001/ash/shelf/shelf.cc
File ash/shelf/shelf.cc (right):

https://codereview.chromium.org/1851883002/diff/140001/ash/shelf/shelf.cc#new...
ash/shelf/shelf.cc:87: return locked ? SHELF_ALIGNMENT_BOTTOM : alignment_;
Seems unusual to force an alignment like this. Couldn't the code explicitly set
the alignment to bottom when necessary? Doing that ensures delegates and what
not are notified when locking if the alignment wasn't bottom.

[commit-bot: I haz the power, 2016-04-04 23:11:25]

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

[commit-bot: I haz the power, 2016-04-04 23:11:26]

Dry run: This issue passed the CQ dry run.

[sky, 2016-04-04 23:59:21]

On 2016/04/04 22:44:32, sky wrote:
> https://codereview.chromium.org/1851883002/diff/140001/ash/shelf/shelf.cc
> File ash/shelf/shelf.cc (right):
> 
>
https://codereview.chromium.org/1851883002/diff/140001/ash/shelf/shelf.cc#new...
> ash/shelf/shelf.cc:87: return locked ? SHELF_ALIGNMENT_BOTTOM : alignment_;
> Seems unusual to force an alignment like this. Couldn't the code explicitly
set
> the alignment to bottom when necessary? Doing that ensures delegates and what
> not are notified when locking if the alignment wasn't bottom.

The code was like this before, so don't bother fixing it now.

[sky, 2016-04-04 23:59:29]

LGTM

[msw, 2016-04-05 00:08:04]

On 2016/04/04 23:59:21, sky wrote:
> On 2016/04/04 22:44:32, sky wrote:
> > https://codereview.chromium.org/1851883002/diff/140001/ash/shelf/shelf.cc
> > File ash/shelf/shelf.cc (right):
> > 
> >
>
https://codereview.chromium.org/1851883002/diff/140001/ash/shelf/shelf.cc#new...
> > ash/shelf/shelf.cc:87: return locked ? SHELF_ALIGNMENT_BOTTOM : alignment_;
> > Seems unusual to force an alignment like this. Couldn't the code explicitly
> set
> > the alignment to bottom when necessary? Doing that ensures delegates and
what
> > not are notified when locking if the alignment wasn't bottom.
> 
> The code was like this before, so don't bother fixing it now.

Yeah, I agree it's not straightforward, but as discussed offline, I'm just
moving the existing logic from ShelfLayoutManager to Shelf.

Changing this in a followup CL may be nice, but AFAICT it won't be entirely
trivial: we'll need a the Shelf (or a helper) to be a ShellObserver,
SessionStateObserver, and a LockStateObserver, and move visibility locking
simultaneously, so we can tell the ShelfLayoutManager to animate visibility
changes on certain lock/unlock changes... Perhaps that all can be simplified,
but I'm not totally sure.

[msw, 2016-04-05 00:08:46]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2016-04-05 00:09:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1851883002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1851883002/140001

[commit-bot: I haz the power, 2016-04-05 00:25:18]

Description was changed from

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
R=sky@chromium.org
==========

to

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
R=sky@chromium.org
==========

[commit-bot: I haz the power, 2016-04-05 00:25:19]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-05 00:27:00]

Description was changed from

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
R=sky@chromium.org
==========

to

==========
Cleanup shelf initialization and observation.

Add ShelfDelegate::OnShelf[Alignment|AutoHideBehavior]Changed.
(needed for mus: https://codereview.chromium.org/1839223003)
(simplifies ChromeLauncherController shelf observation)

Remove ShelfLayoutManagerObserver::OnAutoHideBehaviorChanged.
Add Shell[Observer]::OnShelfAutoHideBehaviorChanged
(similar to alignment calls, used for ShelfLayoutManager)

Move alignment and auto-hide properties to Shelf class.
(lets us better encapsulate ShelfLayoutManager)

Simplify Shell functions that access shelf properties.
Init Shell's ShelfModel earlier (removes ordering constraint).
Call OnShelfCreated after ShelfWidget::shelf_ is actually set.

Make ChromeLauncherController set properties in OnShelfCreated.
(not before the actual shelf exists; update tests similarly)
Skip redundant *FromPrefs init; see SetShelfBehaviorsFromPrefs.
Inline SetShelfAutoHideBehaviorPrefs (like alignment).

BUG=557406
TEST=No regressions
R=sky@chromium.org

Committed: https://crrev.com/174379b507800b1fd819bb8907644682e9a6302a
Cr-Commit-Position: refs/heads/master@{#385060}
==========

[commit-bot: I haz the power, 2016-04-05 00:27:01]

Patchset 8 (id:??) landed as
https://crrev.com/174379b507800b1fd819bb8907644682e9a6302a
Cr-Commit-Position: refs/heads/master@{#385060}

[Daniel Kurtz, 2016-04-11 10:17:15]

On 2016/04/05 00:27:01, commit-bot: I haz the power wrote:
> Patchset 8 (id:??) landed as
> https://crrev.com/174379b507800b1fd819bb8907644682e9a6302a
> Cr-Commit-Position: refs/heads/master@{#385060}

A git bisect suggests that this patch causes crbug.com/602166 - the chrome OS
clock/battery/keyboard have drifted up out from the shelf to the upper left
corner at the login screen.

[James Cook, 2016-04-11 15:02:20]

On 2016/04/11 10:17:15, Daniel Kurtz wrote:
> On 2016/04/05 00:27:01, commit-bot: I haz the power wrote:
> > Patchset 8 (id:??) landed as
> > https://crrev.com/174379b507800b1fd819bb8907644682e9a6302a
> > Cr-Commit-Position: refs/heads/master@{#385060}
> 
> A git bisect suggests that this patch causes crbug.com/602166 - the chrome OS
> clock/battery/keyboard have drifted up out from the shelf to the upper left
> corner at the login screen.

I thought Mike fixed that issue with https://codereview.chromium.org/1866113002/

"Fix Chrome OS Login status area layout.

Let ShelfLayoutManager perform layout before a shelf exists.
Fixes a regression from https://codereview.chromium.org/1851883002"

[msw, 2016-04-11 16:41:04]

On 2016/04/11 15:02:20, James Cook wrote:
> On 2016/04/11 10:17:15, Daniel Kurtz wrote:
> > On 2016/04/05 00:27:01, commit-bot: I haz the power wrote:
> > > Patchset 8 (id:??) landed as
> > > https://crrev.com/174379b507800b1fd819bb8907644682e9a6302a
> > > Cr-Commit-Position: refs/heads/master@{#385060}
> > 
> > A git bisect suggests that this patch causes crbug.com/602166 - the chrome
OS
> > clock/battery/keyboard have drifted up out from the shelf to the upper left
> > corner at the login screen.
> 
> I thought Mike fixed that issue with
https://codereview.chromium.org/1866113002/
> 
> "Fix Chrome OS Login status area layout.
> 
> Let ShelfLayoutManager perform layout before a shelf exists.
> Fixes a regression from https://codereview.chromium.org/1851883002%22

Yes, it should be fixed, I marked the bug as duplicate, lmk if any issues
persist.