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

chrome/browser/search/search.cc

 // Copyright 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/search/search.h"
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
@@ skipping 82 matching lines @@
 }
 
 bool MatchesOrigin(const GURL& my_url, const GURL& other_url) {
   return my_url.host() == other_url.host() &&
          my_url.port() == other_url.port() &&
          (my_url.scheme() == other_url.scheme() ||
           (my_url.SchemeIs(chrome::kHttpsScheme) &&
            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);
   if (search_url.is_valid() && MatchesOriginAndPath(url, search_url))
     return true;
 
   // "URLCount() - 1" because we already tested url_ref above.
   for (size_t i = 0; i < template_url->URLCount() - 1; ++i) {
     TemplateURLRef ref(template_url, i);
     search_url = TemplateURLRefToGURL(ref, kDisableStartMargin);
@@ skipping 68 matching lines @@
     return false;
 
   const InstantService* instant_service =
       InstantServiceFactory::GetForProfile(profile);
   if (!instant_service)
     return false;
 
   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,
                             const content::NavigationEntry* entry) {
   // For security reasons, don't extract search terms if the page is not being
   // rendered in the privileged Instant renderer process. This is to protect
   // against a malicious page somehow scripting the search results page and
   // faking search terms in the URL. Random pages can't get into the Instant
   // renderer and scripting doesn't work cross-process, so if the page is in
   // the Instant process, we know it isn't being exploited.
@@ skipping 62 matching lines @@
 
   FieldTrialFlags flags;
   if (GetFieldTrialInfo(
           base::FieldTrialList::FindFullName(kInstantExtendedFieldTrialName),
           &flags, NULL)) {
     return GetBoolValueForFlagWithDefault(kLocalOnlyFlagName, false, flags);
   }
   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;
 }
 
 string16 GetSearchTermsFromNavigationEntry(
     const content::NavigationEntry* entry) {
   string16 search_terms;
   if (entry)
     entry->GetExtraData(sessions::kSearchTermsKey, &search_terms);
   return search_terms;
 }
@@ skipping 136 matching lines @@
     return l10n_util::GetStringUTF16(IDS_INSTANT_CHECKBOX_PREDICTION_DISABLED);
 
   DCHECK(IsInstantCheckboxEnabled(profile));
   return l10n_util::GetStringUTF16(IDS_INSTANT_CHECKBOX_ENABLED);
 }
 
 GURL GetInstantURL(Profile* profile, int start_margin) {
   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);
+
+  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);
+
+  // 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.

   }
 
-  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;
-    replacements.SetSchemeStr(secure_scheme);
-    instant_url = instant_url.ReplaceComponents(replacements);
-  }
-
-  return instant_url;
+  return instant_url.ReplaceComponents(replacements);
 }
 
 GURL GetLocalInstantURL(Profile* profile) {
   const TemplateURL* default_provider =
       GetDefaultSearchProviderTemplateURL(profile);
 
   if (default_provider &&
       (TemplateURLPrepopulateData::GetEngineType(default_provider->url()) ==

[samarth, 2013/06/21 23:57:25]

It's not terribly important, but this check is failing with a custom
--google-base-url, meaning you get the non-google version of the local NTP.

Perhaps this is not the correct check to be doing in the first place.

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

Fixed, by fixing some of the lower-level functions in google_util to respect a
command-line Google base URL.

        SEARCH_ENGINE_GOOGLE)) {
     return GURL(chrome::kChromeSearchLocalGoogleNtpUrl);
   }
   return GURL(chrome::kChromeSearchLocalNtpUrl);
 }
 
 bool IsInstantEnabled(Profile* profile) {
   return GetInstantURL(profile, kDisableStartMargin).is_valid();
 }
 
@@ skipping 165 matching lines @@
 }
 
 // Given a FieldTrialFlags object, returns the boolean value of the provided
 // flag.
 bool GetBoolValueForFlagWithDefault(const std::string& flag,
                                     bool default_value,
                                     const FieldTrialFlags& flags) {
   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)
     return false;
 
   GURL instant_url = TemplateURLRefToGURL(template_url->instant_url_ref(),
                                           kDisableStartMargin);
   // Extended mode instant requires a search terms replacement key.
   return instant_url.is_valid() &&
          (!IsInstantExtendedAPIEnabled() ||
@@ skipping 13 matching lines @@
   for (size_t i = 0; i < items_b.size(); ++i) {
     if (items_b[i].url != items_a[i].url ||
         items_b[i].title != items_a[i].title) {
       return false;
     }
   }
   return true;
 }
 
 }  // namespace chrome