Fix navigation bugs, patch the right file, nuke an old file.

chrome/browser/ui/browser_navigator.cc

Index: chrome/browser/ui/browser_navigator.cc
diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc
index f7bd4838ecf7e7d2e52ee85c3bd758f45e9b4ad5..ee4138af49c0d41e771e2dcad79a2047fa6a3018 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -180,17 +180,27 @@ void NormalizeDisposition(browser::NavigateParams* params) {
     params->disposition = NEW_FOREGROUND_TAB;
   }
 
-  // Disposition trumps add types. ADD_SELECTED is a default, so we need to
-  // remove it if disposition implies the tab is going to open in the
-  // background.
-  if (params->disposition == NEW_BACKGROUND_TAB)
-    params->tabstrip_add_types &= ~TabStripModel::ADD_SELECTED;
-
-  // Code that wants to open a new window typically expects it to be shown
-  // automatically.
-  if (params->disposition == NEW_WINDOW || params->disposition == NEW_POPUP) {
-    params->show_window = true;
-    params->tabstrip_add_types |= TabStripModel::ADD_SELECTED;
+  switch (params->disposition) {
+    case NEW_BACKGROUND_TAB:
+      // Disposition trumps add types. ADD_SELECTED is a default, so we need to
+      // remove it if disposition implies the tab is going to open in the
+      // background.
+      params->tabstrip_add_types &= ~TabStripModel::ADD_SELECTED;
+      break;
+
+    case NEW_WINDOW:
+    case NEW_POPUP:
+      // Code that wants to open a new window typically expects it to be shown
+      // automatically.
+      params->show_window = true;
+      // Fall-through.
+    case NEW_FOREGROUND_TAB:
+    case SINGLETON_TAB:
+      params->tabstrip_add_types |= TabStripModel::ADD_SELECTED;
+      break;
+
+    default:
+      break;
   }
 }
 
@@ -281,11 +291,18 @@ NavigateParams::~NavigateParams() {
 }
 
 void Navigate(NavigateParams* params) {
+  Browser* browser = params->browser;
   params->browser = GetBrowserForDisposition(params);
   if (!params->browser)
     return;
   // Navigate() must not return early after this point.
 
+  if (browser != params->browser &&
+      params->browser->tabstrip_model()->empty()) {
+    // A new window has been created. So it needs to be displayed.
+    params->show_window = true;

[Ben Goodger (Google), 2010/11/17 15:57:50]

Did you evaluate all cases relating to startup (in browser.cc, browser_init.cc
etc) and verify that no one did any work post-Navigate() but before calling
browser->window()->Show()?

[sadrul, 2010/11/17 17:02:23]

I cannot honestly claim I covered all the places, but I tried my best. It looks
like window()->Show() is called (if at all) immediately after Navigate() (or
after AddBlankTab() etc. which essentially ends with Navigate()).

BrowserInit::LaunchWithProfile::OpenTabsInBrowser does a number of Navigate()
calls in a loop, and then calls Show(). But this change shouldn't affect that,
and |show_window| shouldn't be set since params->browser wouldn't be changed
within GetBrowserForDisposition or AdjustNavigateParamsForURL.

+  }
+
   // Make sure the Browser is shown if params call for it.
   ScopedBrowserDisplayer displayer(params);