Replace base::CallbackList with base::ObserverList in CastConfigDelegate

ash/shell.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/shell.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
@@ skipping 458 matching lines @@
   keyboard::KeyboardController::ResetInstance(nullptr);
 }
 
 void Shell::ShowShelf() {
   RootWindowControllerList controllers = GetAllRootWindowControllers();
   for (RootWindowControllerList::iterator iter = controllers.begin();
        iter != controllers.end(); ++iter)
     (*iter)->ShowShelf();
 }
 
+void Shell::HideShelf() {
+  RootWindowControllerList controllers = GetAllRootWindowControllers();
+  for (RootWindowControllerList::iterator iter = controllers.begin();
+       iter != controllers.end(); ++iter) {
+    if ((*iter)->shelf())
+      (*iter)->shelf()->ShutdownStatusAreaWidget();
+  }
+}
+
 void Shell::AddShellObserver(ShellObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void Shell::RemoveShellObserver(ShellObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
 #if defined(OS_CHROMEOS)
 bool Shell::ShouldSaveDisplaySettings() {
@@ skipping 224 matching lines @@
 
 #if defined(OS_CHROMEOS)
   // Destroy the LastWindowClosedLogoutReminder before the
   // LogoutConfirmationController.
   last_window_closed_logout_reminder_.reset();
 
   // Destroy the LogoutConfirmationController before the SystemTrayDelegate.
   logout_confirmation_controller_.reset();
 #endif
 
-  // Destroy SystemTrayDelegate before destroying the status area(s).
+  // Destroy SystemTrayDelegate before destroying the status area(s). Make sure

[oshima, 2016/01/09 01:05:11]

The original comment should go after HideShelf. HideShelf does seem to destroy
status area though. Are you sure it's safe? Or shouldn't it just call
CloseSystemBubble() ?

[jdufault, 2016/01/11 20:59:34]

Calling (*iter)->GetSystemTray()->CloseSystemBubble() doesn't cause the TrayCast
dtor to run. Thinking about it more, this makes since, since TrayCast lives
beyond the shelf popup - it has methods (CreateTrayView, DestroyTrayView, etc)
that control the lifetime for the popup. So hiding the popup itself will not be
enough.

You're right though - this change is destroying the status area before the
SystemTrayDelegate. Though I think this is the correct order, since the status
area is initialized after the delegate. I'm a bit concerned that this will cause
other things to crash, but it seems OK from a cursory overview on my device.
However, this change needs to be merged into m48, which goes stable in two
weeks.

Any thoughts? Instead of this somewhat risky change, I can look into using weak
pointers in TrayCast instead, though that seems like a band-aid fix.

The comment about the deinit order was made in this[1] CL.

1: https://codereview.chromium.org/25111002

+  // to deinitialize the shelf first, as it is initialized after the delegate.
+  HideShelf();
   system_tray_delegate_->Shutdown();
   system_tray_delegate_.reset();
 
   locale_notification_controller_.reset();
 
   // Drag-and-drop must be canceled prior to close all windows.
   drag_drop_controller_.reset();
 
   // Controllers who have WindowObserver added must be deleted
   // before |window_tree_host_manager_| is deleted.
@@ skipping 435 matching lines @@
 
 void Shell::OnWindowActivated(
     aura::client::ActivationChangeObserver::ActivationReason reason,
     aura::Window* gained_active,
     aura::Window* lost_active) {
   if (gained_active)
     target_root_window_ = gained_active->GetRootWindow();
 }
 
 }  // namespace ash