Add support for SINGLETON_TAB disposition to BrowserNavigator.

Also makes Browser support browser::NavigatorDelegate.

BUG=none
TEST=BrowserNavigatorTest.Disposition_SingletonTab*

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

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

5 files changed, 192 insertions, 94 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index d75465b..125ddd5 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -2305,8 +2305,59 @@ int Browser::GetLastBlockedCommand(WindowOpenDisposition* disposition) {
   return last_blocked_command_id_;
 }
 
+void Browser::UpdateUIForNavigationInTab(TabContents* contents,
+                                         PageTransition::Type transition,
+                                         bool user_initiated) {
+  tabstrip_model()->TabNavigating(contents, transition);
+
+  bool contents_is_selected = contents == GetSelectedTabContents();
+  if (user_initiated && contents_is_selected && window()->GetLocationBar()) {
+    // Forcibly reset the location bar if the url is going to change in the
+    // current tab, since otherwise it won't discard any ongoing user edits,
+    // since it doesn't realize this is a user-initiated action.
+    window()->GetLocationBar()->Revert();
+  }
+
+  if (GetStatusBubble())
+    GetStatusBubble()->Hide();
+
+  // Update the location bar. This is synchronous. We specifically don't
+  // update the load state since the load hasn't started yet and updating it
+  // will put it out of sync with the actual state like whether we're
+  // displaying a favicon, which controls the throbber. If we updated it here,
+  // the throbber will show the default favicon for a split second when
+  // navigating away from the new tab page.
+  ScheduleUIUpdate(contents, TabContents::INVALIDATE_URL);
+
+  if (contents_is_selected)
+    contents->Focus();
+}
+
+GURL Browser::GetHomePage() const {
+  // --homepage overrides any preferences.
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kHomePage)) {
+    FilePath browser_directory;
+    PathService::Get(base::DIR_CURRENT, &browser_directory);
+    GURL home_page(URLFixerUpper::FixupRelativeFile(browser_directory,
+        command_line.GetSwitchValuePath(switches::kHomePage)));
+    if (home_page.is_valid())
+      return home_page;
+  }
+
+  if (profile_->GetPrefs()->GetBoolean(prefs::kHomePageIsNewTabPage))
+    return GURL(chrome::kChromeUINewTabURL);
+  GURL home_page(URLFixerUpper::FixupURL(
+      profile_->GetPrefs()->GetString(prefs::kHomePage),
+      std::string()));
+  if (!home_page.is_valid())
+    return GURL(chrome::kChromeUINewTabURL);
+  return home_page;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, PageNavigator implementation:
+
 void Browser::OpenURL(const GURL& url, const GURL& referrer,
                       WindowOpenDisposition disposition,
                       PageTransition::Type transition) {
@@ -4108,28 +4159,6 @@ void Browser::OpenURLAtIndex(TabContents* source,
   }
 }
 
