chromeos: Refactor shelf to create ShelfView earlier in startup

chromeos: Refactor shelf to create ShelfView earlier in startup

Currently ShelfView is created in the middle of login, after the profile
loads, whereas the ShelfWidget and StatusAreaWidget are created on display
initialization. We also have complex logic around showing and hiding the
shelf based on login state, supervised user creation flows, etc. This makes
it difficult to reason about the state of the shelf.

Instead, always create the ShelfView when the widget is created. The view
stays hidden at login because its window container is hidden. This makes it
easier to reason about ShelfView (it's always there). We also want this
because we want to replace the Web UI "fake shelf" during OOBE and login
with a views-based shelf. It may help with Clusterfuzz crashes we're
seeing due to shelf not being fully initialized during the tests.

This is a refactor and should not cause behavior changes.

BUG=717559, 674773
TEST=ash_unittests, browser_tests, manually testing login, ChromeVox, and
adding/removing displays
TBR=tsepez@chromium.org for rename

Review-Url: https://codereview.chromium.org/2889673002
Cr-Commit-Position: refs/heads/master@{#472832}
Committed: https://chromium.googlesource.com/chromium/src/+/788b4fcb1909904cda962cac4a637ffecfa90c1a

[James Cook, 2017-05-16 19:33:45]

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

[James Cook, 2017-05-16 19:34:20]

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

[commit-bot: I haz the power, 2017-05-16 19:35:07]

Dry run: 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-05-16 20:05:44]

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

[commit-bot: I haz the power, 2017-05-16 20:05:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[James Cook, 2017-05-16 20:17:29]

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

[commit-bot: I haz the power, 2017-05-16 20:18:53]

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-05-16 21:31:35]

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

[James Cook, 2017-05-16 21:32:34]

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

[James Cook, 2017-05-16 21:32:36]

msw, please take a look. There's more ShelfView cleanup to do, but this is as
much as I could handle in a single CL.

jdufault, just FYI.

https://codereview.chromium.org/2889673002/diff/60001/ash/public/interfaces/s...
File ash/public/interfaces/shelf.mojom (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/public/interfaces/s...
ash/public/interfaces/shelf.mojom:77: // to observe and to set prefs.
I think this could dramatically simplify ash/chrome communication. It doesn't
fully work in mash (e.g. crbug.com/721961)

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:244: if (in_shutdown_ || !shelf_window)
It's unclear if the shelf_window check is needed but I don't want to touch it.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_locking...
File ash/shelf/shelf_locking_manager.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_locking...
ash/shelf/shelf_locking_manager.cc:16:
stored_alignment_(SHELF_ALIGNMENT_BOTTOM_LOCKED),
This is to avoid reading out of a partially-constructed WmShelf.

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

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.h:120: ShelfView* const shelf_view_;
This "const" is basically the whole point of this CL. :-)

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc
File ash/shelf/wm_shelf.cc (left):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc#o...
ash/shelf/wm_shelf.cc:124: bezel_event_handler_ =
base::MakeUnique<ShelfBezelEventHandler>(this);
The timing here doesn't matter so I moved it up.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.h
File ash/shelf/wm_shelf.h (left):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.h#ol...
ash/shelf/wm_shelf.h:37: // controller. Note that the shelf widget may not be
created until after login.
This bit about the widget was a lie.

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.h#newcode655
ash/shell.h:655: void InitializeShelf();
Private visibility is best visibility!

https://codereview.chromium.org/2889673002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/accessibility/accessibility_manager.cc (left):

https://codereview.chromium.org/2889673002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/accessibility_manager.cc:1526:
shelf_layout_manager->SetChromeVoxPanelHeight(0);
This code was kind-of copy/pasted so I collapsed the copies.

[commit-bot: I haz the power, 2017-05-16 21:33:36]

Dry run: CQ is trying da patch.

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

[msw, 2017-05-16 22:42:01]

Mostly minor comments; looks pretty good.

https://codereview.chromium.org/2889673002/diff/60001/ash/public/interfaces/s...
File ash/public/interfaces/shelf.mojom (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/public/interfaces/s...
ash/public/interfaces/shelf.mojom:77: // to observe and to set prefs.
On 2017/05/16 21:32:35, James Cook wrote:
> I think this could dramatically simplify ash/chrome communication. It doesn't
> fully work in mash (e.g. crbug.com/721961)

Acknowledged. Maybe file a bug to track that idea and cite it here?

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
ash/root_window_controller.cc:766: // window is hidden.
nit: Maybe tie this comment into Shelf init? (eg. "... hidden; InitializeShelf
will make it visible.")

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
File ash/root_window_controller.h (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
ash/root_window_controller.h:142: // TODO(jamescook): Eliminate this. It always
returns true after the
nit: There's only two occurrences, maybe just do this now by inlining
wm_shelf()->shelf_widget()? (we should also combine |wm_shelf()| and
|GetShelf()| down the line)

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:238: // TODO: Remove this when webui
fake-shelf is replaced with views.
nit: cite the bug, consider "  // TODO(crbug.com/######): Remove this when
login/lock use the real shelf." or similar? ditto below (line 896)

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:244: if (in_shutdown_ || !shelf_window)
On 2017/05/16 21:32:35, James Cook wrote:
> It's unclear if the shelf_window check is needed but I don't want to touch it.

Acknowledged.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_locking...
File ash/shelf/shelf_locking_manager.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_locking...
ash/shelf/shelf_locking_manager.cc:16:
stored_alignment_(SHELF_ALIGNMENT_BOTTOM_LOCKED),
On 2017/05/16 21:32:35, James Cook wrote:
> This is to avoid reading out of a partially-constructed WmShelf.

Acknowledged.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget.cc
File ash/shelf/shelf_widget.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.cc:187: // TODO(jamescook): Is the session check needed?
nit: address now? (so many TODOs!)

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.cc:243: bool ShelfWidget::IsShelfVisible() const {
nit: remove? this will always return true now, right? (also, afaict, the only
caller is WmShelf::IsVisible, which isn't called, so that can also be removed?)

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

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.h:102: ShelfView* shelf_view_for_testing() { return
shelf_view_; }
nit: const?

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.h:120: ShelfView* const shelf_view_;
On 2017/05/16 21:32:35, James Cook wrote:
> This "const" is basically the whole point of this CL. :-)

Acknowledged.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget_...
File ash/shelf/shelf_widget_unittest.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget_...
ash/shelf/shelf_widget_unittest.cc:337: // Shelf components are created.
Should this be disabling TestShellDelegate's ShelfInitializer and ensuring the
shelf has the expected initial values?

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget_...
ash/shelf/shelf_widget_unittest.cc:351: // Settings are applied correctly.
nit: "Ensure settings are applied correctly."

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc
File ash/shelf/wm_shelf.cc (left):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc#o...
ash/shelf/wm_shelf.cc:124: bezel_event_handler_ =
base::MakeUnique<ShelfBezelEventHandler>(this);
On 2017/05/16 21:32:35, James Cook wrote:
> The timing here doesn't matter so I moved it up.

Acknowledged.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc
File ash/shelf/wm_shelf.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc#n...
ash/shelf/wm_shelf.cc:68: :
shelf_locking_manager_(base::MakeUnique<ShelfLockingManager>(this)) {
This should no longer be a unique_ptr

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc#n...
ash/shelf/wm_shelf.cc:149: DCHECK(shelf_locking_manager_);
nit: remove

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.h
File ash/shelf/wm_shelf.h (left):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.h#ol...
ash/shelf/wm_shelf.h:37: // controller. Note that the shelf widget may not be
created until after login.
On 2017/05/16 21:32:35, James Cook wrote:
> This bit about the widget was a lie.

Acknowledged.

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

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.h#ne...
ash/shelf/wm_shelf.h:39: class ASH_EXPORT WmShelf : public
ShelfLayoutManagerObserver {
Should we rename this to Shelf at some point? :D

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.cc#newcode1220
ash/shell.cc:1220: // Finalize the shelf when a session becomes active. It's
safe to do this
nit: s/Finalize/Initialize/?

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.cc#newcode1258
ash/shell.cc:1258: for (WmWindow* root_window :
shell_port_->GetAllRootWindows())
optional nit: avoid WmWindow usage.

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.h#newcode654
ash/shell.h:654: // profile is available.
nit: 'the profile'

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.h#newcode655
ash/shell.h:655: void InitializeShelf();
On 2017/05/16 21:32:35, James Cook wrote:
> Private visibility is best visibility!

Acknowledged.

[commit-bot: I haz the power, 2017-05-16 23:08:58]

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

[commit-bot: I haz the power, 2017-05-16 23:08:58]

Dry run: This issue passed the CQ dry run.

[James Cook, 2017-05-17 16:15:56]

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

[James Cook, 2017-05-17 16:16:13]

msw, please take another look.

https://codereview.chromium.org/2889673002/diff/60001/ash/public/interfaces/s...
File ash/public/interfaces/shelf.mojom (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/public/interfaces/s...
ash/public/interfaces/shelf.mojom:77: // to observe and to set prefs.
On 2017/05/16 22:41:59, msw wrote:
> On 2017/05/16 21:32:35, James Cook wrote:
> > I think this could dramatically simplify ash/chrome communication. It
doesn't
> > fully work in mash (e.g. crbug.com/721961)
> 
> Acknowledged. Maybe file a bug to track that idea and cite it here?

Done.

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
ash/root_window_controller.cc:766: // window is hidden.
On 2017/05/16 22:42:00, msw wrote:
> nit: Maybe tie this comment into Shelf init? (eg. "... hidden; InitializeShelf
> will make it visible.")

Done.

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
File ash/root_window_controller.h (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/root_window_control...
ash/root_window_controller.h:142: // TODO(jamescook): Eliminate this. It always
returns true after the
On 2017/05/16 22:42:00, msw wrote:
> nit: There's only two occurrences, maybe just do this now by inlining
> wm_shelf()->shelf_widget()? (we should also combine |wm_shelf()| and
> |GetShelf()| down the line)

Done. As I suspected, it wasn't needed at all.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager.cc:238: // TODO: Remove this when webui
fake-shelf is replaced with views.
On 2017/05/16 22:42:00, msw wrote:
> nit: cite the bug, consider "  // TODO(crbug.com/######): Remove this when
> login/lock use the real shelf." or similar? ditto below (line 896)

Done.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget.cc
File ash/shelf/shelf_widget.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.cc:187: // TODO(jamescook): Is the session check needed?
On 2017/05/16 22:42:00, msw wrote:
> nit: address now? (so many TODOs!)

Ah, you never would have noticed if I hadn't added the line. :-)

Done. Wasn't needed, removed.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.cc:243: bool ShelfWidget::IsShelfVisible() const {
On 2017/05/16 22:42:00, msw wrote:
> nit: remove? this will always return true now, right? (also, afaict, the only
> caller is WmShelf::IsVisible, which isn't called, so that can also be
removed?)

Good catch! Removed.

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

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget....
ash/shelf/shelf_widget.h:102: ShelfView* shelf_view_for_testing() { return
shelf_view_; }
On 2017/05/16 22:42:00, msw wrote:
> nit: const?

Done.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget_...
File ash/shelf/shelf_widget_unittest.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget_...
ash/shelf/shelf_widget_unittest.cc:337: // Shelf components are created.
On 2017/05/16 22:42:00, msw wrote:
> Should this be disabling TestShellDelegate's ShelfInitializer and ensuring the
> shelf has the expected initial values?

I added a new test to check the initial values and moved the checks for shelf
components there (so we don't check for their existence 4 times). I also added a
check that the shelf view is created (which was kinda the point of this CL,
doh!).

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/shelf_widget_...
ash/shelf/shelf_widget_unittest.cc:351: // Settings are applied correctly.
On 2017/05/16 22:42:00, msw wrote:
> nit: "Ensure settings are applied correctly."

Done.

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc
File ash/shelf/wm_shelf.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc#n...
ash/shelf/wm_shelf.cc:68: :
shelf_locking_manager_(base::MakeUnique<ShelfLockingManager>(this)) {
On 2017/05/16 22:42:00, msw wrote:
> This should no longer be a unique_ptr

Done. Sometimes I wish we used unique_ptr<> a bit more to reduce the number of
headers we include in headers, but that's contrary to the style guide. :-(

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.cc#n...
ash/shelf/wm_shelf.cc:149: DCHECK(shelf_locking_manager_);
On 2017/05/16 22:42:00, msw wrote:
> nit: remove

Done.

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

https://codereview.chromium.org/2889673002/diff/60001/ash/shelf/wm_shelf.h#ne...
ash/shelf/wm_shelf.h:39: class ASH_EXPORT WmShelf : public
ShelfLayoutManagerObserver {
On 2017/05/16 22:42:00, msw wrote:
> Should we rename this to Shelf at some point? :D

Yeah. Or ShelfController (and merge with the existing ShelfController? or rename
that one too?).

Whee, renames.

We should talk about when to do this. Now-ish would be good, since we're right
before the branch.

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.cc#newcode1258
ash/shell.cc:1258: for (WmWindow* root_window :
shell_port_->GetAllRootWindows())
On 2017/05/16 22:42:00, msw wrote:
> optional nit: avoid WmWindow usage.

Done.

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.h
File ash/shell.h (right):

https://codereview.chromium.org/2889673002/diff/60001/ash/shell.h#newcode654
ash/shell.h:654: // profile is available.
On 2017/05/16 22:42:00, msw wrote:
> nit: 'the profile'

Done.

[commit-bot: I haz the power, 2017-05-17 16:16:55]

Dry run: 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-05-17 16:21:31]

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

[commit-bot: I haz the power, 2017-05-17 16:21:32]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[James Cook, 2017-05-17 16:44:38]

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

[commit-bot: I haz the power, 2017-05-17 16:45:55]

Dry run: 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-05-17 17:38:30]

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

[commit-bot: I haz the power, 2017-05-17 17:38:31]

Dry run: This issue passed the CQ dry run.

[msw, 2017-05-17 18:19:47]

lgtm!

https://codereview.chromium.org/2889673002/diff/100001/ash/shelf/shelf_widget...
File ash/shelf/shelf_widget_unittest.cc (right):

https://codereview.chromium.org/2889673002/diff/100001/ash/shelf/shelf_widget...
ash/shelf/shelf_widget_unittest.cc:355: TEST_F(ShelfWidgetAfterLoginTest,
InitialValues) {
Great test!

[James Cook, 2017-05-17 18:29:48]

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

[James Cook, 2017-05-17 18:29:49]

dmazzoni, can I get OWNERS for chrome/browser/chromeos/accessibility/* ?

It consolidates two functions that do chrome -> ash communication.

[James Cook, 2017-05-18 15:22:43]

ping dmazzoni?

I think Mike has a CL in progress that will conflict with this one, so I'd like
to land this so he can rebase.

[dmazzoni, 2017-05-18 15:25:35]

lgtm for accessibility - thanks, looks like the lifetime is
much simpler to deal with now

[James Cook, 2017-05-18 16:03:00]

Description was changed from

==========
chromeos: Refactor shelf to create ShelfView earlier in startup

Currently ShelfView is created in the middle of login, after the profile
loads, whereas the ShelfWidget and StatusAreaWidget are created on display
initialization. We also have complex logic around showing and hiding the
shelf based on login state, supervised user creation flows, etc. This makes
it difficult to reason about the state of the shelf.

Instead, always create the ShelfView when the widget is created. The view
stays hidden at login because its window container is hidden. This makes it
easier to reason about ShelfView (it's always there). We also want this
because we want to replace the Web UI "fake shelf" during OOBE and login
with a views-based shelf. It may help with Clusterfuzz crashes we're
seeing due to shelf not being fully initialized during the tests.

This is a refactor and should not cause behavior changes.

BUG=717559,674773
TEST=ash_unittests, browser_tests, manually testing login, ChromeVox, and
adding/removing displays
==========

to

==========
chromeos: Refactor shelf to create ShelfView earlier in startup

Currently ShelfView is created in the middle of login, after the profile
loads, whereas the ShelfWidget and StatusAreaWidget are created on display
initialization. We also have complex logic around showing and hiding the
shelf based on login state, supervised user creation flows, etc. This makes
it difficult to reason about the state of the shelf.

Instead, always create the ShelfView when the widget is created. The view
stays hidden at login because its window container is hidden. This makes it
easier to reason about ShelfView (it's always there). We also want this
because we want to replace the Web UI "fake shelf" during OOBE and login
with a views-based shelf. It may help with Clusterfuzz crashes we're
seeing due to shelf not being fully initialized during the tests.

This is a refactor and should not cause behavior changes.

BUG=717559,674773
TEST=ash_unittests, browser_tests, manually testing login, ChromeVox, and
adding/removing displays
TBR=tsepez@chromium.org for rename
==========

[James Cook, 2017-05-18 16:03:26]

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

[James Cook, 2017-05-18 16:03:27]

TBR tsepez for renaming a method in a mojo file

[James Cook, 2017-05-18 16:07:34]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2017-05-18 16:08:27]

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-05-18 16:16:09]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1495123654781560,
"parent_rev": "f629235df8a59fd2c0c286512f3230f51a3809fe", "commit_rev":
"788b4fcb1909904cda962cac4a637ffecfa90c1a"}

[commit-bot: I haz the power, 2017-05-18 16:16:25]

Description was changed from

==========
chromeos: Refactor shelf to create ShelfView earlier in startup

Currently ShelfView is created in the middle of login, after the profile
loads, whereas the ShelfWidget and StatusAreaWidget are created on display
initialization. We also have complex logic around showing and hiding the
shelf based on login state, supervised user creation flows, etc. This makes
it difficult to reason about the state of the shelf.

Instead, always create the ShelfView when the widget is created. The view
stays hidden at login because its window container is hidden. This makes it
easier to reason about ShelfView (it's always there). We also want this
because we want to replace the Web UI "fake shelf" during OOBE and login
with a views-based shelf. It may help with Clusterfuzz crashes we're
seeing due to shelf not being fully initialized during the tests.

This is a refactor and should not cause behavior changes.

BUG=717559,674773
TEST=ash_unittests, browser_tests, manually testing login, ChromeVox, and
adding/removing displays
TBR=tsepez@chromium.org for rename
==========

to

==========
chromeos: Refactor shelf to create ShelfView earlier in startup

Currently ShelfView is created in the middle of login, after the profile
loads, whereas the ShelfWidget and StatusAreaWidget are created on display
initialization. We also have complex logic around showing and hiding the
shelf based on login state, supervised user creation flows, etc. This makes
it difficult to reason about the state of the shelf.

Instead, always create the ShelfView when the widget is created. The view
stays hidden at login because its window container is hidden. This makes it
easier to reason about ShelfView (it's always there). We also want this
because we want to replace the Web UI "fake shelf" during OOBE and login
with a views-based shelf. It may help with Clusterfuzz crashes we're
seeing due to shelf not being fully initialized during the tests.

This is a refactor and should not cause behavior changes.

BUG=717559,674773
TEST=ash_unittests, browser_tests, manually testing login, ChromeVox, and
adding/removing displays
TBR=tsepez@chromium.org for rename

Review-Url: https://codereview.chromium.org/2889673002
Cr-Commit-Position: refs/heads/master@{#472832}
Committed:
https://chromium.googlesource.com/chromium/src/+/788b4fcb1909904cda962cac4a63...
==========

[commit-bot: I haz the power, 2017-05-18 16:16:26]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/788b4fcb1909904cda962cac4a63...