cros: Cleanup ShelfWidget::DelegateView shutdown

Make it use DeleteDelegate() to be more consistent with
StatusAreaWidgetDelegate. Defer closing the widget until all the
top-level windows are closed.

Bug: 628655
Test: ash_unittests
Change-Id: I066b45dab9c848b8f990f7fb51f0170e5e66a861
Reviewed-on: https://chromium-review.googlesource.com/907390
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540550}

ash/root_window_controller.cc

@@ -471,6 +471,8 @@ void RootWindowController::Shutdown() {
 
 void RootWindowController::CloseChildWindows() {
   // NOTE: this may be called multiple times.
+  // TODO: Only call this once. The multiple calls cause complexity in the
+  // dependent Shelf and ShelfWidget code.
 
   // Deactivate keyboard container before closing child windows and shutting
   // down associated layout managers.
@@ -505,6 +507,7 @@ void RootWindowController::CloseChildWindows() {
     while (!toplevel_windows.windows().empty())
       delete toplevel_windows.Pop();
   }
+
   // And then remove the containers.
   while (!root->children().empty()) {
     aura::Window* child = root->children()[0];
@@ -514,6 +517,8 @@ void RootWindowController::CloseChildWindows() {
       root->RemoveChild(child);
   }
 
+  // Removing the containers destroys ShelfLayoutManager. ShelfWidget outlives
+  // ShelfLayoutManager because ShelfLayoutManager holds a pointer to it.
   shelf_->DestroyShelfWidget();
 
   aura::client::SetDragDropClient(GetRootWindow(), nullptr);

ash/shelf/shelf.cc

@@ -90,6 +90,7 @@ void Shelf::CreateShelfWidget(aura::Window* root) {
 }
 
 void Shelf::ShutdownShelfWidget() {
+  // May be called multiple times during shutdown.
   if (shelf_widget_)
     shelf_widget_->Shutdown();
 }

ash/shelf/shelf_widget.cc

@@ -73,7 +73,8 @@ class ShelfWidget::DelegateView : public views::WidgetDelegate,
     default_last_focusable_child_ = default_last_focusable_child;
   }
 
-  // views::WidgetDelegateView:
+  // views::WidgetDelegate:
+  void DeleteDelegate() override { delete this; }
   views::Widget* GetWidget() override { return View::GetWidget(); }
   const views::Widget* GetWidget() const override { return View::GetWidget(); }
 
@@ -106,6 +107,8 @@ ShelfWidget::DelegateView::DelegateView(ShelfWidget* shelf_widget)
       focus_cycler_(nullptr),
       opaque_background_(ui::LAYER_SOLID_COLOR) {
   DCHECK(shelf_widget_);
+  set_owned_by_client();  // Deleted by DeleteDelegate().
+
   SetLayoutManager(std::make_unique<views::FillLayout>());
   set_allow_deactivate_on_esc(true);
   opaque_background_.SetBounds(GetLocalBounds());
@@ -216,12 +219,36 @@ ShelfWidget::ShelfWidget(aura::Window* shelf_container, Shelf* shelf)
 
 ShelfWidget::~ShelfWidget() {
   // Must call Shutdown() before destruction.
-  DCHECK(!status_area_widget_);
+  DCHECK(did_shutdown_);
+}
+
+void ShelfWidget::Shutdown() {
+  if (did_shutdown_)
+    return;
+
+  // Shutting down the status area widget may cause some widgets (e.g. bubbles)
+  // to close, so uninstall the ShelfLayoutManager event filters first. Don't
+  // reset the pointer until later because other widgets (e.g. app list) may
+  // access it later in shutdown.
+  if (shelf_layout_manager_)
+    shelf_layout_manager_->PrepareForShutdown();
+
+  if (status_area_widget_) {
+    background_animator_.RemoveObserver(status_area_widget_.get());
+    Shell::Get()->focus_cycler()->RemoveWidget(status_area_widget_.get());
+    status_area_widget_.reset();
+  }
+
+  // Don't need to update the shelf background during shutdown.
   background_animator_.RemoveObserver(delegate_view_);
   background_animator_.RemoveObserver(this);
+
+  // Don't need to observe focus/activation during shutdown.
   Shell::Get()->focus_cycler()->RemoveWidget(this);
   SetFocusCycler(nullptr);
   RemoveObserver(this);
+
+  did_shutdown_ = true;
 }
 
 void ShelfWidget::CreateStatusAreaWidget(aura::Window* status_container) {
@@ -290,23 +317,6 @@ FocusCycler* ShelfWidget::GetFocusCycler() {
   return delegate_view_->focus_cycler();
 }
 
-void ShelfWidget::Shutdown() {
-  // Shutting down the status area widget may cause some widgets (e.g. bubbles)
-  // to close, so uninstall the ShelfLayoutManager event filters first. Don't
-  // reset the pointer until later because other widgets (e.g. app list) may
-  // access it later in shutdown.
-  if (shelf_layout_manager_)
-    shelf_layout_manager_->PrepareForShutdown();
-
-  if (status_area_widget_) {
-    background_animator_.RemoveObserver(status_area_widget_.get());
-    Shell::Get()->focus_cycler()->RemoveWidget(status_area_widget_.get());
-    status_area_widget_.reset();
-  }
-
-  CloseNow();
-}
-
 void ShelfWidget::UpdateIconPositionForPanel(aura::Window* panel) {
   ShelfID id = ShelfID::Deserialize(panel->GetProperty(kShelfIDKey));
   if (id.IsNull())

ash/shelf/shelf_widget.h

@@ -143,6 +143,8 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
 
   ScopedSessionObserver scoped_session_observer_;
 
+  bool did_shutdown_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(ShelfWidget);
 };
 

ash/shell.cc

@@ -721,8 +721,6 @@ Shell::~Shell() {
 
   toast_manager_.reset();
 
-  for (aura::Window* root : GetAllRootWindows())
-    Shelf::ForWindow(root)->ShutdownShelfWidget();
   tray_bluetooth_helper_.reset();
 
   // Accesses root window containers.