Fixing activation states from the new launcher. Also adding a whole bunch of unit tests for the new…

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

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
index 88638fa32be86abf18049be48edb3e688ec2fb43..9b2de60427687ab7279f17dcc8977c20a21249d7 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
@@ -165,6 +165,7 @@ ChromeLauncherControllerPerApp::ChromeLauncherControllerPerApp(
   }
 
   model_->AddObserver(this);
+  BrowserList::AddObserver(this);
   // TODO(stevenjb): Find a better owner for shell_window_controller_?
   shell_window_controller_.reset(new ShellWindowLauncherController(this));
   app_tab_helper_.reset(new LauncherAppTabHelper(profile_));
@@ -197,6 +198,7 @@ ChromeLauncherControllerPerApp::~ChromeLauncherControllerPerApp() {
   shell_window_controller_.reset();
 
   model_->RemoveObserver(this);
+  BrowserList::RemoveObserver(this);
   for (IDToItemControllerMap::iterator i = id_to_item_controller_map_.begin();
        i != id_to_item_controller_map_.end(); ++i) {
     i->second->OnRemoved();
@@ -285,23 +287,7 @@ void ChromeLauncherControllerPerApp::SetItemStatus(
     if (model_->items()[index].type == ash::TYPE_BROWSER_SHORTCUT)
       return;
   }
-  // Determine the new browser's active state and change if necessary.
-  int browser_index = ash::launcher::GetBrowserItemIndex(*model_);
-  DCHECK(browser_index >= 0);
-  ash::LauncherItem browser_item = model_->items()[browser_index];
-  ash::LauncherItemStatus browser_status = browser_item.status;
-  // See if the active window is a browser.
-  if (chrome::FindBrowserWithWindow(ash::wm::GetActiveWindow())) {
-    browser_status = ash::STATUS_ACTIVE;
-  } else if (!BrowserList::empty()) {
-    browser_status = ash::STATUS_RUNNING;
-  } else {
-    browser_status = ash::STATUS_CLOSED;
-  }
-  if (browser_status != browser_item.status) {
-    browser_item.status = browser_status;
-    model_->Set(browser_index, browser_item);
-  }
+  UpdateBrowserItemStatus();
 }
 
 void ChromeLauncherControllerPerApp::SetItemController(
@@ -333,6 +319,8 @@ void ChromeLauncherControllerPerApp::Pin(ash::LauncherID id) {
   DCHECK(HasItemController(id));
 
   int index = model_->ItemIndexByID(id);
+  DCHECK_GE(index, 0);
+
   ash::LauncherItem item = model_->items()[index];
 
   if (item.type != ash::TYPE_PLATFORM_APP)
@@ -351,6 +339,7 @@ void ChromeLauncherControllerPerApp::Unpin(ash::LauncherID id) {
   LauncherItemController* controller = id_to_item_controller_map_[id];
   if (controller->type() == LauncherItemController::TYPE_APP) {
     int index = model_->ItemIndexByID(id);
+    DCHECK_GE(index, 0);
     ash::LauncherItem item = model_->items()[index];
     item.type = ash::TYPE_PLATFORM_APP;
     model_->Set(index, item);
@@ -363,6 +352,8 @@ void ChromeLauncherControllerPerApp::Unpin(ash::LauncherID id) {
 
 bool ChromeLauncherControllerPerApp::IsPinned(ash::LauncherID id) {
   int index = model_->ItemIndexByID(id);
+  if (index < 0)
+    return false;
   ash::LauncherItemType type = model_->items()[index].type;
   return type == ash::TYPE_APP_SHORTCUT;
 }
@@ -457,8 +448,14 @@ void ChromeLauncherControllerPerApp::ActivateApp(const std::string& app_id,
     return;
   }
 
-  // Otherwise launch the app.
-  LaunchApp(app_id, event_flags);
+  // Create a temporary application launcher item and use it to see if there are
+  // running instances.
+  scoped_ptr<AppShortcutLauncherItemController> app_controller(
+      new AppShortcutLauncherItemController(app_id, this));
+  if (!app_controller->GetRunningApplications().empty())
+    app_controller->Activate();
+  else
+    LaunchApp(app_id, event_flags);
 }
 
 extensions::ExtensionPrefs::LaunchType
@@ -513,6 +510,8 @@ void ChromeLauncherControllerPerApp::SetAppImage(
       continue;
 
     int index = model_->ItemIndexByID(i->first);
+    if (index == -1)
+      continue;
     ash::LauncherItem item = model_->items()[index];
     item.image = image;
     model_->Set(index, item);
@@ -694,8 +693,12 @@ void ChromeLauncherControllerPerApp::UpdateAppState(
       RemoveTabFromRunningApp(contents, last_app_id);
   }
 
-  if (app_id.empty())
+  if (app_id.empty()) {
+    // Even if there is no application running, we should update the activation
+    // state of the associated browser.
+    UpdateBrowserItemStatus();
     return;
+  }
 
   web_contents_to_app_id_[contents] = app_id;
 
@@ -1017,6 +1020,13 @@ gfx::Image ChromeLauncherControllerPerApp::GetAppListIcon(
   return favicon_tab_helper->GetFavicon();
 }
 
+void ChromeLauncherControllerPerApp::OnBrowserRemoved(Browser* browser) {
+  // When called by a unit test it is possible that there is no shell.
+  // In that case, the following function should not get called.
+  if (ash::Shell::HasInstance())
+    UpdateBrowserItemStatus();
+}
+
 ash::LauncherID ChromeLauncherControllerPerApp::CreateAppShortcutLauncherItem(
     const std::string& app_id,
     int index) {
@@ -1043,6 +1053,27 @@ ChromeLauncherControllerPerApp::GetAppIdFromLauncherIdForTest(
   return id_to_item_controller_map_[id]->app_id();
 }
 
+void ChromeLauncherControllerPerApp::UpdateBrowserItemStatus() {
+  // Determine the new browser's active state and change if necessary.
+  int browser_index = ash::launcher::GetBrowserItemIndex(*model_);
+  DCHECK(browser_index >= 0);
+  ash::LauncherItem browser_item = model_->items()[browser_index];
+  ash::LauncherItemStatus browser_status = browser_item.status;
+  // See if the active window is a browser.
+  aura::Window* window = ash::wm::GetActiveWindow();
+  if (window && chrome::FindBrowserWithWindow(window)) {
+    browser_status = ash::STATUS_ACTIVE;
+  } else if (!BrowserList::empty()) {
+    browser_status = ash::STATUS_RUNNING;
+  } else {
+    browser_status = ash::STATUS_CLOSED;
+  }
+  if (browser_status != browser_item.status) {
+    browser_item.status = browser_status;
+    model_->Set(browser_index, browser_item);
+  }
+}
+
 Profile* ChromeLauncherControllerPerApp::GetProfileForNewWindows() {
   return ProfileManager::GetDefaultProfileOrOffTheRecord();
 }