Make chrome::Navigate take in a Browser* that is never NULL. That function doesn't have a context, …

chrome/browser/ui/browser_navigator.cc

 // Copyright (c) 2012 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 "base/stringprintf.h"
@@ skipping 56 matching lines @@
 // technically an incognito window, these URLs are allowed.
 // Returns true on success. Otherwise, if changing params leads the browser into
 // an erroneous state, returns false.
 bool AdjustNavigateParamsForURL(chrome::NavigateParams* params) {
   if (params->target_contents != NULL ||
       chrome::IsURLAllowedInIncognito(params->url) ||
       Profile::IsGuestSession()) {
     return true;
   }
 
-  Profile* profile =
-      params->browser ? params->browser->profile() : params->profile;
+  Profile* profile = params->browser->profile();
 
   if (profile->IsOffTheRecord() || params->disposition == OFF_THE_RECORD) {
     profile = profile->GetOriginalProfile();
 
     // If incognito is forced, we punt.
     PrefService* prefs = profile->GetPrefs();
     if (prefs && IncognitoModePrefs::GetAvailability(prefs) ==
             IncognitoModePrefs::FORCED) {
       return false;
     }
 
     params->disposition = SINGLETON_TAB;
-    params->profile = profile;
     params->browser = browser::FindOrCreateTabbedBrowser(profile);
     params->window_action = chrome::NavigateParams::SHOW_WINDOW;
   }
 
   return true;
 }
 
 // Returns a Browser that can host the navigation or tab addition specified in
 // |params|. This might just return the same Browser specified in |params|, or
 // some other if that Browser is deemed incompatible.
 Browser* GetBrowserForDisposition(chrome::NavigateParams* params) {
   // If no source TabContents was specified, we use the selected one from
   // the target browser. This must happen first, before
   // GetBrowserForDisposition() has a chance to replace |params->browser| with
   // another one.
-  if (!params->source_contents && params->browser)
+  if (!params->source_contents)
     params->source_contents = chrome::GetActiveTabContents(params->browser);
 
-  Profile* profile =
-      params->browser ? params->browser->profile() : params->profile;
+  Profile* profile = params->browser->profile();
 
   switch (params->disposition) {
     case CURRENT_TAB:
-      if (!params->browser && profile) {
-        // We specified a profile instead of a browser; find or create one.
-        params->browser = browser::FindOrCreateTabbedBrowser(profile);
-      }
       return params->browser;
     case SINGLETON_TAB:
     case NEW_FOREGROUND_TAB:
     case NEW_BACKGROUND_TAB:
       // See if we can open the tab in the window this navigator is bound to.
-      if (params->browser && WindowCanOpenTabs(params->browser))
+      if (WindowCanOpenTabs(params->browser))
         return params->browser;
       // Find a compatible window and re-execute this command in it. Otherwise
       // re-run with NEW_WINDOW.
-      if (profile)
-        return GetOrCreateBrowser(profile);
-      return NULL;
+      return GetOrCreateBrowser(profile);
     case NEW_POPUP: {
       // Make a new popup window.
-      if (profile) {
-        // Coerce app-style if |params->browser| or |source| represents an app.
-        std::string app_name;
-        if (!params->extension_app_id.empty()) {
-          app_name = web_app::GenerateApplicationNameFromExtensionId(
-              params->extension_app_id);
-        } else if (params->browser && !params->browser->app_name().empty()) {
-          app_name = params->browser->app_name();
-        } else if (params->source_contents &&
-                   params->source_contents->extension_tab_helper()->is_app()) {
-          app_name = web_app::GenerateApplicationNameFromExtensionId(
-              params->source_contents->extension_tab_helper()->
-                  extension_app()->id());
-        }
-        if (app_name.empty()) {
-          Browser::CreateParams browser_params(Browser::TYPE_POPUP, profile);
-          browser_params.initial_bounds = params->window_bounds;
-          return Browser::CreateWithParams(browser_params);
-        } else {
-          return Browser::CreateWithParams(
-              Browser::CreateParams::CreateForApp(
-                  Browser::TYPE_POPUP, app_name, params->window_bounds,
-                  profile));
-        }
+      // Coerce app-style if |source| represents an app.
+      std::string app_name;
+      if (!params->extension_app_id.empty()) {
+        app_name = web_app::GenerateApplicationNameFromExtensionId(
+            params->extension_app_id);
+      } else if (!params->browser->app_name().empty()) {
+        app_name = params->browser->app_name();
+      } else if (params->source_contents &&
+                 params->source_contents->extension_tab_helper()->is_app()) {
+        app_name = web_app::GenerateApplicationNameFromExtensionId(
+            params->source_contents->extension_tab_helper()->
+                extension_app()->id());
       }
-      return NULL;
+      if (app_name.empty()) {
+        Browser::CreateParams browser_params(Browser::TYPE_POPUP, profile);
+        browser_params.initial_bounds = params->window_bounds;
+        return Browser::CreateWithParams(browser_params);
+      }
+
+      return Browser::CreateWithParams(

[Mihai Parparita -not on Chrome, 2012/07/13 21:09:27]

Given that NEW_POPUP ends up creating a Browser, it seems odd to require one to
be passed in.

We'd like to be able to open a popup while in an app to show the sign-in UI
(https://chromiumcodereview.appspot.com/10701041/diff/11002/chrome/browser/ext...),
and there may not be a Browser that we can pass in.

[jam, 2012/07/13 21:18:39]

There are a number of ways that this function creates a new Browser. The passed
in Browser is used to give it context when it does.
I'm not familiar with that code. In that case, could there be no Browser* at
all?

+          Browser::CreateParams::CreateForApp(
+              Browser::TYPE_POPUP, app_name, params->window_bounds,
+              profile));
     }
-    case NEW_WINDOW:
+    case NEW_WINDOW: {
       // Make a new normal browser window.
-      if (profile) {
-        Browser* browser = new Browser(Browser::TYPE_TABBED, profile);
-        browser->InitBrowserWindow();
-        return browser;
-      }
-      return NULL;
+      Browser* browser = new Browser(Browser::TYPE_TABBED, profile);
+      browser->InitBrowserWindow();
+      return browser;
+    }
     case OFF_THE_RECORD:
       // Make or find an incognito window.
-      if (profile)
-        return GetOrCreateBrowser(profile->GetOffTheRecordProfile());
-      return NULL;
+      return GetOrCreateBrowser(profile->GetOffTheRecordProfile());
     // The following types all result in no navigation.
     case SUPPRESS_OPEN:
     case SAVE_TO_DISK:
     case IGNORE_ACTION:
       return NULL;
     default:
       NOTREACHED();
   }
   return NULL;
 }
@@ skipping 41 matching lines @@
 }
 
 // Obtain the profile used by the code that originated the Navigate() request.
 // |source_browser| represents the Browser that was supplied in |params| before
 // it was modified.
 Profile* GetSourceProfile(chrome::NavigateParams* params,
                           Browser* source_browser) {
   if (params->source_contents)
     return params->source_contents->profile();
 
-  if (source_browser)
-    return source_browser->profile();
-
-  if (params->profile)
-    return params->profile;
-
-  // We couldn't find one in any of the source metadata, so we'll fall back to
-  // the profile associated with the target browser.
-  return params->browser->profile();
+  return source_browser->profile();
 }
 
 void LoadURLInContents(WebContents* target_contents,
                        const GURL& url,
                        chrome::NavigateParams* params,
                        const std::string& extra_headers) {
   if (params->transferred_global_request_id != GlobalRequestID()) {
     target_contents->GetController().TransferURL(
         url,
         params->referrer,
@@ skipping 63 matching lines @@
  private:
   chrome::NavigateParams* params_;
   scoped_ptr<TabContents> target_contents_owner_;
   DISALLOW_COPY_AND_ASSIGN(ScopedTargetContentsOwner);
 };
 
 void InitializeExtraHeaders(chrome::NavigateParams* params,
                             Profile* profile,
                             std::string* extra_headers) {
 #if defined(ENABLE_RLZ)
-  if (!profile)
-    profile = params->profile;
-
   // If this is a home page navigation, check to see if the home page is
   // set to Google and add RLZ HTTP headers to the request.  This is only
   // done if Google was the original home page, and not changed afterwards by
   // the user.
   if (profile &&
       (params->transition & content::PAGE_TRANSITION_HOME_PAGE) != 0) {
     PrefService* pref_service = profile->GetPrefs();
     if (pref_service) {
       if (!pref_service->GetBoolean(prefs::kHomePageChanged)) {
         std::string homepage = pref_service->GetString(prefs::kHomePage);
@@ skipping 35 matching lines @@
       source_contents(NULL),
       disposition(CURRENT_TAB),
       transition(a_transition),
       is_renderer_initiated(false),
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
       user_gesture(true),
       path_behavior(RESPECT),
       ref_behavior(IGNORE_REF),
-      browser(a_browser),
-      profile(NULL) {
+      browser(a_browser) {
 }
 
 NavigateParams::NavigateParams(Browser* a_browser,
                                TabContents* a_target_contents)
     : target_contents(a_target_contents),
       source_contents(NULL),
       disposition(CURRENT_TAB),
       transition(content::PAGE_TRANSITION_LINK),
       is_renderer_initiated(false),
       tabstrip_index(-1),
       tabstrip_add_types(TabStripModel::ADD_ACTIVE),
       window_action(NO_ACTION),
       user_gesture(true),
       path_behavior(RESPECT),
       ref_behavior(IGNORE_REF),
-      browser(a_browser),
-      profile(NULL) {
+      browser(a_browser) {
 }
 
 NavigateParams::~NavigateParams() {
 }
 
 void Navigate(NavigateParams* params) {
   Browser* source_browser = params->browser;
 
   if (!AdjustNavigateParamsForURL(params))
     return;
 
   // The browser window may want to adjust the disposition.
-  if (params->disposition == NEW_POPUP &&
-      (source_browser && source_browser->window())) {
+  if (params->disposition == NEW_POPUP && source_browser->window()) {
     params->disposition =
         source_browser->window()->GetDispositionForPopupBounds(
             params->window_bounds);
   }
 
   // Adjust disposition for the navigation happending in the sad page of the
   // panel window.
   if (params->source_contents &&
       params->source_contents->web_contents()->IsCrashed() &&
       params->disposition == CURRENT_TAB &&
@@ skipping 158 matching lines @@
   }
 
   if (singleton_index >= 0) {
     WebContents* target =
         chrome::GetWebContentsAt(params->browser, singleton_index);
 
     if (target->IsCrashed()) {
       target->GetController().Reload(true);
     } else if (params->path_behavior == NavigateParams::IGNORE_AND_NAVIGATE &&
         target->GetURL() != params->url) {
-      InitializeExtraHeaders(params, NULL, &extra_headers);
+      TabContents* target_tab = TabContents::FromWebContents(target);
+      InitializeExtraHeaders(params, target_tab->profile(), &extra_headers);
       LoadURLInContents(target, params->url, params, extra_headers);
     }
 
     // If the singleton tab isn't already selected, select it.
     if (params->source_contents != params->target_contents)
       chrome::ActivateTabAt(params->browser, singleton_index, user_initiated);
   }
 
   if (params->disposition != CURRENT_TAB) {
     content::NotificationService::current()->Notify(
@@ skipping 11 matching lines @@
   return !(url.scheme() == chrome::kChromeUIScheme &&
       (url.host() == chrome::kChromeUISettingsHost ||
        url.host() == chrome::kChromeUISettingsFrameHost ||
        url.host() == chrome::kChromeUIExtensionsHost ||
        url.host() == chrome::kChromeUIBookmarksHost ||
        url.host() == chrome::kChromeUISyncPromoHost ||
        url.host() == chrome::kChromeUIUberHost));
 }
 
 }  // namespace chrome