Close notification windows while message loop is still active

chrome/browser/chromeos/system/ash_system_tray_delegate.cc

Index: chrome/browser/chromeos/system/ash_system_tray_delegate.cc
diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
index f1f5f260e6ed6fc319426becc12eeaa1f5a751bb..fdd9bb5d6df93f74b9629877437e871f8fa8bd78 100644
--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
@@ -604,6 +604,10 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
     BaseLoginDisplayHost::default_host()->OpenProxySettings();
   }
 
+  virtual void OnTrayDestroyed() OVERRIDE {
+    tray_ = NULL;

[Ben Goodger (Google), 2012/04/12 23:33:56]

rather than NULL checking everything in this file, can we just remove this
object as an observer from the various things it observes?

[oshima, 2012/04/12 23:50:27]

Yes, I thought about it. I still need NULL check for
Shell::GetInstance()->tray_delegate() (but not all places),
and I felt it's cleaner to do NULL check in one place instead of
doing this in multiple places.

This NULL check is necessary only if we close tray/widgets before
message loop is destroyed, so another option is to exclude it for now.

Let me know which way you prefer.

+  }
+
  private:
   // Returns the last active browser. If there is no such browser, creates a new
   // browser window with an empty tab and returns it.
@@ -625,22 +629,19 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
   void UpdateClockType(PrefService* service) {
     clock_type_ = service->GetBoolean(prefs::kUse24HourClock) ?
         base::k24HourClock : base::k12HourClock;
-    ash::ClockObserver* observer =
-        ash::Shell::GetInstance()->tray()->clock_observer();
+    ash::ClockObserver* observer = tray_ ? tray_->clock_observer() : NULL;
     if (observer)
       observer->OnDateFormatChanged();
   }
 
   void NotifyRefreshClock() {
-    ash::ClockObserver* observer =
-        ash::Shell::GetInstance()->tray()->clock_observer();
+    ash::ClockObserver* observer = tray_ ? tray_->clock_observer() : NULL;
     if (observer)
       observer->Refresh();
   }
 
   void NotifyRefreshNetwork() {
-    ash::NetworkObserver* observer =
-        ash::Shell::GetInstance()->tray()->network_observer();
+    ash::NetworkObserver* observer = tray_ ? tray_->network_observer() : NULL;
     if (observer) {
       NetworkLibrary* crosnet = CrosLibrary::Get()->GetNetworkLibrary();
       ash::NetworkIconInfo info;
@@ -653,14 +654,13 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
 
   void NotifyRefreshBluetooth() {
     ash::BluetoothObserver* observer =
-        ash::Shell::GetInstance()->tray()->bluetooth_observer();
+        tray_ ? tray_->bluetooth_observer() : NULL;
     if (observer)
       observer->OnBluetoothRefresh();
   }
 
   void NotifyRefreshIME() {
-    ash::IMEObserver* observer =
-        ash::Shell::GetInstance()->tray()->ime_observer();
+    ash::IMEObserver* observer = tray_ ? tray_->ime_observer() : NULL;
     if (observer)
       observer->OnIMERefresh();
   }
@@ -687,21 +687,24 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
 
   // Overridden from AudioHandler::VolumeObserver.
   virtual void OnVolumeChanged() OVERRIDE {
-    float level = AudioHandler::GetInstance()->GetVolumePercent() / 100.f;
-    ash::Shell::GetInstance()->tray()->audio_observer()->
-        OnVolumeChanged(level);
+    if (tray_) {
+      float level = AudioHandler::GetInstance()->GetVolumePercent() / 100.f;
+      tray_->audio_observer()->OnVolumeChanged(level);
+    }
   }
 
   // Overridden from PowerManagerClient::Observer.
   virtual void BrightnessChanged(int level, bool user_initiated) OVERRIDE {
-    ash::Shell::GetInstance()->tray()->brightness_observer()->
-        OnBrightnessChanged(static_cast<double>(level), user_initiated);
+    if (tray_) {
+      tray_->brightness_observer()->
+          OnBrightnessChanged(static_cast<double>(level), user_initiated);
+    }
   }
 
   virtual void PowerChanged(const PowerSupplyStatus& power_status) OVERRIDE {
     power_supply_status_ = power_status;
     ash::PowerStatusObserver* observer =
-        ash::Shell::GetInstance()->tray()->power_status_observer();
+        tray_ ? tray_->power_status_observer() : NULL;
     if (observer)
       observer->OnPowerStatusChanged(power_status);
   }
@@ -712,12 +715,14 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
 
   virtual void LockScreen() OVERRIDE {
     screen_locked_ = true;
-    tray_->UpdateAfterLoginStatusChange(GetUserLoginStatus());
+    if (tray_)
+      tray_->UpdateAfterLoginStatusChange(GetUserLoginStatus());
   }
 
   virtual void UnlockScreen() OVERRIDE {
     screen_locked_ = false;
-    tray_->UpdateAfterLoginStatusChange(GetUserLoginStatus());
+    if (tray_)
+      tray_->UpdateAfterLoginStatusChange(GetUserLoginStatus());
   }
 
   virtual void UnlockScreenFailed() OVERRIDE {
@@ -754,8 +759,10 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
   virtual void OnNetworkManagerChanged(NetworkLibrary* crosnet) OVERRIDE {
     RefreshNetworkObserver(crosnet);
     RefreshNetworkDeviceObserver(crosnet);
-    data_promo_notification_->ShowOptionalMobileDataPromoNotification(crosnet,
-        tray_, this);
+    if (tray_) {
+      data_promo_notification_->ShowOptionalMobileDataPromoNotification(
+          crosnet, tray_, this);
+    }
 
     NotifyRefreshNetwork();
   }
@@ -777,12 +784,12 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
                        const content::NotificationDetails& details) OVERRIDE {
     switch (type) {
       case chrome::NOTIFICATION_LOGIN_USER_CHANGED: {
-        tray_->UpdateAfterLoginStatusChange(GetUserLoginStatus());
+        if (tray_)
+          tray_->UpdateAfterLoginStatusChange(GetUserLoginStatus());
         break;
       }
       case chrome::NOTIFICATION_UPGRADE_RECOMMENDED: {
-        ash::UpdateObserver* observer =
-            ash::Shell::GetInstance()->tray()->update_observer();
+        ash::UpdateObserver* observer = tray_ ? tray_->update_observer() : NULL;
         if (observer)
           observer->OnUpdateRecommended();
         break;
@@ -791,8 +798,7 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
         // This notification is also sent on login screen when user avatar
         // is loaded from file.
         if (GetUserLoginStatus() != ash::user::LOGGED_IN_NONE) {
-          ash::UserObserver* observer =
-              ash::Shell::GetInstance()->tray()->user_observer();
+          ash::UserObserver* observer = tray_ ? tray_->user_observer() : NULL;
           if (observer)
             observer->OnUserUpdate();
         }
@@ -808,7 +814,7 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
               service->GetInteger(prefs::kLanguageXkbRemapSearchKeyTo);
         } else if (pref == prefs::kSpokenFeedbackEnabled) {
           ash::AccessibilityObserver* observer =
-              ash::Shell::GetInstance()->tray()->accessibility_observer();
+              tray_ ? tray_->accessibility_observer() : NULL;
           if (observer) {
             observer->OnAccessibilityModeChanged(
                 service->GetBoolean(prefs::kSpokenFeedbackEnabled),
@@ -901,7 +907,7 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
       id = IDS_STATUSBAR_CAPS_LOCK_ENABLED_PRESS_SEARCH;
 
     ash::CapsLockObserver* observer =
-      ash::Shell::GetInstance()->tray()->caps_lock_observer();
+        tray_ ? tray_->caps_lock_observer() : NULL;
     if (observer)
       observer->OnCapsLockChanged(enabled, id);
   }