chromeos: Refactor shelf to create ShelfView earlier in startup

ash/shelf/shelf_widget.cc

Index: ash/shelf/shelf_widget.cc
diff --git a/ash/shelf/shelf_widget.cc b/ash/shelf/shelf_widget.cc
index c8419e393b1aa94d37f2cd8a939387414d4d39e4..37df52910c803a8bc9a591558b5156220e999165 100644
--- a/ash/shelf/shelf_widget.cc
+++ b/ash/shelf/shelf_widget.cc
@@ -126,14 +126,13 @@ ShelfWidget::ShelfWidget(WmWindow* shelf_container, WmShelf* wm_shelf)
     : wm_shelf_(wm_shelf),
       status_area_widget_(nullptr),
       delegate_view_(new DelegateView(this)),
-      shelf_view_(nullptr),
+      shelf_view_(new ShelfView(Shell::Get()->shelf_model(), wm_shelf_, this)),
       background_animator_(SHELF_BACKGROUND_DEFAULT,
                            wm_shelf_,
                            Shell::Get()->wallpaper_controller()),
       activating_as_fallback_(false) {
+  DCHECK(shelf_container);
   DCHECK(wm_shelf_);
-  background_animator_.AddObserver(this);
-  background_animator_.AddObserver(delegate_view_);
 
   views::Widget::InitParams params(
       views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
@@ -151,6 +150,11 @@ ShelfWidget::ShelfWidget(WmWindow* shelf_container, WmShelf* wm_shelf)
   SetContentsView(delegate_view_);
   delegate_view_->SetParentLayer(GetLayer());
 
+  // The shelf view observes the shelf model and creates icons as items are
+  // added to the model.
+  shelf_view_->Init();
+  GetContentsView()->AddChildView(shelf_view_);
+
   shelf_layout_manager_ = new ShelfLayoutManager(this, wm_shelf_);
   shelf_layout_manager_->AddObserver(this);
   shelf_container->aura_window()->SetLayoutManager(shelf_layout_manager_);
@@ -159,24 +163,28 @@ ShelfWidget::ShelfWidget(WmWindow* shelf_container, WmShelf* wm_shelf)
       AnimationChangeType::IMMEDIATE);
 
   views::Widget::AddObserver(this);
+
+  // Calls back into |this| and depends on |shelf_view_|.
+  background_animator_.AddObserver(this);
+  background_animator_.AddObserver(delegate_view_);
 }
 
 ShelfWidget::~ShelfWidget() {
   // Must call Shutdown() before destruction.
   DCHECK(!status_area_widget_);
+  background_animator_.RemoveObserver(delegate_view_);
+  background_animator_.RemoveObserver(this);
   Shell::Get()->focus_cycler()->RemoveWidget(this);
   SetFocusCycler(nullptr);
   RemoveObserver(this);
-  background_animator_.RemoveObserver(delegate_view_);
-  background_animator_.RemoveObserver(this);
 }
 
 void ShelfWidget::CreateStatusAreaWidget(WmWindow* status_container) {
   DCHECK(status_container);
   DCHECK(!status_area_widget_);
-  // TODO(jamescook): Move ownership to RootWindowController.
   status_area_widget_ = new StatusAreaWidget(status_container, wm_shelf_);
   status_area_widget_->CreateTrayViews();
+  // TODO(jamescook): Is the session check needed?

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

nit: address now? (so many TODOs!)

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

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

Done. Wasn't needed, removed.

   if (Shell::Get()->session_controller()->IsActiveUserSessionStarted())
     status_area_widget_->Show();
   Shell::Get()->focus_cycler()->AddWidget(status_area_widget_);
@@ -221,38 +229,19 @@ void ShelfWidget::OnShelfAlignmentChanged() {
   delegate_view_->SchedulePaint();
 }
 
-ShelfView* ShelfWidget::CreateShelfView() {
-  DCHECK(!shelf_view_);
-  shelf_view_ = new ShelfView(Shell::Get()->shelf_model(), wm_shelf_, this);
-  shelf_view_->Init();
-  GetContentsView()->AddChildView(shelf_view_);
-  return shelf_view_;
-}
-
 void ShelfWidget::PostCreateShelf() {
   SetFocusCycler(Shell::Get()->focus_cycler());
 
   // Ensure the newly created |shelf_| gets current values.
   background_animator_.NotifyObserver(this);
 
-  // TODO(jamescook): The IsActiveUserSessionStarted() check may not be needed
-  // because the shelf is only created after the first user session is active.
-  // The ShelfView seems to always be visible after login. At the lock screen
-  // the shelf is hidden because its container is hidden. During auto-hide it is
-  // hidden because ShelfWidget is transparent. Some of the ShelfView visibility
-  // code could be simplified. http://crbug.com/674773
-  // TODO(jdufualt|jamescook): Remove CHECK and shelf_view_->SetVisible call in
-  // m60 or beyond (see above TODO).
-  CHECK(Shell::Get()->session_controller()->IsActiveUserSessionStarted());
-  shelf_view_->SetVisible(
-      Shell::Get()->session_controller()->IsActiveUserSessionStarted());
   shelf_layout_manager_->LayoutShelf();
   shelf_layout_manager_->UpdateAutoHideState();
   Show();
 }
 
 bool ShelfWidget::IsShelfVisible() const {

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

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?)

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

Good catch! Removed.

-  return shelf_view_ && shelf_view_->visible();
+  return shelf_view_->visible();
 }
 
 bool ShelfWidget::IsShowingAppList() const {
@@ -260,11 +249,11 @@ bool ShelfWidget::IsShowingAppList() const {
 }
 
 bool ShelfWidget::IsShowingContextMenu() const {
-  return shelf_view_ && shelf_view_->IsShowingMenu();
+  return shelf_view_->IsShowingMenu();
 }
 
 bool ShelfWidget::IsShowingOverflowBubble() const {
-  return shelf_view_ && shelf_view_->IsShowingOverflowBubble();
+  return shelf_view_->IsShowingOverflowBubble();
 }
 
 void ShelfWidget::SetFocusCycler(FocusCycler* focus_cycler) {
@@ -298,7 +287,7 @@ void ShelfWidget::Shutdown() {
 void ShelfWidget::UpdateIconPositionForPanel(WmWindow* panel) {
   ShelfID id =
       ShelfID::Deserialize(panel->aura_window()->GetProperty(kShelfIDKey));
-  if (!shelf_view_ || id.IsNull())
+  if (id.IsNull())
     return;
 
   WmWindow* shelf_window = WmWindow::Get(this->GetNativeWindow());
@@ -310,9 +299,7 @@ void ShelfWidget::UpdateIconPositionForPanel(WmWindow* panel) {
 gfx::Rect ShelfWidget::GetScreenBoundsOfItemIconForWindow(WmWindow* window) {
   ShelfID id =
       ShelfID::Deserialize(window->aura_window()->GetProperty(kShelfIDKey));
-  // Window animations can be triggered during session restore before the shelf
-  // view is created. In that case, return default empty bounds.
-  if (!shelf_view_ || id.IsNull())
+  if (id.IsNull())
     return gfx::Rect();
 
   gfx::Rect bounds(shelf_view_->GetIdealBoundsOfItemIcon(id));
@@ -324,7 +311,7 @@ gfx::Rect ShelfWidget::GetScreenBoundsOfItemIconForWindow(WmWindow* window) {
 }
 
 AppListButton* ShelfWidget::GetAppListButton() const {
-  return shelf_view_ ? shelf_view_->GetAppListButton() : nullptr;
+  return shelf_view_->GetAppListButton();
 }
 
 app_list::ApplicationDragAndDropHost*
@@ -342,8 +329,7 @@ void ShelfWidget::OnWidgetActivationChanged(views::Widget* widget,
 }
 
 void ShelfWidget::UpdateShelfItemBackground(SkColor color) {
-  if (shelf_view_)
-    shelf_view_->UpdateShelfItemBackground(color);
+  shelf_view_->UpdateShelfItemBackground(color);
 }
 
 void ShelfWidget::WillDeleteShelfLayoutManager() {