chromeos: Refactor shelf to create ShelfView earlier in startup

ash/shelf/shelf_widget_unittest.cc

Index: ash/shelf/shelf_widget_unittest.cc
diff --git a/ash/shelf/shelf_widget_unittest.cc b/ash/shelf/shelf_widget_unittest.cc
index 7cdac60ef9c5b6fb1b7dac06c4e311d92a9cf832..f10ff5182954f0a14e745bbe31595ade1534387c 100644
--- a/ash/shelf/shelf_widget_unittest.cc
+++ b/ash/shelf/shelf_widget_unittest.cc
@@ -15,6 +15,7 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/test/ash_test_helper.h"
 #include "ash/test/shelf_view_test_api.h"
+#include "ash/test/test_shell_delegate.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm_window.h"
 #include "ui/aura/window_event_dispatcher.h"
@@ -24,7 +25,6 @@
 #include "ui/views/widget/widget.h"
 
 namespace ash {
-
 namespace {
 
 ShelfWidget* GetShelfWidget() {
@@ -35,10 +35,6 @@ ShelfLayoutManager* GetShelfLayoutManager() {
   return GetShelfWidget()->shelf_layout_manager();
 }
 
-}  // namespace
-
-using ShelfWidgetTest = test::AshTestBase;
-
 void TestLauncherAlignment(WmWindow* root,
                            ShelfAlignment alignment,
                            const gfx::Rect& expected) {
@@ -47,6 +43,8 @@ void TestLauncherAlignment(WmWindow* root,
             root->GetDisplayNearestWindow().work_area().ToString());
 }
 
+using ShelfWidgetTest = test::AshTestBase;
+
 TEST_F(ShelfWidgetTest, TestAlignment) {
   UpdateDisplay("400x400");
   {
@@ -323,47 +321,20 @@ TEST_F(ShelfWidgetTest, HiddenShelfHitTestTouch) {
   }
 }
 
-namespace {
-
-// A ShellObserver that sets the shelf alignment and auto hide behavior when the
-// shelf is created, to simulate ChromeLauncherController's behavior.
-class ShelfInitializer : public ShellObserver {
- public:
-  ShelfInitializer(ShelfAlignment alignment, ShelfAutoHideBehavior auto_hide)
-      : alignment_(alignment), auto_hide_(auto_hide) {
-    Shell::Get()->AddShellObserver(this);
-  }
-  ~ShelfInitializer() override { Shell::Get()->RemoveShellObserver(this); }
-
-  // ShellObserver:
-  void OnShelfCreatedForRootWindow(WmWindow* root_window) override {
-    WmShelf* shelf = root_window->GetRootWindowController()->GetShelf();
-    shelf->SetAlignment(alignment_);
-    shelf->SetAutoHideBehavior(auto_hide_);
-    shelf->UpdateVisibilityState();
-  }
-
- private:
-  ShelfAlignment alignment_;
-  ShelfAutoHideBehavior auto_hide_;
-
-  DISALLOW_COPY_AND_ASSIGN(ShelfInitializer);
-};
-
-class ShelfWidgetTestWithInitializer : public ShelfWidgetTest {
+class ShelfWidgetAfterLoginTest : public test::AshTestBase {
  public:
-  ShelfWidgetTestWithInitializer() { set_start_session(false); }
-  ~ShelfWidgetTestWithInitializer() override {}
-
-  void TestCreateShelfWithInitialValues(
-      ShelfAlignment initial_alignment,
-      ShelfAutoHideBehavior initial_auto_hide_behavior,
-      ShelfVisibilityState expected_shelf_visibility_state,
-      ShelfAutoHideState expected_shelf_auto_hide_state) {
-    ShelfInitializer initializer(initial_alignment, initial_auto_hide_behavior);
+  ShelfWidgetAfterLoginTest() { set_start_session(false); }
+  ~ShelfWidgetAfterLoginTest() override = default;
+
+  void TestShelf(ShelfAlignment alignment,
+                 ShelfAutoHideBehavior auto_hide_behavior,
+                 ShelfVisibilityState expected_shelf_visibility_state,
+                 ShelfAutoHideState expected_shelf_auto_hide_state) {
+    // Simulate login.
     SetUserLoggedIn(true);
     SetSessionStarted(true);
 
+    // Shelf components are created.

[msw, 2017/05/16 22:42:00]

Should this be disabling TestShellDelegate's ShelfInitializer and ensuring the
shelf has the expected initial values?

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

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!).

     ShelfWidget* shelf_widget = GetShelfWidget();
     ASSERT_NE(nullptr, shelf_widget);
     WmShelf* shelf = GetPrimaryShelf();
@@ -372,8 +343,14 @@ class ShelfWidgetTestWithInitializer : public ShelfWidgetTest {
         shelf_widget->shelf_layout_manager();
     ASSERT_NE(nullptr, shelf_layout_manager);
 
-    EXPECT_EQ(initial_alignment, shelf->alignment());
-    EXPECT_EQ(initial_auto_hide_behavior, shelf->auto_hide_behavior());
+    // Simulate shelf settings being applied from profile prefs.
+    shelf->SetAlignment(alignment);
+    shelf->SetAutoHideBehavior(auto_hide_behavior);
+    shelf->UpdateVisibilityState();
+
+    // Settings are applied correctly.

[msw, 2017/05/16 22:42:00]

nit: "Ensure settings are applied correctly."

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

Done.

+    EXPECT_EQ(alignment, shelf->alignment());
+    EXPECT_EQ(auto_hide_behavior, shelf->auto_hide_behavior());
     EXPECT_EQ(expected_shelf_visibility_state,
               shelf_layout_manager->visibility_state());
     EXPECT_EQ(expected_shelf_auto_hide_state,
@@ -381,37 +358,32 @@ class ShelfWidgetTestWithInitializer : public ShelfWidgetTest {
   }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(ShelfWidgetTestWithInitializer);
+  DISALLOW_COPY_AND_ASSIGN(ShelfWidgetAfterLoginTest);
 };
 
-}  // namespace
-
-TEST_F(ShelfWidgetTestWithInitializer, CreateAutoHideAlwaysShelf) {
+TEST_F(ShelfWidgetAfterLoginTest, CreateAutoHideAlwaysShelf) {
   // The actual auto hide state is shown because there are no open windows.
-  TestCreateShelfWithInitialValues(SHELF_ALIGNMENT_BOTTOM,
-                                   SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS,
-                                   SHELF_AUTO_HIDE, SHELF_AUTO_HIDE_SHOWN);
+  TestShelf(SHELF_ALIGNMENT_BOTTOM, SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS,
+            SHELF_AUTO_HIDE, SHELF_AUTO_HIDE_SHOWN);
 }
 
-TEST_F(ShelfWidgetTestWithInitializer, CreateAutoHideNeverShelf) {
+TEST_F(ShelfWidgetAfterLoginTest, CreateAutoHideNeverShelf) {
   // The auto hide state 'HIDDEN' is returned for any non-auto-hide behavior.
-  TestCreateShelfWithInitialValues(SHELF_ALIGNMENT_LEFT,
-                                   SHELF_AUTO_HIDE_BEHAVIOR_NEVER,
-                                   SHELF_VISIBLE, SHELF_AUTO_HIDE_HIDDEN);
+  TestShelf(SHELF_ALIGNMENT_LEFT, SHELF_AUTO_HIDE_BEHAVIOR_NEVER, SHELF_VISIBLE,
+            SHELF_AUTO_HIDE_HIDDEN);
 }
 
-TEST_F(ShelfWidgetTestWithInitializer, CreateAutoHideAlwaysHideShelf) {
+TEST_F(ShelfWidgetAfterLoginTest, CreateAutoHideAlwaysHideShelf) {
   // The auto hide state 'HIDDEN' is returned for any non-auto-hide behavior.
-  TestCreateShelfWithInitialValues(SHELF_ALIGNMENT_RIGHT,
-                                   SHELF_AUTO_HIDE_ALWAYS_HIDDEN, SHELF_HIDDEN,
-                                   SHELF_AUTO_HIDE_HIDDEN);
+  TestShelf(SHELF_ALIGNMENT_RIGHT, SHELF_AUTO_HIDE_ALWAYS_HIDDEN, SHELF_HIDDEN,
+            SHELF_AUTO_HIDE_HIDDEN);
 }
 
-TEST_F(ShelfWidgetTestWithInitializer, CreateLockedShelf) {
+TEST_F(ShelfWidgetAfterLoginTest, CreateLockedShelf) {
   // The auto hide state 'HIDDEN' is returned for any non-auto-hide behavior.
-  TestCreateShelfWithInitialValues(SHELF_ALIGNMENT_BOTTOM_LOCKED,
-                                   SHELF_AUTO_HIDE_BEHAVIOR_NEVER,
-                                   SHELF_VISIBLE, SHELF_AUTO_HIDE_HIDDEN);
+  TestShelf(SHELF_ALIGNMENT_BOTTOM_LOCKED, SHELF_AUTO_HIDE_BEHAVIOR_NEVER,
+            SHELF_VISIBLE, SHELF_AUTO_HIDE_HIDDEN);
 }
 
+}  // namespace
 }  // namespace ash