Show settings browser windows as a separate launcher item

This also fixes an issue where the shelf id for browser windows was
getting set directly by BrowserStatusMonitor regardless of whether the
browser window was represented by BrowserShortcutLauncherItemController,
causing strange shelf behavior.

BUG=359816

Review URL: https://codereview.chromium.org/237693003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@264874 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 401 insertions, 94 deletions

diff --git a/ash/resources/ash_resources.grd b/ash/resources/ash_resources.grd
index 4fbf0b8..1201745 100644
--- a/ash/resources/ash_resources.grd
+++ b/ash/resources/ash_resources.grd
@@ -26,6 +26,7 @@
       <structure type="chrome_scaled_image" name="IDR_ASH_SHELF_OVERFLOW" file="common/shelf/shelf_overflow.png" />
       <structure type="chrome_scaled_image" name="IDR_ASH_SHELF_UNDERLINE_ACTIVE" file="common/shelf/shelf_underline_active.png" />
       <structure type="chrome_scaled_image" name="IDR_ASH_SHELF_UNDERLINE_RUNNING" file="common/shelf/shelf_underline_running.png" />
+      <structure type="chrome_scaled_image" name="IDR_ASH_SHELF_ICON_SETTINGS" file="common/shelf/settings_app_icon.png" />
       <structure type="chrome_scaled_image" name="IDR_ASH_SHELF_ICON_TASK_MANAGER" file="common/shelf/task_manager.png" />
       <structure type="chrome_scaled_image" name="IDR_AURA_MULTI_WINDOW_RESIZE_H" file="common/multi_window_resize_horizontal.png" />
       <structure type="chrome_scaled_image" name="IDR_AURA_MULTI_WINDOW_RESIZE_V" file="common/multi_window_resize_vertical.png" />
diff --git a/ash/resources/default_100_percent/common/shelf/settings_app_icon.png b/ash/resources/default_100_percent/common/shelf/settings_app_icon.png
new file mode 100644
index 0000000..9682a9b
--- /dev/null
+++ b/ash/resources/default_100_percent/common/shelf/settings_app_icon.png
diff --git a/ash/resources/default_200_percent/common/shelf/settings_app_icon.png b/ash/resources/default_200_percent/common/shelf/settings_app_icon.png
new file mode 100644
index 0000000..716b9cf
--- /dev/null
+++ b/ash/resources/default_200_percent/common/shelf/settings_app_icon.png
diff --git a/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc
index edc9d2c..d35a1f8 100644
--- a/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc
+++ b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc
@@ -22,10 +22,12 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/url_constants.h"
 #include "grit/ash_resources.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
@@ -98,6 +100,16 @@ void BrowserShortcutLauncherItemController::UpdateBrowserItemState() {
   }
 }
 
