Replace --google-base-suggest-url and --instant-url with --google-base-url.

chrome/browser/search/search.cc

Index: chrome/browser/search/search.cc
===================================================================
--- chrome/browser/search/search.cc	(revision 206485)
+++ chrome/browser/search/search.cc	(working copy)
@@ -100,12 +100,6 @@
            other_url.SchemeIs(chrome::kHttpScheme)));
 }
 
-bool IsCommandLineInstantURL(const GURL& url) {
-  const CommandLine* cl = CommandLine::ForCurrentProcess();
-  const GURL instant_url(cl->GetSwitchValueASCII(switches::kInstantURL));
-  return instant_url.is_valid() && MatchesOrigin(url, instant_url);
-}
-
 bool MatchesAnySearchURL(const GURL& url, TemplateURL* template_url) {
   GURL search_url =
       TemplateURLRefToGURL(template_url->url_ref(), kDisableStartMargin);
@@ -194,42 +188,47 @@
   return instant_service->IsInstantProcess(process_host->GetID());
 }
 
+// Returns true if |url| starts with a command-line-specified Google base URL.
+bool StartsWithCommandLineGoogleBaseURL(const GURL& url) {
+  const std::string base_url(CommandLine::ForCurrentProcess()->
+      GetSwitchValueASCII(switches::kGoogleBaseURL));
+  // We do a direct string comparison because we're not worried about trying to
+  // support e.g. one URL saying ":80" after the hostname.
+  return !base_url.empty() &&
+      StartsWithASCII(url.possibly_invalid_spec(), base_url, true);
+}
+
+// Returns true if |url| passes some basic checks that must succeed for it to be
+// usable as an instant URL:
+// (1) It contains the search terms replacement key of |default_turl|, which is
+//     expected to be the TemplateURL* for the default search provider.
+// (2) Either it has a secure scheme, or else the user has manually specified a
+//     --google-base-url and it uses that base URL.  (This allows testers to use
+//     --google-base-url to point at non-HTTPS servers, which eases testing.)
+bool IsSuitableURLForInstant(const GURL& url, const TemplateURL* default_turl) {

[Jered, 2013/06/25 16:30:39]

default_turl -> search_template or template_url ("turl" sounds weird to me and I
wasn't immediately sure what it was).

[Peter Kasting, 2013/06/25 23:06:36]

Changed, though I actually preferred the old terminology (and would like to
switch the rest of the file to it).  The TemplateURL, Sync, Autocomplete, etc.
subsystems commonly refer to TemplateURLs as "turls" and "default_turl" is the
common way of indicating "this is the DSP's TemplateURL".

+  return default_turl->HasSearchTermsReplacementKey(url) &&
+      (url.SchemeIsSecure() || StartsWithCommandLineGoogleBaseURL(url));
+}
+
 // Returns true if |url| can be used as an Instant URL for |profile|.
 bool IsInstantURL(const GURL& url, Profile* profile) {
+  if (!url.is_valid())
+    return false;
+
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
   if (!template_url)
     return false;
 
-  const TemplateURLRef& instant_url_ref = template_url->instant_url_ref();
   const bool extended_api_enabled = IsInstantExtendedAPIEnabled();
-  GURL effective_url = url;
-
-  if (IsCommandLineInstantURL(url))
-    effective_url = CoerceCommandLineURLToTemplateURL(url, instant_url_ref,
-                                                      kDisableStartMargin);
-
-  if (!effective_url.is_valid())
+  if (extended_api_enabled && !IsSuitableURLForInstant(url, template_url))
     return false;
 
-  if (extended_api_enabled && !effective_url.SchemeIsSecure())
-    return false;
-
-  if (extended_api_enabled &&
-      !template_url->HasSearchTermsReplacementKey(effective_url))
-    return false;
-
+  const TemplateURLRef& instant_url_ref = template_url->instant_url_ref();
   const GURL instant_url =
       TemplateURLRefToGURL(instant_url_ref, kDisableStartMargin);
-  if (!instant_url.is_valid())
-    return false;
-
-  if (MatchesOriginAndPath(effective_url, instant_url))
-    return true;
-
-  if (extended_api_enabled && MatchesAnySearchURL(effective_url, template_url))
-    return true;
-
-  return false;
+  return instant_url.is_valid() &&
+      (MatchesOriginAndPath(url, instant_url) ||
+       (extended_api_enabled && MatchesAnySearchURL(url, template_url)));
 }
 
 string16 GetSearchTermsImpl(const content::WebContents* contents,
@@ -312,21 +311,11 @@
   return false;
 }
 
-string16 GetSearchTermsFromURL(Profile* profile, const GURL& in_url) {
-  GURL url(in_url);
+string16 GetSearchTermsFromURL(Profile* profile, const GURL& url) {
   string16 search_terms;
-
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
-  if (!template_url)
-    return string16();
-
-  if (IsCommandLineInstantURL(url))
-    url = CoerceCommandLineURLToTemplateURL(url, template_url->url_ref(),
-                                            kDisableStartMargin);
-
-  if (url.SchemeIsSecure() && template_url->HasSearchTermsReplacementKey(url))
+  if (template_url && IsSuitableURLForInstant(url, template_url))
     template_url->ExtractSearchTermsFromURL(url, &search_terms);
-
   return search_terms;
 }
 
@@ -483,37 +472,37 @@
   if (!IsInstantCheckboxEnabled(profile))
     return GURL();
 
+  // In non-extended mode, the checkbox must be checked.

[Jered, 2013/06/25 16:30:39]

There is no checkbox. Can you update this comment and the accompanying logic to
match reality?

[Peter Kasting, 2013/06/25 23:06:36]

No.  There are a variety of functions throughout this file that refer to the
checkbox, and I am not knowledgeable enough in this area to sort all of them
out.  This should be something you guys clean up.

   const bool extended_api_enabled = IsInstantExtendedAPIEnabled();
-
-  // In non-extended mode, the checkbox must be checked.
   if (!extended_api_enabled && !IsInstantCheckboxChecked(profile))
     return GURL();
 
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
-  CommandLine* cl = CommandLine::ForCurrentProcess();
-  if (cl->HasSwitch(switches::kInstantURL)) {
-    GURL instant_url(cl->GetSwitchValueASCII(switches::kInstantURL));
-    if (extended_api_enabled) {
-      // Extended mode won't work if the search terms replacement key is absent.
-      GURL coerced_url = CoerceCommandLineURLToTemplateURL(
-          instant_url, template_url->instant_url_ref(), start_margin);
-      if (!template_url->HasSearchTermsReplacementKey(coerced_url))
-        return GURL();
-    }
-    return instant_url;
-  }
-
   GURL instant_url =
       TemplateURLRefToGURL(template_url->instant_url_ref(), start_margin);
-  if (extended_api_enabled && !instant_url.SchemeIsSecure()) {
-    // Extended mode requires HTTPS. Force it if necessary.
-    const std::string secure_scheme = chrome::kHttpsScheme;
-    GURL::Replacements replacements;
+
+  GURL::Replacements replacements;
+
+  // Extended mode requires HTTPS.  Force it unless the base URL was overridden
+  // on the command line, in which case we allow HTTP (see comments on
+  // IsSuitableURLForInstant()).
+  const std::string secure_scheme(chrome::kHttpsScheme);
+  if (extended_api_enabled && !instant_url.SchemeIsSecure() &&
+      !StartsWithCommandLineGoogleBaseURL(instant_url))
     replacements.SetSchemeStr(secure_scheme);
-    instant_url = instant_url.ReplaceComponents(replacements);
+
+  // If the user specified additional query params on the command line, add
+  // them.
+  std::string query_params(CommandLine::ForCurrentProcess()->
+    GetSwitchValueASCII(switches::kExtraSearchQueryParams));

[Jered, 2013/06/25 16:30:39]

Indent two more spaces.

[Peter Kasting, 2013/06/25 23:06:36]

Done.

+  if (!query_params.empty()) {
+    const std::string existing_query_params(instant_url.query());
+    if (!existing_query_params.empty())
+      query_params += "&" + existing_query_params;
+    replacements.SetQueryStr(query_params);

[Jered, 2013/06/25 16:30:39]

Developers might find it confusing that --extra-search-query-params applies only
to Instant, and not to anything else we might do with a search template URL.

[Peter Kasting, 2013/06/25 23:06:36]

I haven't yet made it apply to the search URL, but that's coming before I check
in.

   }
 
-  return instant_url;
+  return instant_url.ReplaceComponents(replacements);
 }
 
 GURL GetLocalInstantURL(Profile* profile) {
@@ -707,27 +696,6 @@
   return !!GetUInt64ValueForFlagWithDefault(flag, default_value ? 1 : 0, flags);
 }
 
-// Coerces the commandline Instant URL to look like a template URL, so that we
-// can extract search terms from it.
-GURL CoerceCommandLineURLToTemplateURL(const GURL& instant_url,
-                                       const TemplateURLRef& ref,
-                                       int start_margin) {
-  GURL search_url = TemplateURLRefToGURL(ref, start_margin);
-
-  // NOTE(samarth): GURL returns temporaries which we must save because
-  // GURL::Replacements expects the replacements to live until
-  // ReplaceComponents is called.
-  const std::string search_scheme = chrome::kHttpsScheme;
-  const std::string search_host = search_url.host();
-  const std::string search_port = search_url.port();
-
-  GURL::Replacements replacements;
-  replacements.SetSchemeStr(search_scheme);
-  replacements.SetHostStr(search_host);
-  replacements.SetPortStr(search_port);
-  return instant_url.ReplaceComponents(replacements);
-}
-
 bool DefaultSearchProviderSupportsInstant(Profile* profile) {
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
   if (!template_url)