-GURL Browser::GetHomePage() const {
-  // --homepage overrides any preferences.
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kHomePage)) {
-    FilePath browser_directory;
-    PathService::Get(base::DIR_CURRENT, &browser_directory);
-    GURL home_page(URLFixerUpper::FixupRelativeFile(browser_directory,
-        command_line.GetSwitchValuePath(switches::kHomePage)));
-    if (home_page.is_valid())
-      return home_page;
-  }
-
-  if (profile_->GetPrefs()->GetBoolean(prefs::kHomePageIsNewTabPage))
-    return GURL(chrome::kChromeUINewTabURL);
-  GURL home_page(URLFixerUpper::FixupURL(
-      profile_->GetPrefs()->GetString(prefs::kHomePage),
-      std::string()));
-  if (!home_page.is_valid())
-    return GURL(chrome::kChromeUINewTabURL);
-  return home_page;
-}
-
 void Browser::FindInPage(bool find_next, bool forward_direction) {
   ShowFindBar();
   if (find_next) {
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 0936bbb..f757e62 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -631,6 +631,16 @@ class Browser : public TabHandlerDelegate,
   // not null.
   int GetLastBlockedCommand(WindowOpenDisposition* disposition);
 
+  // Called by browser::Navigate() when a navigation has occurred in a tab in
+  // this Browser. Updates the UI for the start of this navigation.
+  void UpdateUIForNavigationInTab(TabContents* contents,
+                                  PageTransition::Type transition,
+                                  bool user_initiated);
+
+  // Called by browser::Navigate() to retrieve the home page if no URL is
+  // specified.
+  GURL GetHomePage() const;
+
   // Interface implementations ////////////////////////////////////////////////
 
   // Overridden from PageNavigator:
@@ -645,6 +655,7 @@ class Browser : public TabHandlerDelegate,
   virtual void TabRestoreServiceChanged(TabRestoreService* service);
   virtual void TabRestoreServiceDestroyed(TabRestoreService* service);
 
+
   // Overridden from TabHandlerDelegate:
   virtual Profile* GetProfile() const;
   virtual Browser* AsBrowser();
@@ -930,10 +941,6 @@ class Browser : public TabHandlerDelegate,
                       int index,
                       int add_types);
 
-  // Returns what the user's home page is, or the new tab page if the home page
-  // has not been set.
-  GURL GetHomePage() const;
-
   // Shows the Find Bar, optionally selecting the next entry that matches the
   // existing search string for that Tab. |forward_direction| controls the
   // search direction.
diff --git a/chrome/browser/browser_navigator.cc b/chrome/browser/browser_navigator.cc
index e34bcba..2950d1f 100644
--- a/chrome/browser/browser_navigator.cc
+++ b/chrome/browser/browser_navigator.cc
@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
+#include "chrome/browser/browser_url_handler.h"
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/location_bar.h"
 #include "chrome/browser/profile.h"
@@ -48,6 +49,48 @@ Browser* GetOrCreateBrowser(Profile* profile) {
   return browser ? browser : Browser::Create(profile);
 }
 
+// Returns true if two URLs are equal ignoring their ref (hash fragment).
+bool CompareURLsIgnoreRef(const GURL& url, const GURL& other) {
+  if (url == other)
+    return true;
+  // If neither has a ref than there is no point in stripping the refs and
+  // the URLs are different since the comparison failed in the previous if
+  // statement.
+  if (!url.has_ref() && !other.has_ref())
+    return false;
+  url_canon::Replacements<char> replacements;
+  replacements.ClearRef();
+  GURL url_no_ref = url.ReplaceComponents(replacements);
+  GURL other_no_ref = other.ReplaceComponents(replacements);
+  return url_no_ref == other_no_ref;
+}
+
+// Returns the index of an existing singleton tab in |params->browser| matching
+// the URL specified in |params|.
+int GetIndexOfSingletonTab(browser::NavigateParams* params) {
+  if (params->disposition != SINGLETON_TAB)
+    return -1;
+
+  // In case the URL was rewritten by the BrowserURLHandler we need to ensure
+  // that we do not open another URL that will get redirected to the rewritten
+  // URL.
+  GURL rewritten_url(params->url);
+  bool reverse_on_redirect = false;
+  BrowserURLHandler::RewriteURLIfNecessary(&rewritten_url,
+                                           params->browser->profile(),
+                                           &reverse_on_redirect);
+
+  for (int i = 0; i < params->browser->tab_count(); ++i) {
+    TabContents* tab = params->browser->GetTabContentsAt(i);
+    if (CompareURLsIgnoreRef(tab->GetURL(), params->url) ||
+        CompareURLsIgnoreRef(tab->GetURL(), rewritten_url)) {
+      params->target_contents = tab;
+      return i;
+    }
+  }
+  return -1;
+}
+
 // Returns a Browser that can host the navigation or tab addition specified in
 // |params|. This might just return the same Browser specified in |params|, or
 // some other if that Browser is deemed incompatible.
@@ -207,8 +250,7 @@ NavigateParams::NavigateParams(Browser* a_browser,
 NavigateParams::~NavigateParams() {
 }
 
-void Navigate(NavigateParams* params, NavigatorDelegate* delegate) {
-  DCHECK(delegate);
+void Navigate(NavigateParams* params) {
   params->browser = GetBrowserForDisposition(params);
   if (!params->browser)
     return;
@@ -266,27 +308,33 @@ void Navigate(NavigateParams* params, NavigatorDelegate* delegate) {
   }
 
   // Perform the actual navigation.
-  GURL url = params->url.is_empty() ? delegate->GetHomePage() : params->url;
+  GURL url = params->url.is_empty() ? params->browser->GetHomePage()
+                                    : params->url;
   params->target_contents->controller().LoadURL(url, params->referrer,
                                                 params->transition);
 
-  // Update the UI for the navigation.
   if (params->source_contents == params->target_contents) {
-    // The navigation occurred in the source tab.
-    delegate->UpdateUIForNavigationInTab(params->target_contents,
-                                         params->transition,
-                                         user_initiated);
+    // The navigation occurred in the source tab, so update the UI.
+    params->browser->UpdateUIForNavigationInTab(params->target_contents,
+                                                params->transition,
+                                                user_initiated);
   } else {
-    // The navigation occurred in some other tab yet to be added to the
-    // tabstrip. Add it now.
-    params->browser->tabstrip_model()->AddTabContents(
-        params->target_contents,
-        params->tabstrip_index,
-        params->transition,
-        params->tabstrip_add_types);
-    // Now that the |params->target_contents| is safely owned by the target
-    // Browser's TabStripModel, we can release ownership.
-    target_contents_owner.ReleaseOwnership();
+    // The navigation occurred in some other tab.
+    int singleton_index = GetIndexOfSingletonTab(params);
+    if (params->disposition == SINGLETON_TAB && singleton_index >= 0) {
+      // The navigation should re-select an existing tab in the target Browser.
+      params->browser->SelectTabContentsAt(singleton_index, user_initiated);
+    } else {
+      // The navigation should insert a new tab into the target Browser.
+      params->browser->tabstrip_model()->AddTabContents(
+          params->target_contents,
+          params->tabstrip_index,
+          params->transition,
+          params->tabstrip_add_types);
+      // Now that the |params->target_contents| is safely owned by the target
+      // Browser's TabStripModel, we can release ownership.
+      target_contents_owner.ReleaseOwnership();
+    }
   }
 }
 
diff --git a/chrome/browser/browser_navigator.h b/chrome/browser/browser_navigator.h
index 1998536..277830ef 100644
--- a/chrome/browser/browser_navigator.h
+++ b/chrome/browser/browser_navigator.h
@@ -4,6 +4,7 @@
 
 #ifndef CHROME_BROWSER_BROWSER_NAVIGATOR_H_
 #define CHROME_BROWSER_BROWSER_NAVIGATOR_H_
+#pragma once
 
 #include <string>
 
@@ -16,21 +17,6 @@ class Browser;
 class Profile;
 class TabContents;
 
-class NavigatorDelegate {
- public:
-  // Called by Navigate() after a navigation in |contents| has been performed.
-  virtual void UpdateUIForNavigationInTab(TabContents* contents,
-                                          PageTransition::Type transition,
-                                          bool user_initiated) = 0;
-
-  // Returns the URL of the home page. This URL will be loaded if the URL
-  // specified in NavigateParams is empty.
-  virtual GURL GetHomePage() const = 0;
-
- protected:
-  virtual ~NavigatorDelegate() {}
-};
-
 namespace browser {
 
 // Parameters that tell Navigate() what to do.
@@ -40,17 +26,17 @@ namespace browser {
 // Simple Navigate to URL in current tab:
 // browser::NavigateParams params(browser, GURL("http://www.google.com/"),
 //                                PageTransition::LINK);
-// browser::Navigate(&params, delegate);
+// browser::Navigate(&params);
 //
 // Open bookmark in new background tab:
 // browser::NavigateParams params(browser, url, PageTransition::AUTO_BOOKMARK);
 // params.disposition = NEW_BACKGROUND_TAB;
-// browser::Navigate(&params, delegate);
+// browser::Navigate(&params);
 //
 // Opens a popup TabContents:
 // browser::NavigateParams params(browser, popup_contents);
 // params.source_contents = source_contents;
-// browser::Navigate(&params, delegate);
+// browser::Navigate(&params);
 //
 // See browser_navigator_browsertest.cc for more examples.
 //
@@ -67,8 +53,8 @@ struct NavigateParams {
   GURL referrer;
 
   // [in]  A TabContents to be navigated or inserted into the target Browser's
-  //       tabstrip. If NULL, |url| or the homepage supplied by the
-  //       NavigatorDelegate will be used instead. Default is NULL.
+  //       tabstrip. If NULL, |url| or the homepage will be used instead.
+  //       Default is NULL.
   // [out] The TabContents in which the navigation occurred or that was
   //       inserted. Guaranteed non-NULL except for note below:
   // Note: If this field is set to NULL by the caller and Navigate() creates
@@ -135,7 +121,7 @@ struct NavigateParams {
 };
 
 // Navigates according to the configuration specified in |params|.
-void Navigate(NavigateParams* params, NavigatorDelegate* delegate);
+void Navigate(NavigateParams* params);
 
 }  // namespace browser
 
diff --git a/chrome/browser/browser_navigator_browsertest.cc b/chrome/browser/browser_navigator_browsertest.cc
index 844cd3b..4047524 100644
--- a/chrome/browser/browser_navigator_browsertest.cc
+++ b/chrome/browser/browser_navigator_browsertest.cc
@@ -16,19 +16,7 @@
 
 namespace {
 
-class BrowserNavigatorTest : public InProcessBrowserTest,
-                             public NavigatorDelegate {
- public:
-  // Overridden from NavigatorDelegate:
-  virtual void UpdateUIForNavigationInTab(TabContents* contents,
-                                          PageTransition::Type transition,
-                                          bool user_initiated) {
-    // Nothing needed.
-  }
-  virtual GURL GetHomePage() const {
-    return GURL("http://www.google.com/ig");
-  }
-
+class BrowserNavigatorTest : public InProcessBrowserTest {
  protected:
   GURL GetGoogleURL() const {
     return GURL("http://www.google.com/");
@@ -62,7 +50,7 @@ class BrowserNavigatorTest : public InProcessBrowserTest,
     GURL old_url = browser()->GetSelectedTabContents()->GetURL();
     browser::NavigateParams p(MakeNavigateParams());
     p.disposition = disposition;
-    browser::Navigate(&p, this);
+    browser::Navigate(&p);
 
     // Nothing should have happened as a result of Navigate();
     EXPECT_EQ(1, browser()->tab_count());
@@ -75,7 +63,7 @@ class BrowserNavigatorTest : public InProcessBrowserTest,
 // of the Browser remains the same and the current tab bears the loaded URL.
 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_CurrentTab) {
   browser::NavigateParams p(MakeNavigateParams());
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
   ui_test_utils::WaitForNavigationInCurrentTab(browser());
   EXPECT_EQ(GetGoogleURL(), browser()->GetSelectedTabContents()->GetURL());
   // We should have one window with one tab.
@@ -83,8 +71,48 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_CurrentTab) {
   EXPECT_EQ(1, browser()->tab_count());
 }
 
-IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_SingletonTab) {
-  // TODO(beng): TBD
+// This test verifies that a singleton tab is refocused if one is already open
+// in another or an existing window, or added if it is not.
+IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_SingletonTabExisting) {
+  GURL url("http://www.google.com/");
+  GURL singleton_url1("http://maps.google.com/");
+  browser()->AddSelectedTabWithURL(singleton_url1, PageTransition::LINK);
+  browser()->AddSelectedTabWithURL(url, PageTransition::LINK);
+
+  // We should have one browser with 3 tabs, the 3rd selected.
+  EXPECT_EQ(1u, BrowserList::size());
+  EXPECT_EQ(2, browser()->selected_index());
+
+  // Navigate to singleton_url1.
+  browser::NavigateParams p(MakeNavigateParams());
+  p.disposition = SINGLETON_TAB;
+  p.url = singleton_url1;
+  browser::Navigate(&p);
+
+  // The middle tab should now be selected.
+  EXPECT_EQ(browser(), p.browser);
+  EXPECT_EQ(1, browser()->selected_index());
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
+                       Disposition_SingletonTabNoneExisting) {
+  GURL url("http://www.google.com/");
+  GURL singleton_url1("http://maps.google.com/");
+
+  // We should have one browser with 3 tabs, the 3rd selected.
+  EXPECT_EQ(1u, BrowserList::size());
+  EXPECT_EQ(0, browser()->selected_index());
+
+  // Navigate to singleton_url1.
+  browser::NavigateParams p(MakeNavigateParams());
+  p.disposition = SINGLETON_TAB;
+  p.url = singleton_url1;
+  browser::Navigate(&p);
+
+  // We should now have 2 tabs, the 2nd one selected.
+  EXPECT_EQ(browser(), p.browser);
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_EQ(1, browser()->selected_index());
 }
 
 // This test verifies that when a navigation results in a foreground tab, the
@@ -94,7 +122,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewForegroundTab) {
   TabContents* old_contents = browser()->GetSelectedTabContents();
   browser::NavigateParams p(MakeNavigateParams());
   p.disposition = NEW_FOREGROUND_TAB;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
   EXPECT_NE(old_contents, browser()->GetSelectedTabContents());
   EXPECT_EQ(browser()->GetSelectedTabContents(), p.target_contents);
   EXPECT_EQ(2, browser()->tab_count());
@@ -106,7 +134,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewBackgroundTab) {
   TabContents* old_contents = browser()->GetSelectedTabContents();
   browser::NavigateParams p(MakeNavigateParams());
   p.disposition = NEW_BACKGROUND_TAB;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
   TabContents* new_contents = browser()->GetSelectedTabContents();
   // The selected tab should have remained unchanged, since the new tab was
   // opened in the background.
@@ -125,7 +153,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
                                              browser()->profile());
   browser::NavigateParams p(MakeNavigateParams(popup));
   p.disposition = NEW_FOREGROUND_TAB;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened the tab in a different browser since the
   // one we supplied didn't support additional tabs.
@@ -157,7 +185,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
       Browser::TYPE_POPUP, browser()->profile()->GetOffTheRecordProfile());
   browser::NavigateParams p(MakeNavigateParams(popup));
   p.disposition = NEW_FOREGROUND_TAB;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened the tab in a different browser since the
   // one we supplied didn't support additional tabs.
@@ -183,7 +211,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewPopup) {
   browser::NavigateParams p(MakeNavigateParams());
   p.disposition = NEW_POPUP;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened a new popup window.
   EXPECT_NE(browser(), p.browser);
@@ -204,7 +232,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
                                                    browser()->profile());
   browser::NavigateParams p(MakeNavigateParams(app_browser));
   p.disposition = NEW_POPUP;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened a new popup app window.
   EXPECT_NE(app_browser, p.browser);
@@ -231,7 +259,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewWindow) {
   browser::NavigateParams p(MakeNavigateParams());
   p.disposition = NEW_WINDOW;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened a new toplevel window.
   EXPECT_NE(browser(), p.browser);
@@ -249,7 +277,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewWindow) {
 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_Incognito) {
   browser::NavigateParams p(MakeNavigateParams());
   p.disposition = OFF_THE_RECORD;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened a new toplevel incognito window.
   EXPECT_NE(browser(), p.browser);
@@ -271,7 +299,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_IncognitoRefocus) {
                                 browser()->profile()->GetOffTheRecordProfile());
   browser::NavigateParams p(MakeNavigateParams());
   p.disposition = OFF_THE_RECORD;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened a new tab in the existing incognito window.
   EXPECT_NE(browser(), p.browser);
@@ -307,7 +335,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, TargetContents_ForegroundTab) {
   browser::NavigateParams p(MakeNavigateParams());
   p.disposition = NEW_FOREGROUND_TAB;
   p.target_contents = CreateTabContents();
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened the contents in a new foreground in the
   // current Browser.
@@ -326,7 +354,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, TargetContents_Popup) {
   p.disposition = NEW_POPUP;
   p.target_contents = CreateTabContents();
   p.window_bounds = gfx::Rect(10, 10, 500, 500);
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have opened a new popup window.
   EXPECT_NE(browser(), p.browser);
@@ -367,7 +395,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Tabstrip_InsertAtIndex) {
   p.disposition = NEW_FOREGROUND_TAB;
   p.tabstrip_index = 0;
   p.tabstrip_add_types = TabStripModel::ADD_FORCE_INDEX;
-  browser::Navigate(&p, this);
+  browser::Navigate(&p);
 
   // Navigate() should have inserted a new tab at slot 0 in the tabstrip.
   EXPECT_EQ(browser(), p.browser);