+void BrowserShortcutLauncherItemController::SetShelfIDForBrowserWindowContents(
+    Browser* browser,
+    content::WebContents* web_contents) {
+  if (!IsBrowserRepresentedInBrowserList(browser))
+    return;
+  ash::SetShelfIDForWindow(
+      launcher_controller()->GetShelfIDForWebContents(web_contents),
+      browser->window()->GetNativeWindow());
+}
+
 bool BrowserShortcutLauncherItemController::IsOpen() const {
   const BrowserList* ash_browser_list =
       BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
@@ -317,13 +329,23 @@ void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() {
 
 bool BrowserShortcutLauncherItemController::IsBrowserRepresentedInBrowserList(
     Browser* browser) {
-  return (browser &&
-          launcher_controller()->IsBrowserFromActiveUser(browser) &&
-          browser->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH &&
-          (browser->is_type_tabbed() ||
-           !browser->is_app() ||
-           !browser->is_type_popup() ||
-           launcher_controller()->
-               GetShelfIDForAppID(web_app::GetExtensionIdFromApplicationName(
-                   browser->app_name())) <= 0));
+  // Only Ash desktop browser windows for the active user are represented.
+  if (!browser ||
+      !launcher_controller()->IsBrowserFromActiveUser(browser) ||
+      browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
+    return false;
+
+  // v1 App popup windows with a valid app id have their own icon.
+  if (browser->is_app() &&
+      browser->is_type_popup() &&
+      launcher_controller()->GetShelfIDForAppID(
+          web_app::GetExtensionIdFromApplicationName(browser->app_name())) > 0)
+    return false;
+
+  // Stand-alone chrome:// windows (e.g. settings) have their own icon.
+  if (chrome::IsTrustedPopupWindowWithScheme(browser, content::kChromeUIScheme))
+    return false;
+
+  // Tabbed browser and other popup windows are all represented.
+  return true;
 }
diff --git a/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h
index 74791201..5f5b39e 100644
--- a/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h
+++ b/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h
@@ -29,6 +29,10 @@ class BrowserShortcutLauncherItemController : public LauncherItemController {
   // Updates the activation state of the Broswer item.
   void UpdateBrowserItemState();
 
+  // Sets the shelf id for the browser window if the browser is represented.
+  void SetShelfIDForBrowserWindowContents(Browser* browser,
+                                          content::WebContents* web_contents);
+
   // LauncherItemController overrides:
   virtual bool IsOpen() const OVERRIDE;
   virtual bool IsVisible() const OVERRIDE;
diff --git a/chrome/browser/ui/ash/launcher/browser_status_monitor.cc b/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
index 7d8123d..f10792c 100644
--- a/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
+++ b/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
@@ -10,68 +10,100 @@
 #include "base/stl_util.h"
 #include "chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
+#include "chrome/browser/ui/ash/launcher/launcher_item_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/settings_window_manager.h"
+#include "chrome/browser/ui/settings_window_manager_observer.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "grit/ash_resources.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/gfx/screen.h"
 #include "ui/wm/public/activation_client.h"
 
-BrowserStatusMonitor::LocalWebContentsObserver::LocalWebContentsObserver(
-    content::WebContents* contents,
-    BrowserStatusMonitor* monitor)
-    : content::WebContentsObserver(contents),
-      monitor_(monitor) {
-}
+// This class monitors the WebContent of the all tab and notifies a navigation
+// to the BrowserStatusMonitor.
+class BrowserStatusMonitor::LocalWebContentsObserver
+    : public content::WebContentsObserver {
+ public:
+  LocalWebContentsObserver(content::WebContents* contents,
+                           BrowserStatusMonitor* monitor)
+      : content::WebContentsObserver(contents),
+        monitor_(monitor) {}
+
+  virtual ~LocalWebContentsObserver() {}
+
+  // content::WebContentsObserver
+  virtual void DidNavigateMainFrame(
+      const content::LoadCommittedDetails& details,
+      const content::FrameNavigateParams& params) OVERRIDE {
+    Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
+    ChromeLauncherController::AppState state =
+        ChromeLauncherController::APP_STATE_INACTIVE;
+    if (browser->window()->IsActive() &&
+        browser->tab_strip_model()->GetActiveWebContents() == web_contents())
+      state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;
+    else if (browser->window()->IsActive())
+      state = ChromeLauncherController::APP_STATE_ACTIVE;
+
+    monitor_->UpdateAppItemState(web_contents(), state);
+    monitor_->UpdateBrowserItemState();
+
+    // Navigating may change the ShelfID associated with the WebContents.
+    if (browser->tab_strip_model()->GetActiveWebContents() == web_contents())
+      monitor_->SetShelfIDForBrowserWindowContents(browser, web_contents());
+  }
 
-BrowserStatusMonitor::LocalWebContentsObserver::~LocalWebContentsObserver() {
-}
+  virtual void WebContentsDestroyed(
+      content::WebContents* web_content) OVERRIDE {
+    if (web_content == web_contents()) {
+      // We can only come here when there was a non standard termination like
+      // an app got un-installed while running, etc.
+      monitor_->WebContentsDestroyed(web_content);
+      // |this| is gone now.
+    }
+  }
 
-void BrowserStatusMonitor::LocalWebContentsObserver::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
-  Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
-  ChromeLauncherController::AppState state =
-      ChromeLauncherController::APP_STATE_INACTIVE;
-  if (browser->window()->IsActive() &&
-      browser->tab_strip_model()->GetActiveWebContents() == web_contents())
-    state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;
-  else if (browser->window()->IsActive())
-    state = ChromeLauncherController::APP_STATE_ACTIVE;
+ private:
+  BrowserStatusMonitor* monitor_;
 
-  monitor_->UpdateAppItemState(web_contents(), state);
-  monitor_->UpdateBrowserItemState();
+  DISALLOW_COPY_AND_ASSIGN(LocalWebContentsObserver);
+};
 
-  // Navigating may change the ShelfID associated with the WebContents.
-  if (browser->tab_strip_model()->GetActiveWebContents() == web_contents()) {
-    ash::SetShelfIDForWindow(
-        monitor_->GetShelfIDForWebContents(web_contents()),
-        browser->window()->GetNativeWindow());
-  }
-}
+// Observes any new settings windows and sets their shelf icon (since they
+// are excluded from BrowserShortcutLauncherItem).
+class BrowserStatusMonitor::SettingsWindowObserver
+    : public chrome::SettingsWindowManagerObserver {
+ public:
+  SettingsWindowObserver() {}
+  virtual ~SettingsWindowObserver() {}
 
-void BrowserStatusMonitor::LocalWebContentsObserver::WebContentsDestroyed(
-    content::WebContents* web_content) {
-  if (web_content == web_contents()) {
-    // We can only come here when there was a non standard termination like
-    // an app got un-installed while running, etc.
-    monitor_->WebContentsDestroyed(web_content);
-    // |this| is gone now.
+  // SettingsWindowManagerObserver
+  virtual void OnNewSettingsWindow(Browser* settings_browser) OVERRIDE {
+    CreateShelfItemForDialog(IDR_ASH_SHELF_ICON_SETTINGS,
+                             settings_browser->window()->GetNativeWindow());
   }
-}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SettingsWindowObserver);
+};
 
 BrowserStatusMonitor::BrowserStatusMonitor(
     ChromeLauncherController* launcher_controller)
     : launcher_controller_(launcher_controller),
       observed_activation_clients_(this),
-      observed_root_windows_(this) {
+      observed_root_windows_(this),
+      settings_window_observer_(new SettingsWindowObserver) {
   DCHECK(launcher_controller_);
   BrowserList::AddObserver(this);
+  chrome::SettingsWindowManager::GetInstance()->AddObserver(
+      settings_window_observer_.get());
 
   // This check needs for win7_aura. Without this, all tests in
   // ChromeLauncherController will fail in win7_aura.
@@ -247,8 +279,7 @@ void BrowserStatusMonitor::ActiveTabChanged(content::WebContents* old_contents,
         ChromeLauncherController::APP_STATE_ACTIVE;
     UpdateAppItemState(new_contents, state);
     UpdateBrowserItemState();
-    ash::SetShelfIDForWindow(GetShelfIDForWebContents(new_contents),
-                             browser->window()->GetNativeWindow());
+    SetShelfIDForBrowserWindowContents(browser, new_contents);
   }
 }
 
@@ -274,10 +305,8 @@ void BrowserStatusMonitor::TabReplacedAt(TabStripModel* tab_strip_model,
   UpdateAppItemState(new_contents, state);
   UpdateBrowserItemState();
 
-  if (tab_strip_model->GetActiveWebContents() == new_contents) {
-    ash::SetShelfIDForWindow(GetShelfIDForWebContents(new_contents),
-                             browser->window()->GetNativeWindow());
-  }
+  if (tab_strip_model->GetActiveWebContents() == new_contents)
+    SetShelfIDForBrowserWindowContents(browser, new_contents);
 
   AddWebContentsObserver(new_contents);
 }
@@ -355,3 +384,10 @@ ash::ShelfID BrowserStatusMonitor::GetShelfIDForWebContents(
     content::WebContents* contents) {
   return launcher_controller_->GetShelfIDForWebContents(contents);
 }
+
+void BrowserStatusMonitor::SetShelfIDForBrowserWindowContents(
+    Browser* browser,
+    content::WebContents* web_contents) {
+  launcher_controller_->GetBrowserShortcutLauncherItemController()->
+      SetShelfIDForBrowserWindowContents(browser, web_contents);
+}
diff --git a/chrome/browser/ui/ash/launcher/browser_status_monitor.h b/chrome/browser/ui/ash/launcher/browser_status_monitor.h
index fd1d205..426ef6e 100644
--- a/chrome/browser/ui/ash/launcher/browser_status_monitor.h
+++ b/chrome/browser/ui/ash/launcher/browser_status_monitor.h
@@ -15,7 +15,6 @@
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/browser_list_observer.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
-#include "content/public/browser/web_contents_observer.h"
 #include "ui/aura/window_observer.h"
 #include "ui/gfx/display_observer.h"
 #include "ui/wm/public/activation_change_observer.h"
@@ -106,26 +105,8 @@ class BrowserStatusMonitor : public aura::client::ActivationChangeObserver,
   bool IsV1AppInShelf(Browser* browser);
 
  private:
-  // This class monitors the WebContent of the all tab and notifies a navigation
-  // to the BrowserStatusMonitor.
-  class LocalWebContentsObserver : public content::WebContentsObserver {
-   public:
-    LocalWebContentsObserver(content::WebContents* contents,
-                             BrowserStatusMonitor* monitor);
-    virtual ~LocalWebContentsObserver();
-
-    // content::WebContentsObserver overrides:
-    virtual void DidNavigateMainFrame(
-        const content::LoadCommittedDetails& details,
-        const content::FrameNavigateParams& params) OVERRIDE;
-    virtual void WebContentsDestroyed(
-        content::WebContents* web_contents) OVERRIDE;
-
-   private:
-    BrowserStatusMonitor* monitor_;
-
-    DISALLOW_COPY_AND_ASSIGN(LocalWebContentsObserver);
-  };
+  class LocalWebContentsObserver;
+  class SettingsWindowObserver;
 
   typedef std::map<Browser*, std::string> BrowserToAppIDMap;
   typedef std::map<content::WebContents*, LocalWebContentsObserver*>
@@ -137,9 +118,13 @@ class BrowserStatusMonitor : public aura::client::ActivationChangeObserver,
   // Remove LocalWebContentsObserver for |contents|.
   void RemoveWebContentsObserver(content::WebContents* contents);
 
-  // Retruns the ShelfID for |contents|.
+  // Returns the ShelfID for |contents|.
   ash::ShelfID GetShelfIDForWebContents(content::WebContents* contents);
 
+  // Sets the shelf id for browsers represented by the browser shortcut item.
+  void SetShelfIDForBrowserWindowContents(Browser* browser,
+                                          content::WebContents* web_contents);
+
   ChromeLauncherController* launcher_controller_;
 
   // Hold all observed activation clients.
@@ -150,8 +135,8 @@ class BrowserStatusMonitor : public aura::client::ActivationChangeObserver,
   ScopedObserver<aura::Window, aura::WindowObserver> observed_root_windows_;
 
   BrowserToAppIDMap browser_to_app_id_map_;
-
   WebContentsToObserverMap webcontents_to_observer_map_;
+  scoped_ptr<SettingsWindowObserver> settings_window_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserStatusMonitor);
 };
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
index 800eecb..02ff9a0 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
@@ -43,6 +43,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/host_desktop.h"
+#include "chrome/browser/ui/settings_window_manager.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -2031,3 +2032,23 @@ IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, V1AppNavigation) {
   base::MessageLoop::current()->RunUntilIdle();
   EXPECT_EQ(ash::STATUS_CLOSED, model_->ItemByID(id)->status);
 }
+
+// Checks that a opening a settings window creates a new launcher item.
+IN_PROC_BROWSER_TEST_F(ShelfAppBrowserTest, SettingsWindow) {
+  chrome::SettingsWindowManager* settings_manager =
+      chrome::SettingsWindowManager::GetInstance();
+  ash::ShelfModel* shelf_model = ash::Shell::GetInstance()->shelf_model();
+
+  // Get the number of items in the shelf and browser menu.
+  int item_count = shelf_model->item_count();
+  size_t browser_count = NumberOfDetectedLauncherBrowsers(false);
+
+  // Open a settings window. Number of browser items should remain unchanged,
+  // number of shelf items should increase.
+  settings_manager->ShowForProfile(browser()->profile(), std::string());
+  Browser* settings_browser =
+      settings_manager->FindBrowserForProfile(browser()->profile());
+  ASSERT_TRUE(settings_browser);
+  EXPECT_EQ(browser_count, NumberOfDetectedLauncherBrowsers(false));
+  EXPECT_EQ(item_count + 1, shelf_model->item_count());
+}
diff --git a/chrome/browser/ui/chrome_pages.cc b/chrome/browser/ui/chrome_pages.cc
index bb713e8..08df687 100644
--- a/chrome/browser/ui/chrome_pages.cc
+++ b/chrome/browser/ui/chrome_pages.cc
@@ -28,7 +28,6 @@
 #include "components/signin/core/browser/signin_manager.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
-#include "content/public/common/url_constants.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "net/base/url_util.h"
 
@@ -212,6 +211,20 @@ GURL GetSettingsUrl(const std::string& sub_page) {
   return GURL(url);
 }
 
+bool IsTrustedPopupWindowWithScheme(const Browser* browser,
+                                    const std::string& scheme) {
+  if (!browser->is_type_popup() || !browser->is_trusted_source())
+    return false;
+  if (scheme.empty())  // Any trusted popup window
+    return true;
+  const content::WebContents* web_contents =
+      browser->tab_strip_model()->GetWebContentsAt(0);
+  if (!web_contents)
+    return false;
+  GURL url(web_contents->GetURL());
+  return url.SchemeIs(scheme.c_str());
+}
+
 void ShowSettings(Browser* browser) {
   ShowSettingsSubPage(browser, std::string());
 }
diff --git a/chrome/browser/ui/chrome_pages.h b/chrome/browser/ui/chrome_pages.h
index 0214132..0705e46 100644
--- a/chrome/browser/ui/chrome_pages.h
+++ b/chrome/browser/ui/chrome_pages.h
@@ -57,6 +57,11 @@ void ShowSlow(Browser* browser);
 // Constructs a settings GURL for the specified |sub_page|.
 GURL GetSettingsUrl(const std::string& sub_page);
 
+// Returns true if |browser| is a trusted popup window containing a page with
+// matching |scheme| (or any trusted popup if |scheme| is empty).
+bool IsTrustedPopupWindowWithScheme(const Browser* browser,
+                                    const std::string& scheme);
+
 // Various things that open in a settings UI.
 void ShowSettings(Browser* browser);
 void ShowSettingsSubPage(Browser* browser, const std::string& sub_page);
diff --git a/chrome/browser/ui/settings_window_manager.cc b/chrome/browser/ui/settings_window_manager.cc
index 5c8f345..1e991e4 100644
--- a/chrome/browser/ui/settings_window_manager.cc
+++ b/chrome/browser/ui/settings_window_manager.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/browser/ui/settings_window_manager_observer.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
@@ -21,30 +22,38 @@ SettingsWindowManager* SettingsWindowManager::GetInstance() {
   return Singleton<SettingsWindowManager>::get();
 }
 
+void SettingsWindowManager::AddObserver(
+    SettingsWindowManagerObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void SettingsWindowManager::RemoveObserver(
+    SettingsWindowManagerObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void SettingsWindowManager::ShowForProfile(Profile* profile,
                                            const std::string& sub_page) {
   content::RecordAction(base::UserMetricsAction("ShowOptions"));
   GURL gurl = chrome::GetSettingsUrl(sub_page);
   // Look for an existing browser window.
-  ProfileSessionMap::iterator iter = settings_session_map_.find(profile);
-  if (iter != settings_session_map_.end()) {
-    Browser* browser = chrome::FindBrowserWithID(iter->second);
-    if (browser) {
-      DCHECK(browser->profile() == profile);
-      const content::WebContents* web_contents =
-          browser->tab_strip_model()->GetWebContentsAt(0);
-      if (web_contents && web_contents->GetURL() == gurl) {
-        browser->window()->Show();
-        return;
-      }
-      NavigateParams params(browser, gurl,
-                            content::PAGE_TRANSITION_AUTO_BOOKMARK);
-      params.window_action = NavigateParams::SHOW_WINDOW;
-      params.user_gesture = true;
-      chrome::Navigate(&params);
+  Browser* browser = FindBrowserForProfile(profile);
+  if (browser) {
+    DCHECK(browser->profile() == profile);
+    const content::WebContents* web_contents =
+        browser->tab_strip_model()->GetWebContentsAt(0);
+    if (web_contents && web_contents->GetURL() == gurl) {
+      browser->window()->Show();
       return;
     }
+    NavigateParams params(browser, gurl,
+                          content::PAGE_TRANSITION_AUTO_BOOKMARK);
+    params.window_action = NavigateParams::SHOW_WINDOW;
+    params.user_gesture = true;
+    chrome::Navigate(&params);
+    return;
   }
+
   // No existing browser window, create one.
   NavigateParams params(profile, gurl, content::PAGE_TRANSITION_AUTO_BOOKMARK);
   params.disposition = NEW_POPUP;
@@ -54,6 +63,16 @@ void SettingsWindowManager::ShowForProfile(Profile* profile,
   params.path_behavior = NavigateParams::IGNORE_AND_NAVIGATE;
   chrome::Navigate(&params);
   settings_session_map_[profile] = params.browser->session_id().id();
+
+  FOR_EACH_OBSERVER(SettingsWindowManagerObserver,
+                    observers_, OnNewSettingsWindow(params.browser));
+}
+
+Browser* SettingsWindowManager::FindBrowserForProfile(Profile* profile) {
+  ProfileSessionMap::iterator iter = settings_session_map_.find(profile);
+  if (iter != settings_session_map_.end())
+    return chrome::FindBrowserWithID(iter->second);
+  return NULL;
 }
 
 SettingsWindowManager::SettingsWindowManager() {
diff --git a/chrome/browser/ui/settings_window_manager.h b/chrome/browser/ui/settings_window_manager.h
index fc96bcd..79b07d4 100644
--- a/chrome/browser/ui/settings_window_manager.h
+++ b/chrome/browser/ui/settings_window_manager.h
@@ -9,12 +9,16 @@
 #include <string>
 
 #include "base/memory/singleton.h"
+#include "base/observer_list.h"
 #include "chrome/browser/sessions/session_id.h"
 
+class Browser;
 class Profile;
 
 namespace chrome {
 
+class SettingsWindowManagerObserver;
+
 // Class for managing settings windows when --enable-settings-window is enabled.
 // TODO(stevenjb): Remove flag comment if enabled by default.
 
@@ -22,10 +26,17 @@ class SettingsWindowManager {
  public:
   static SettingsWindowManager* GetInstance();
 
-  // Show an existing settings window for |profile| or create a new one, and
-  // navigate to |sub_page|.
+  void AddObserver(SettingsWindowManagerObserver* observer);
+  void RemoveObserver(SettingsWindowManagerObserver* observer);
+
+  // Shows an existing settings Browser window for |profile| or creates a new
+  // one. Navigates that window to |sub_page|.
   void ShowForProfile(Profile* profile, const std::string& sub_page);
 
+  // If a Browser settings window for |profile| has already been created,
+  // returns it, otherwise returns NULL.
+  Browser* FindBrowserForProfile(Profile* profile);
+
  private:
   friend struct DefaultSingletonTraits<SettingsWindowManager>;
   typedef std::map<Profile*, SessionID::id_type> ProfileSessionMap;
@@ -33,6 +44,7 @@ class SettingsWindowManager {
   SettingsWindowManager();
   ~SettingsWindowManager();
 
+  ObserverList<SettingsWindowManagerObserver> observers_;
   ProfileSessionMap settings_session_map_;
 
   DISALLOW_COPY_AND_ASSIGN(SettingsWindowManager);
diff --git a/chrome/browser/ui/settings_window_manager_browsertest.cc b/chrome/browser/ui/settings_window_manager_browsertest.cc
new file mode 100644
index 0000000..50aabfc
--- /dev/null
+++ b/chrome/browser/ui/settings_window_manager_browsertest.cc
@@ -0,0 +1,164 @@
+// Copyright 2014 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 "chrome/browser/ui/settings_window_manager.h"
+
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/settings_window_manager_observer.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/test_utils.h"
+
+namespace {
+
+class SettingsWindowTestObserver
+    : public chrome::SettingsWindowManagerObserver {
+ public:
+  SettingsWindowTestObserver() : browser_(NULL), new_settings_count_(0) {}
+  virtual ~SettingsWindowTestObserver() {}
+
+  virtual void OnNewSettingsWindow(Browser* settings_browser) OVERRIDE {
+    browser_ = settings_browser;
+    ++new_settings_count_;
+  }
+
+  Browser* browser() { return browser_; }
+  size_t new_settings_count() const { return new_settings_count_; }
+
+ private:
+  Browser* browser_;
+  size_t new_settings_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(SettingsWindowTestObserver);
+};
+
+}  // namespace
+
+class SettingsWindowManagerTest : public InProcessBrowserTest {
+ public:
+  SettingsWindowManagerTest() : test_profile_(NULL) {
+    chrome::SettingsWindowManager::GetInstance()->AddObserver(&observer_);
+  }
+  virtual ~SettingsWindowManagerTest() {
+    chrome::SettingsWindowManager::GetInstance()->RemoveObserver(&observer_);
+  }
+
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    command_line->AppendSwitch(::switches::kMultiProfiles);
+  }
+
+  Profile* CreateTestProfile() {
+    CHECK(!test_profile_);
+
+    ProfileManager* profile_manager = g_browser_process->profile_manager();
+    base::RunLoop run_loop;
+    profile_manager->CreateProfileAsync(
+        profile_manager->GenerateNextProfileDirectoryPath(),
+        base::Bind(&SettingsWindowManagerTest::ProfileInitialized,
+                   base::Unretained(this),
+                   run_loop.QuitClosure()),
+        base::string16(),
+        base::string16(),
+        std::string());
+    run_loop.Run();
+
+    return test_profile_;
+  }
+
+  void ProfileInitialized(const base::Closure& closure,
+                          Profile* profile,
+                          Profile::CreateStatus status) {
+    if (status == Profile::CREATE_STATUS_INITIALIZED) {
+      test_profile_ = profile;
+      closure.Run();
+    }
+  }
+
+  void CloseBrowserSynchronously(Browser* browser) {
+    content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_BROWSER_CLOSED,
+        content::NotificationService::AllSources());
+    browser->window()->Close();
+    observer.Wait();
+  }
+
+ protected:
+  SettingsWindowTestObserver observer_;
+  base::ScopedTempDir temp_profile_dir_;
+  Profile* test_profile_;  // Owned by g_browser_process->profile_manager()
+
+  DISALLOW_COPY_AND_ASSIGN(SettingsWindowManagerTest);
+};
+
+
+IN_PROC_BROWSER_TEST_F(SettingsWindowManagerTest, OpenSettingsWindow) {
+  chrome::SettingsWindowManager* settings_manager =
+      chrome::SettingsWindowManager::GetInstance();
+
+  // Open a settings window.
+  settings_manager->ShowForProfile(browser()->profile(), std::string());
+  Browser* settings_browser =
+      settings_manager->FindBrowserForProfile(browser()->profile());
+  ASSERT_TRUE(settings_browser);
+  // Ensure the observer fired correctly.
+  EXPECT_EQ(1u, observer_.new_settings_count());
+  EXPECT_EQ(settings_browser, observer_.browser());
+
+  // Open the settings again: no new window.
+  settings_manager->ShowForProfile(browser()->profile(), std::string());
+  EXPECT_EQ(settings_browser,
+            settings_manager->FindBrowserForProfile(browser()->profile()));
+  EXPECT_EQ(1u, observer_.new_settings_count());
+
+  // Close the settings window.
+  CloseBrowserSynchronously(settings_browser);
+  EXPECT_FALSE(settings_manager->FindBrowserForProfile(browser()->profile()));
+
+  // Open a new settings window.
+  settings_manager->ShowForProfile(browser()->profile(), std::string());
+  Browser* settings_browser2 =
+      settings_manager->FindBrowserForProfile(browser()->profile());
+  ASSERT_TRUE(settings_browser2);
+  EXPECT_EQ(2u, observer_.new_settings_count());
+
+  CloseBrowserSynchronously(settings_browser2);
+}
+
+#if !defined(OS_CHROMEOS)
+IN_PROC_BROWSER_TEST_F(SettingsWindowManagerTest, SettingsWindowMultiProfile) {
+  chrome::SettingsWindowManager* settings_manager =
+      chrome::SettingsWindowManager::GetInstance();
+  Profile* test_profile = CreateTestProfile();
+  ASSERT_TRUE(test_profile);
+
+  // Open a settings window.
+  settings_manager->ShowForProfile(browser()->profile(), std::string());
+  Browser* settings_browser =
+      settings_manager->FindBrowserForProfile(browser()->profile());
+  ASSERT_TRUE(settings_browser);
+  // Ensure the observer fired correctly.
+  EXPECT_EQ(1u, observer_.new_settings_count());
+  EXPECT_EQ(settings_browser, observer_.browser());
+
+  // Open a settings window for a new profile.
+  settings_manager->ShowForProfile(test_profile, std::string());
+  Browser* settings_browser2 =
+      settings_manager->FindBrowserForProfile(test_profile);
+  ASSERT_TRUE(settings_browser2);
+  // Ensure the observer fired correctly.
+  EXPECT_EQ(2u, observer_.new_settings_count());
+  EXPECT_EQ(settings_browser2, observer_.browser());
+
+  CloseBrowserSynchronously(settings_browser);
+  CloseBrowserSynchronously(settings_browser2);
+}
+#endif
diff --git a/chrome/browser/ui/settings_window_manager_observer.h b/chrome/browser/ui/settings_window_manager_observer.h
new file mode 100644
index 0000000..69bfec4
--- /dev/null
+++ b/chrome/browser/ui/settings_window_manager_observer.h
@@ -0,0 +1,23 @@
+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_UI_SETTINGS_WINDOW_MANAGER_OBSERVER_H_
+#define CHROME_BROWSER_UI_SETTINGS_WINDOW_MANAGER_OBSERVER_H_
+
+class Browser;
+
+namespace chrome {
+
+class SettingsWindowManagerObserver {
+ public:
+  // Called when a new settings browser window is created.
+  virtual void OnNewSettingsWindow(Browser* settings_browser) = 0;
+
+ protected:
+  virtual ~SettingsWindowManagerObserver() {}
+};
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_UI_SETTINGS_WINDOW_MANAGER_OBSERVER_H_
diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi
index e8f37e5..2c796a0 100644
--- a/chrome/chrome_browser_ui.gypi
+++ b/chrome/chrome_browser_ui.gypi
@@ -1301,6 +1301,7 @@
         'browser/ui/search_engines/template_url_table_model.h',
         'browser/ui/settings_window_manager.cc',
         'browser/ui/settings_window_manager.h',
+        'browser/ui/settings_window_manager_observer.h',
         'browser/ui/profile_reset_bubble.h',
         'browser/ui/profile_reset_bubble_stub.cc',
         'browser/ui/simple_message_box.h',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index c05a240..17a7572 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1355,6 +1355,7 @@
         'browser/ui/panels/panel_extension_browsertest.cc',
         'browser/ui/pdf/pdf_browsertest.cc',
         'browser/ui/prefs/prefs_tab_helper_browsertest.cc',
+        'browser/ui/settings_window_manager_browsertest.cc',
         'browser/ui/startup/startup_browser_creator_browsertest.cc',
         'browser/ui/sync/one_click_signin_bubble_links_delegate_browsertest.cc',
         'browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc',