Consolidate all NavigationController::LoadURL and family functions

chrome/browser/instant/instant_loader.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/instant/instant_loader.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 738 matching lines @@
       }
     } else {
       LoadInstantURL(template_url, transition_type, user_text_, verbatim,
           override_user_agent);
     }
   } else {
     DCHECK(template_url_id_ == 0);
     preview_tab_contents_delegate_->PrepareForNewLoad();
     frame_load_observer_.reset(NULL);
 
+    content::NavigationController::LoadURLParams load_url_params(url_);
+    load_url_params.transition_type = transition_type;
+    load_url_params.should_inherit_user_agent_override = false;

[gone, 2012/08/03 20:17:37]

Seems like this should automatically be set to false whenever the
override_user_agent boolean is set.  Change the direct setting to a function
that does this?

[boliu, 2012/08/03 20:46:27]

I'm against doing this kind of hidden "magic" logic in a simple value struct.
The struct is simply holding parameter values. If patterns like this emerge
throughout the code base, then we could add simple helper methods that make the
intention clear.

But I think in the case of user agent override, it should really be a tri-state
variable: override to true, override to false, inherit. I'll make this change
next.

+    load_url_params.is_overriding_user_agent = override_user_agent;
     preview_contents_->web_contents()->GetController().
-        LoadURLWithUserAgentOverride(url_, content::Referrer(), transition_type,
-            false, std::string(), override_user_agent);
+        LoadURLWithParams(load_url_params);
   }
   return true;
 }
 
 void InstantLoader::SetOmniboxBounds(const gfx::Rect& bounds) {
   if (omnibox_bounds_ == bounds)
     return;
 
   // Don't update the page while the mouse is down. http://crbug.com/71952
   if (IsPointerDownFromActivate())
@@ skipping 397 matching lines @@
   // really have the search params, but we need to use the replace
   // functionality so that embeded tags (like {google:baseURL}) are escaped
   // correctly.
   // TODO(sky): having to use a replaceable url is a bit of a hack here.
   GURL instant_url(template_url->instant_url_ref().ReplaceSearchTerms(
       TemplateURLRef::SearchTermsArgs(string16())));
   CommandLine* cl = CommandLine::ForCurrentProcess();
   if (cl->HasSwitch(switches::kInstantURL))
     instant_url = GURL(cl->GetSwitchValueASCII(switches::kInstantURL));
 
+  content::NavigationController::LoadURLParams load_url_params(
+      instant_url);
+  load_url_params.transition_type = transition_type;
+  load_url_params.should_inherit_user_agent_override = false;
+  load_url_params.is_overriding_user_agent = override_user_agent;
   preview_contents_->web_contents()->GetController().
-      LoadURLWithUserAgentOverride(instant_url, content::Referrer(),
-          transition_type, false, std::string(), override_user_agent);
+      LoadURLWithParams(load_url_params);
 
   RenderViewHost* host = preview_contents_->web_contents()->GetRenderViewHost();
   preview_contents_->web_contents()->WasHidden();
 
   // If user_text is empty, this must be a preload of the search homepage. In
   // that case, send down a SearchBoxResize message, which will switch the page
   // to "search results" UI. This avoids flicker when the page is shown with
   // results. In addition, we don't want the page accidentally causing the
   // preloaded page to be displayed yet (by calling setSuggestions), so don't
   // send a SearchBoxChange message.
   if (user_text.empty()) {
     host->Send(new ChromeViewMsg_SearchBoxResize(
         host->GetRoutingID(), GetOmniboxBoundsInTermsOfPreview()));
   } else {
     host->Send(new ChromeViewMsg_SearchBoxChange(
         host->GetRoutingID(), user_text, verbatim, 0, 0));
   }
 
   frame_load_observer_.reset(new FrameLoadObserver(
       this, preview_contents()->web_contents(), user_text, verbatim));
 }