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

chrome/browser/ui/browser_navigator.cc

 // 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/browser_navigator.h"
+#include "chrome/browser/ui/browser_navigator.h"
 
 #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"
 #include "chrome/browser/renderer_host/site_instance.h"
 #include "chrome/browser/status_bubble.h"
@@ skipping 157 matching lines @@
       (params->disposition == NEW_BACKGROUND_TAB ||
        params->disposition == CURRENT_TAB ||
        params->disposition == SINGLETON_TAB)) {
     params->disposition = NEW_FOREGROUND_TAB;
   }
   if (params->browser->profile()->IsOffTheRecord() &&
       params->disposition == OFF_THE_RECORD) {
     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;
+  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;
 
-  // 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;
+    case NEW_WINDOW:
+    case NEW_POPUP:
+    case NEW_FOREGROUND_TAB:

[Ben Goodger (Google), 2010/11/10 18:43:38]

What are the other implications of this? There are several cases on startup
where we create a window, add a foreground tab to it, do some other work and
then show the window. This makes it automatic and thus changes the show order. 

[sadrul, 2010/11/10 18:50:31]

Are there tests/scenarios I could try to reproduce those cases? Perhaps
|show_window| could be an enum (UNDECIDED, SHOW, DONOT_SHOW), and in these
cases, it will be set to DONOT_SHOW, in which case it will not be set to SHOW in
NormalizeDisposition.

+    case SINGLETON_TAB:
+      // Code that wants to open a new window typically expects it to be shown
+      // automatically.
+      params->show_window = true;
+      params->tabstrip_add_types |= TabStripModel::ADD_SELECTED;
+      break;
+
+    default:

[sky, 2010/11/10 19:18:47]

Can you deal with OFF_THE_RECORD mode too? I was going to fix it, but it looks
like you're moving everything so I won't try and complicate your life more.

The specific use case to test for is right click on a link and chose open in
incognito window. Currently we drop it on the floor (or rather create a browser
but never show it).

[sadrul, 2010/11/10 19:25:08]

I think you are referring to bug 62022? In which case, this CL fixes that
problem too.

[sky, 2010/11/10 19:30:12]

 2010/11/10 19:25:08, sadrul wrote:
Indeed I am. Glad to hear it!

+      break;
   }
 }
 
 // This class makes sure the Browser object held in |params| is made visible
 // by the time it goes out of scope, provided |params| wants it to be shown.
 class ScopedBrowserDisplayer {
  public:
   explicit ScopedBrowserDisplayer(browser::NavigateParams* params)
       : params_(params) {
   }
@@ skipping 169 matching lines @@
           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();
     }
   }
 }
 
 }  // namespace browser