Fixes bugs in usage of adding a new tab with a disposition of

NEW_WINDOW. Many places were leaking a Browser and preventing Chrome
from shutting down.

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/3655006

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

13 files changed, 96 insertions, 79 deletions

diff --git a/chrome/browser/cocoa/about_window_controller.mm b/chrome/browser/cocoa/about_window_controller.mm
index bb8044c..7d96803 100644
--- a/chrome/browser/cocoa/about_window_controller.mm
+++ b/chrome/browser/cocoa/about_window_controller.mm
@@ -13,6 +13,7 @@
 #include "base/string_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/browser_list.h"
+#include "chrome/browser/browser_window.h"
 #include "chrome/browser/platform_util.h"
 #import "chrome/browser/cocoa/background_tile_view.h"
 #import "chrome/browser/cocoa/keystone_glue.h"
@@ -620,10 +621,9 @@ static BOOL recentShownUserActionFailedStatus = NO;
   // We always create a new window, so there's no need to try to re-use
   // an existing one just to pass in the NEW_WINDOW disposition.
   Browser* browser = Browser::Create(profile_);
-  if (browser) {
-    browser->OpenURL(GURL([link UTF8String]), GURL(), NEW_WINDOW,
-                     PageTransition::LINK);
-  }
+  browser->OpenURL(GURL([link UTF8String]), GURL(), NEW_FOREGROUND_TAB,
+                   PageTransition::LINK);
+  browser->window()->Show();
   return YES;
 }
 
diff --git a/chrome/browser/cocoa/preferences_window_controller.mm b/chrome/browser/cocoa/preferences_window_controller.mm
index d49c0ba..6492245 100644
--- a/chrome/browser/cocoa/preferences_window_controller.mm
+++ b/chrome/browser/cocoa/preferences_window_controller.mm
@@ -45,6 +45,7 @@
 #include "chrome/browser/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/shell_integration.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/sync_ui_util.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
