Cleanup popup related browser navigation code.

chrome/browser/ui/browser_navigator.cc

 // Copyright (c) 2011 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/ui/browser_navigator.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "chrome/browser/browser_url_handler.h"
@@ skipping 328 matching lines @@
     const GURL& a_url,
     PageTransition::Type a_transition)
     : url(a_url),
       target_contents(NULL),
       source_contents(NULL),
       disposition(CURRENT_TAB),
       transition(a_transition),
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
+      user_gesture(true),
       path_behavior(RESPECT),
       browser(a_browser),
       profile(NULL) {
 }
 
 NavigateParams::NavigateParams(Browser* a_browser,
                                TabContentsWrapper* a_target_contents)
     : target_contents(a_target_contents),
       source_contents(NULL),
       disposition(CURRENT_TAB),
       transition(PageTransition::LINK),
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
+      user_gesture(true),
       path_behavior(RESPECT),
       browser(a_browser),
       profile(NULL) {
 }
 
 NavigateParams::~NavigateParams() {
 }
 
 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;
   // Navigate() must not return early after this point.
 
   if (GetSourceProfile(params, source_browser) != params->browser->profile()) {
     // A tab is being opened from a link from a different profile, we must reset
     // source information that may cause state to be shared.
     params->source_contents = NULL;
     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);
 
   // Makes sure any TabContents created by this function is destroyed if
   // not properly added to a tab strip.
   ScopedTargetContentsOwner target_contents_owner(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 =
       PageTransition::StripQualifier(params->transition);
   bool user_initiated = base_transition == PageTransition::TYPED ||
                         base_transition == PageTransition::AUTO_BOOKMARK;
 
   // Check if this is a singleton tab that already exists
   int singleton_index = GetIndexOfSingletonTab(params);
 
@@ skipping 78 matching lines @@
           params->url, params->referrer, params->transition);
     }
 
     // If the singleton tab isn't already selected, select it.
     if (params->source_contents != params->target_contents)
       params->browser->ActivateTabAt(singleton_index, user_initiated);
   }
 }
 
 }  // namespace browser