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 54edd144c7823c7e45588c0e5ad516685f562ace..2dfb99839a03c98af75972d35840375b2322ff61 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -178,6 +178,7 @@ void NormalizeDisposition(browser::NavigateParams* params) {
   if (params->browser->profile()->IsOffTheRecord() &&
       params->disposition == OFF_THE_RECORD) {
     params->disposition = NEW_FOREGROUND_TAB;
+    params->show_window = true;
   }
 
   // Disposition trumps add types. ADD_SELECTED is a default, so we need to
@@ -185,6 +186,8 @@ void NormalizeDisposition(browser::NavigateParams* params) {
   // background.
   if (params->disposition == NEW_BACKGROUND_TAB)
     params->tabstrip_add_types &= ~TabStripModel::ADD_SELECTED;
+  else if (params->disposition == NEW_FOREGROUND_TAB)

[Evan Stade, 2010/11/09 20:29:23]

maybe NEW_FOREGROUND_TAB can be added to the dispositions below on L194. Also,
can you update the comment in the header about this?

[sadrul, 2010/11/09 20:50:54]

I thought about doing a fallthrough switch-case on this:

switch (params->disposition) {
  case NEW_BACKGROUND_TAB:
    params->tabstrip_add_types &= ~TabStripModel::ADD_SELECTED;
    break;
  case NEW_WINDOW:
  case NEW_POPUP:
    params->show_window = true;
    // Fall-through
  case NEW_FOREGROUND_TAB:
    params->tabstrip_add_types |= TabStripModel::ADD_SELECTED;
    break;

  default:
    break;
}

How does that sound?

[Evan Stade, 2010/11/09 20:56:18]

what's wrong with showing the window for NEW_FOREGROUND? I think Show is
harmless if the window's already showing (actually, it probably Activates the
window, which seems desirable).

if there were a context menu item like "open in incognito" but instead it were
called "open in normal", and you didn't have any normal windows when you
selected it in an OTR window, you'd want to show the new window you had to
create.

[sadrul, 2010/11/09 21:06:39]

[snip]
That makes sense to me (and |show_window| does activate the window if already
exists). However, I do not know what 'activate the window' may mean for
different window-managers, and I am not familiar with this code enough to know
if this can cause problems for some. But if it sounds like an OK change to
you/Ben, I do not personally have a problem with it.

[sadrul, 2010/11/10 05:07:57]

I have gone ahead and made this change. This essentially makes a single change
fix both 62022 and 62137!

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