@@ -1488,17 +1489,15 @@ const int kDisabledIndex = 1;
 - (IBAction)privacyLearnMore:(id)sender {
   // We open a new browser window so the Options dialog doesn't get lost
   // behind other windows.
-  Browser* browser = Browser::Create(profile_);
-  browser->OpenURL(GURL(l10n_util::GetStringUTF16(IDS_LEARN_MORE_PRIVACY_URL)),
-                   GURL(), NEW_WINDOW, PageTransition::LINK);
+  browser::ShowOptionsURL(
+      profile_,
+      GURL(l10n_util::GetStringUTF16(IDS_LEARN_MORE_PRIVACY_URL)));
 }
 
 - (IBAction)backgroundModeLearnMore:(id)sender {
-  // We open a new browser window so the Options dialog doesn't get lost
-  // behind other windows.
-  Browser::Create(profile_)->OpenURL(
-      GURL(l10n_util::GetStringUTF16(IDS_LEARN_MORE_BACKGROUND_MODE_URL)),
-      GURL(), NEW_WINDOW, PageTransition::LINK);
+  browser::ShowOptionsURL(
+      profile_,
+      GURL(l10n_util::GetStringUTF16(IDS_LEARN_MORE_BACKGROUND_MODE_URL)));
 }
 
 - (IBAction)resetAutoOpenFiles:(id)sender {
diff --git a/chrome/browser/gtk/options/advanced_contents_gtk.cc b/chrome/browser/gtk/options/advanced_contents_gtk.cc
index edf89b8..343f9bd 100644
--- a/chrome/browser/gtk/options/advanced_contents_gtk.cc
+++ b/chrome/browser/gtk/options/advanced_contents_gtk.cc
@@ -39,6 +39,7 @@
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
@@ -367,7 +368,8 @@ class NetworkSection : public OptionsPageBase {
   static bool SearchPATH(ProxyConfigCommand* commands, size_t ncommands,
                          size_t* index);
   // Start the given proxy configuration utility.
-  static void StartProxyConfigUtil(const ProxyConfigCommand& command);
+  static void StartProxyConfigUtil(Profile* profile,
+                                   const ProxyConfigCommand& command);
 
   // Tracks the state of proxy preferences.
   scoped_ptr<PrefSetObserver> proxy_prefs_;
@@ -455,14 +457,12 @@ void NetworkSection::OnChangeProxiesButtonClicked(GtkButton *button,
   }
 
   if (found_command) {
-    StartProxyConfigUtil(command);
+    StartProxyConfigUtil(section->profile(), command);
   } else {
     const char* name = base::GetDesktopEnvironmentName(env.get());
     if (name)
       LOG(ERROR) << "Could not find " << name << " network settings in $PATH";
-    BrowserList::GetLastActive()->
-        OpenURL(GURL(kLinuxProxyConfigUrl),
-                GURL(), NEW_FOREGROUND_TAB, PageTransition::LINK);
+    browser::ShowOptionsURL(section->profile(), GURL(kLinuxProxyConfigUrl));
   }
 }
 
@@ -491,7 +491,8 @@ bool NetworkSection::SearchPATH(ProxyConfigCommand* commands, size_t ncommands,
 }
 
 // static
-void NetworkSection::StartProxyConfigUtil(const ProxyConfigCommand& command) {
+void NetworkSection::StartProxyConfigUtil(Profile* profile,
+                                          const ProxyConfigCommand& command) {
   std::vector<std::string> argv;
   argv.push_back(command.binary);
   for (size_t i = 1; command.argv[i]; i++)
@@ -500,9 +501,7 @@ void NetworkSection::StartProxyConfigUtil(const ProxyConfigCommand& command) {
   base::ProcessHandle handle;
   if (!base::LaunchApp(argv, no_files, false, &handle)) {
     LOG(ERROR) << "StartProxyConfigUtil failed to start " << command.binary;
-    BrowserList::GetLastActive()->
-        OpenURL(GURL(kLinuxProxyConfigUrl), GURL(), NEW_FOREGROUND_TAB,
-                PageTransition::LINK);
+    browser::ShowOptionsURL(profile, GURL(kLinuxProxyConfigUrl));
     return;
   }
   ProcessWatcher::EnsureProcessGetsReaped(handle);
@@ -683,9 +682,9 @@ void ChromeAppsSection::OnBackgroundModeClicked(GtkWidget* widget) {
 }
 
 void ChromeAppsSection::OnLearnMoreLinkClicked(GtkWidget* widget) {
-  BrowserList::GetLastActive()->OpenURL(
-      GURL(l10n_util::GetStringUTF8(IDS_LEARN_MORE_BACKGROUND_MODE_URL)),
-      GURL(), NEW_WINDOW, PageTransition::LINK);
+  browser::ShowOptionsURL(
+      profile(),
+      GURL(l10n_util::GetStringUTF8(IDS_LEARN_MORE_BACKGROUND_MODE_URL)));
 }
 
 
@@ -884,9 +883,9 @@ void PrivacySection::OnClearBrowsingDataButtonClicked(GtkButton* widget,
 // static
 void PrivacySection::OnLearnMoreLinkClicked(GtkButton *button,
                                             PrivacySection* privacy_section) {
-  BrowserList::GetLastActive()->
-      OpenURL(GURL(l10n_util::GetStringUTF8(IDS_LEARN_MORE_PRIVACY_URL)),
-              GURL(), NEW_WINDOW, PageTransition::LINK);
+  browser::ShowOptionsURL(
+      privacy_section->profile(),
+      GURL(l10n_util::GetStringUTF8(IDS_LEARN_MORE_PRIVACY_URL)));
 }
 
 // static
@@ -1184,9 +1183,8 @@ void SecuritySection::NotifyPrefChanged(const std::string* pref_name) {
 // static
 void SecuritySection::OnManageCertificatesClicked(GtkButton* button,
                                                   SecuritySection* section) {
-  BrowserList::GetLastActive()->
-      OpenURL(GURL(kLinuxCertificatesConfigUrl), GURL(), NEW_WINDOW,
-              PageTransition::LINK);
+  browser::ShowOptionsURL(section->profile(),
+                          GURL(kLinuxCertificatesConfigUrl));
 }
 
 // static
diff --git a/chrome/browser/gtk/options/content_filter_page_gtk.cc b/chrome/browser/gtk/options/content_filter_page_gtk.cc
index 68ef44f..6645a22 100644
--- a/chrome/browser/gtk/options/content_filter_page_gtk.cc
+++ b/chrome/browser/gtk/options/content_filter_page_gtk.cc
@@ -19,6 +19,7 @@
 #include "chrome/browser/gtk/options/simple_content_exceptions_window.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/notifications/notification_exceptions_table_model.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
@@ -226,10 +227,5 @@ void ContentFilterPageGtk::OnExceptionsClicked(GtkWidget* button) {
 }
 
 void ContentFilterPageGtk::OnPluginsPageLinkClicked(GtkWidget* button) {
-  // We open a new browser window so the Options dialog doesn't get lost
-  // behind other windows.
-  Browser* browser = Browser::Create(profile());
-  browser->OpenURL(GURL(chrome::kChromeUIPluginsURL),
-                   GURL(), NEW_FOREGROUND_TAB, PageTransition::LINK);
-  browser->window()->Show();
+  browser::ShowOptionsURL(profile(), GURL(chrome::kChromeUIPluginsURL));
 }
diff --git a/chrome/browser/gtk/options/cookie_filter_page_gtk.cc b/chrome/browser/gtk/options/cookie_filter_page_gtk.cc
index f60bebd..9b63262 100644
--- a/chrome/browser/gtk/options/cookie_filter_page_gtk.cc
+++ b/chrome/browser/gtk/options/cookie_filter_page_gtk.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/gtk/options/cookies_view.h"
 #include "chrome/browser/host_content_settings_map.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "grit/generated_resources.h"
@@ -200,10 +201,6 @@ void CookieFilterPageGtk::OnShowCookiesClicked(GtkWidget* button) {
 }
 
 void CookieFilterPageGtk::OnFlashLinkClicked(GtkWidget* button) {
-  // We open a new browser window so the Options dialog doesn't get lost
-  // behind other windows.
-  Browser* browser = Browser::Create(profile());
-  browser->OpenURL(GURL(l10n_util::GetStringUTF8(IDS_FLASH_STORAGE_URL)),
-                   GURL(), NEW_FOREGROUND_TAB, PageTransition::LINK);
-  browser->window()->Show();
+  browser::ShowOptionsURL(
+      profile(), GURL(l10n_util::GetStringUTF8(IDS_FLASH_STORAGE_URL)));
 }
diff --git a/chrome/browser/show_options_url.cc b/chrome/browser/show_options_url.cc
new file mode 100644
index 0000000..ec18e4f
--- /dev/null
+++ b/chrome/browser/show_options_url.cc
@@ -0,0 +1,21 @@
+// Copyright (c) 2010 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/show_options_url.h"
+
+#include "chrome/browser/browser.h"
+#include "chrome/browser/browser_window.h"
+#include "chrome/browser/profile.h"
+
+namespace browser {
+
+void ShowOptionsURL(Profile* profile, const GURL& url) {
+  // We open a new browser window so the Options dialog doesn't get lost behind
+  // other windows.
+  Browser* browser = Browser::Create(profile);
+  browser->AddSelectedTabWithURL(url, PageTransition::LINK);
+  browser->window()->Show();
+}
+
+}  // namespace browser
diff --git a/chrome/browser/show_options_url.h b/chrome/browser/show_options_url.h
new file mode 100644
index 0000000..afa339dd
--- /dev/null
+++ b/chrome/browser/show_options_url.h
@@ -0,0 +1,20 @@
+// Copyright (c) 2010 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_SHOW_OPTIONS_URL_H_
+#define CHROME_BROWSER_SHOW_OPTIONS_URL_H_
+#pragma once
+
+class GURL;
+class Profile;
+
+namespace browser {
+
+// Opens a tab showing the specified url. This is intended for use any place
+// we show a URL in the options dialogs.
+void ShowOptionsURL(Profile* profile, const GURL& url);
+
+}  // namespace browser
+
+#endif  // CHROME_BROWSER_SHOW_OPTIONS_URL_H_
diff --git a/chrome/browser/task_manager/task_manager.cc b/chrome/browser/task_manager/task_manager.cc
index d422805..0793a4d 100644
--- a/chrome/browser/task_manager/task_manager.cc
+++ b/chrome/browser/task_manager/task_manager.cc
@@ -944,8 +944,9 @@ void TaskManager::OpenAboutMemory() {
             g_browser_process->profile_manager()->end())
       return;
     browser = Browser::Create(*g_browser_process->profile_manager()->begin());
-    browser->OpenURL(GURL(chrome::kAboutMemoryURL), GURL(), NEW_WINDOW,
+    browser->OpenURL(GURL(chrome::kAboutMemoryURL), GURL(), NEW_FOREGROUND_TAB,
                      PageTransition::LINK);
+    browser->window()->Show();
   } else {
     browser->OpenURL(GURL(chrome::kAboutMemoryURL), GURL(), NEW_FOREGROUND_TAB,
                      PageTransition::LINK);
diff --git a/chrome/browser/views/options/advanced_contents_view.cc b/chrome/browser/views/options/advanced_contents_view.cc
index e6a2090..f99531e 100644
--- a/chrome/browser/views/options/advanced_contents_view.cc
+++ b/chrome/browser/views/options/advanced_contents_view.cc
@@ -39,6 +39,7 @@
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/shell_dialogs.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/browser/views/browser_dialogs.h"
 #include "chrome/browser/views/clear_data_view.h"
 #include "chrome/browser/views/list_background.h"
@@ -542,13 +543,10 @@ void PrivacySection::ButtonPressed(
 }
 
 void PrivacySection::LinkActivated(views::Link* source, int event_flags) {
-  if (source == learn_more_link_) {
-    // We open a new browser window so the Options dialog doesn't get lost
-    // behind other windows.
-    Browser* browser = Browser::Create(profile());
-    browser->OpenURL(GURL(l10n_util::GetString(IDS_LEARN_MORE_PRIVACY_URL)),
-                     GURL(), NEW_WINDOW, PageTransition::LINK);
-  }
+  DCHECK(source == learn_more_link_);
+  browser::ShowOptionsURL(
+      profile(),
+      GURL(l10n_util::GetString(IDS_LEARN_MORE_PRIVACY_URL)));
 }
 
 void PrivacySection::InitControlLayout() {
@@ -1330,9 +1328,9 @@ void ChromeAppsSection::ButtonPressed(
 
 void ChromeAppsSection::LinkActivated(views::Link* source, int event_flags) {
   DCHECK(source == learn_more_link_);
-  Browser::Create(profile())->OpenURL(
-      GURL(l10n_util::GetString(IDS_LEARN_MORE_BACKGROUND_MODE_URL)), GURL(),
-      NEW_WINDOW, PageTransition::LINK);
+  browser::ShowOptionsURL(
+      profile(),
+      GURL(l10n_util::GetString(IDS_LEARN_MORE_BACKGROUND_MODE_URL)));
 }
 
 void ChromeAppsSection::InitControlLayout() {
@@ -1436,11 +1434,9 @@ void CloudPrintProxySection::ButtonPressed(views::Button* sender,
   } else if (sender == manage_printer_button_) {
     UserMetricsRecordAction(
         UserMetricsAction("Options_ManageCloudPrinters"), NULL);
-    // Open a new browser window for the management tab.  The browser
-    // will go away when the user closes that tab.
-    Browser* browser = Browser::Create(profile());
-    browser->OpenURL(CloudPrintURL(profile()).GetCloudPrintServiceManageURL(),
-                     GURL(), NEW_WINDOW, PageTransition::LINK);
+    browser::ShowOptionsURL(
+        profile(),
+        CloudPrintURL(profile()).GetCloudPrintServiceManageURL());
   }
 }
 
diff --git a/chrome/browser/views/options/cookie_filter_page_view.cc b/chrome/browser/views/options/cookie_filter_page_view.cc
index 320445c..d8ab81b 100644
--- a/chrome/browser/views/options/cookie_filter_page_view.cc
+++ b/chrome/browser/views/options/cookie_filter_page_view.cc
@@ -5,10 +5,9 @@
 #include "chrome/browser/views/options/cookie_filter_page_view.h"
 
 #include "app/l10n_util.h"
-#include "chrome/browser/browser.h"
-#include "chrome/browser/browser_window.h"
 #include "chrome/browser/host_content_settings_map.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/browser/views/options/cookies_view.h"
 #include "chrome/common/pref_names.h"
 #include "grit/generated_resources.h"
@@ -112,10 +111,6 @@ void CookieFilterPageView::ButtonPressed(views::Button* sender,
 // CookieFilterPageView, views::LinkController implementation:
 
 void CookieFilterPageView::LinkActivated(views::Link* source, int event_flags) {
-  // We open a new browser window so the Options dialog doesn't get lost
-  // behind other windows.
-  Browser* browser = Browser::Create(profile());
-  browser->OpenURL(GURL(l10n_util::GetString(IDS_FLASH_STORAGE_URL)), GURL(),
-                   NEW_FOREGROUND_TAB, PageTransition::LINK);
-  browser->window()->Show();
+  browser::ShowOptionsURL(profile(),
+                          GURL(l10n_util::GetString(IDS_FLASH_STORAGE_URL)));
 }
diff --git a/chrome/browser/views/options/general_page_view.cc b/chrome/browser/views/options/general_page_view.cc
index b194f0f..e7c4d3e 100644
--- a/chrome/browser/views/options/general_page_view.cc
+++ b/chrome/browser/views/options/general_page_view.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/search_engines/template_url_model_observer.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/browser/views/keyword_editor_view.h"
 #include "chrome/browser/views/options/managed_prefs_banner_view.h"
 #include "chrome/browser/views/options/options_group_view.h"
@@ -451,12 +452,8 @@ void GeneralPageView::HighlightGroup(OptionsGroup highlight_group) {
 
 void GeneralPageView::LinkActivated(views::Link* source, int event_flags) {
   DCHECK(source == instant_link_);
-  // We open a new browser window so the Options dialog doesn't get lost behind
-  // other windows.
-  Browser* browser = Browser::Create(profile());
-  browser->OpenURL(GURL(browser::kInstantLearnMoreURL),
-                   GURL(), NEW_FOREGROUND_TAB, PageTransition::LINK);
-  browser->window()->Show();
+  browser::ShowOptionsURL(profile(),
+                          GURL(browser::kInstantLearnMoreURL));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/views/options/plugin_filter_page_view.cc b/chrome/browser/views/options/plugin_filter_page_view.cc
index d3dda10..eeb5c6c 100644
--- a/chrome/browser/views/options/plugin_filter_page_view.cc
+++ b/chrome/browser/views/options/plugin_filter_page_view.cc
@@ -6,7 +6,7 @@
 
 #include "app/l10n_util.h"
 #include "chrome/browser/browser.h"
-#include "chrome/browser/browser_window.h"
+#include "chrome/browser/show_options_url.h"
 #include "chrome/common/url_constants.h"
 #include "grit/generated_resources.h"
 #include "views/grid_layout.h"
@@ -45,10 +45,5 @@ void PluginFilterPageView::InitControlLayout() {
 
 void PluginFilterPageView::LinkActivated(views::Link* source,
                                          int event_flags) {
-  // We open a new browser window so the Options dialog doesn't get lost
-  // behind other windows.
-  Browser* browser = Browser::Create(profile());
-  browser->OpenURL(GURL(chrome::kChromeUIPluginsURL), GURL(),
-                   NEW_FOREGROUND_TAB, PageTransition::LINK);
-  browser->window()->Show();
+  browser::ShowOptionsURL(profile(), GURL(chrome::kChromeUIPluginsURL));
 }
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 498ea1d9..b1a3be5 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2618,6 +2618,8 @@
         'browser/shell_integration_mac.mm',
         'browser/shell_integration_linux.cc',
         'browser/shell_integration_win.cc',
+        'browser/show_options_url.cc',
+        'browser/show_options_url.h',
         'browser/sidebar/sidebar_container.cc',
         'browser/sidebar/sidebar_container.h',
         'browser/sidebar/sidebar_manager.cc',