Makes BrowserStatusMonitor remove observers in destructor

chrome/browser/ui/ash/launcher/browser_status_monitor.cc

Index: chrome/browser/ui/ash/launcher/browser_status_monitor.cc
diff --git a/chrome/browser/ui/ash/launcher/browser_status_monitor.cc b/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
index 738e2fbdfdbcc2d2b63aec04b752cb6f4e411466..cedc21a5ed5adde5ba782c6d1a2d7d018c2c394c 100644
--- a/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
+++ b/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
@@ -27,10 +27,7 @@ BrowserStatusMonitor::BrowserStatusMonitor(
       observed_activation_clients_(this),
       observed_root_windows_(this) {
   DCHECK(launcher_controller_);
-  BrowserList* browser_list =
-      BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
-
-  browser_list->AddObserver(this);
+  BrowserList::AddObserver(this);
 
   // This check needs for win7_aura. Without this, all tests in
   // ChromeLauncherController will fail in win7_aura.
@@ -57,10 +54,14 @@ BrowserStatusMonitor::~BrowserStatusMonitor() {
   if (ash::Shell::HasInstance())
     ash::Shell::GetInstance()->GetScreen()->RemoveObserver(this);
 
-  BrowserList* browser_list =
-      BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
+  BrowserList::RemoveObserver(this);
 
-  browser_list->RemoveObserver(this);
+  BrowserList* brower_list =
+      BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
+  for (BrowserList::const_iterator i = browser_list->begin();
+       i != browser_list->end(); ++i) {
+    OnBrowserRemoved(*i);
+  }
 }
 
 void BrowserStatusMonitor::OnWindowActivated(aura::Window* gained_active,
@@ -86,6 +87,9 @@ void BrowserStatusMonitor::OnWindowDestroyed(aura::Window* window) {
 }
 
 void BrowserStatusMonitor::OnBrowserAdded(Browser* browser) {
+  if (browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
+    return;
+
   browser->tab_strip_model()->AddObserver(this);
 
   if (browser->is_type_popup() && browser->is_app()) {
@@ -99,6 +103,9 @@ void BrowserStatusMonitor::OnBrowserAdded(Browser* browser) {
 }
 
 void BrowserStatusMonitor::OnBrowserRemoved(Browser* browser) {
+  if (browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
+    return;
+
   browser->tab_strip_model()->RemoveObserver(this);
 
   if (browser_to_app_id_map_.find(browser) != browser_to_app_id_map_.end()) {
@@ -141,6 +148,9 @@ void BrowserStatusMonitor::ActiveTabChanged(content::WebContents* old_contents,
   if (old_contents)
     browser = chrome::FindBrowserWithWebContents(old_contents);
 
+  if (browser && browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
+    return;

[Mr4D (OOO till 08-26), 2013/09/09 18:48:45]

Maybe an early exit if !browser since it gets checked twice here?

[sky, 2013/09/09 19:29:28]

I thought we still wanted to invoke UpdateAppAndBrowserState in this case. Maybe
I'm wrong?

[Mr4D (OOO till 08-26), 2013/09/09 19:38:28]

I've meant early exit, but calling UpdateApp.. .

So, yes we want to call. It only felt odd that we are testing twice for browser
and then an additional condition. So testing once and then adding the closure
for the two tests does of course do the same thing, but I got several times the
request to early out, so I suggested the same :).

[sky, 2013/09/09 19:39:54]

I'm going to leave it. If you feel strongly I'll update.

+
   // Update immediately on a tab change.
   if (browser &&
       (TabStripModel::kNoTab !=
@@ -190,6 +200,9 @@ void BrowserStatusMonitor::UpdateAppAndBrowserState(
         ChromeLauncherController::APP_STATE_INACTIVE;
 
     Browser* browser = chrome::FindBrowserWithWebContents(contents);
+    if (browser &&

[Mr4D (OOO till 08-26), 2013/09/09 18:48:45]

If browser is null it would crash later on as well. I would guess that a DCHECK
instead might be a good idea?

[sky, 2013/09/09 19:29:28]

Good point. Added DCHECK.

+        browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
+      return;
     if (browser->tab_strip_model()->GetActiveWebContents() == contents) {
       if (browser->window()->IsActive())
         app_state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;