Cleanup popup related browser navigation code.

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 cee32a872f622d569ce54cd7a0f16ef95a5deb6b..5cf73ec780e8f685d4a20e0f6e59503c1113614b 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -346,6 +346,7 @@ NavigateParams::NavigateParams(
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
+      user_gesture(true),
       path_behavior(RESPECT),
       browser(a_browser),
       profile(NULL) {
@@ -360,6 +361,7 @@ NavigateParams::NavigateParams(Browser* a_browser,
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
+      user_gesture(true),
       path_behavior(RESPECT),
       browser(a_browser),
       profile(NULL) {
@@ -372,6 +374,10 @@ void Navigate(NavigateParams* params) {
   Browser* source_browser = params->browser;
   AdjustNavigateParamsForURL(params);
 
+  // Adjustments based on size and type of window; may affect disposition.
+  if (source_browser && source_browser->window())
+    source_browser->window()->AdjustNavigateParams(params);

[Ben Goodger (Google), 2011/04/20 19:10:32]

I worry that this connection couples the BrowserWindow too closely to the
navigator. Can we restate this more generally? ... 

e.g. 

if (source_browser && source_browser->window())
  disposition =
source_browser->window()->GetDispositionForPopupBounds(params->bounds ...)

[stevenjb, 2011/04/20 20:43:40]

Sounds reasonable to me. Done.

On 2011/04/20 19:10:32, Ben Goodger wrote:

+
   params->browser = GetBrowserForDisposition(params);
   if (!params->browser)
     return;
@@ -384,13 +390,6 @@ void Navigate(NavigateParams* params) {
     params->referrer = GURL();
   }
 
-  if (params->window_action == browser::NavigateParams::NO_ACTION &&
-      source_browser != params->browser &&
-      params->browser->tabstrip_model()->empty()) {
-    // A new window has been created. So it needs to be displayed.
-    params->window_action = browser::NavigateParams::SHOW_WINDOW;
-  }
-
   // Make sure the Browser is shown if params call for it.
   ScopedBrowserDisplayer displayer(params);
 
@@ -401,6 +400,20 @@ void Navigate(NavigateParams* params) {
   // Some dispositions need coercion to base types.
   NormalizeDisposition(params);
 
+  // If a new window has been created, it needs to be displayed.
+  if (params->window_action == browser::NavigateParams::NO_ACTION &&
+      source_browser != params->browser &&
+      params->browser->tabstrip_model()->empty()) {
+    params->window_action = browser::NavigateParams::SHOW_WINDOW;
+  }
+
+  // If we create a popup window from a non user-gesture, don't activate it.
+  if (params->window_action == browser::NavigateParams::SHOW_WINDOW &&
+      params->disposition == NEW_POPUP &&
+      params->user_gesture == false) {
+    params->window_action = browser::NavigateParams::SHOW_WINDOW_INACTIVE;
+  }
+
   // Determine if the navigation was user initiated. If it was, we need to
   // inform the target TabContents, and we may need to update the UI.
   PageTransition::Type base_transition =