Misc. cleanup:

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 132 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| 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 |template_url|, which is
+//     expected to be the TemplateURL* for the default search provider.
+// (2) It has a secure scheme.
+bool IsSuitableURLForInstant(const GURL& url, const TemplateURL* template_url) {
+  return template_url->HasSearchTermsReplacementKey(url) &&
+      url.SchemeIsSecure();
+}
+
 // 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();
-
-  if (!url.is_valid())
+  if (extended_api_enabled && !IsSuitableURLForInstant(url, template_url))
     return false;
 
-  if (extended_api_enabled && !url.SchemeIsSecure())
-    return false;
-
-  if (extended_api_enabled && !template_url->HasSearchTermsReplacementKey(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(url, instant_url))
-    return true;
-
-  if (extended_api_enabled && MatchesAnySearchURL(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) {
   if (!IsQueryExtractionEnabled())
     return string16();
 
   // 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.
   // Since iOS and Android doesn't use the instant framework, these checks are
   // disabled for the two platforms.
   Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
 #if !defined(OS_IOS) && !defined(OS_ANDROID)
   if (!IsRenderedInInstantProcess(contents, profile) &&
-      (contents->GetController().GetLastCommittedEntry() == entry ||
+      ((entry == contents->GetController().GetLastCommittedEntry()) ||
        !ShouldAssignURLToInstantRenderer(entry->GetURL(), profile)))
     return string16();
 #endif  // !defined(OS_IOS) && !defined(OS_ANDROID)
   // Check to see if search terms have already been extracted.
   string16 search_terms = GetSearchTermsFromNavigationEntry(entry);
   if (!search_terms.empty())
     return search_terms;
 
   // Otherwise, extract from the URL.
   return GetSearchTermsFromURL(profile, entry->GetVirtualURL());
@@ skipping 41 matching lines @@
   }
   return kEmbeddedPageVersionDisabled;
 }
 
 bool IsQueryExtractionEnabled() {
   return EmbeddedSearchPageVersion() != kEmbeddedPageVersionDisabled;
 }
 
 string16 GetSearchTermsFromURL(Profile* profile, const GURL& url) {
   string16 search_terms;
-
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
-  if (!template_url)
-    return string16();
-
-  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 132 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.
   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);
   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;
+  // Extended mode requires HTTPS.  Force it.

[Jered, 2013/06/28 21:57:28]

Move this comment down below the if, since that's the code that does the
forcing.

[Peter Kasting, 2013/06/28 22:16:02]

I agree, but I'm going to leave it where it is only because my next patch
modifies this comment to refer more deeply to the conditional.

+  if (!extended_api_enabled || instant_url.SchemeIsSecure())
+    return instant_url;
+  GURL::Replacements replacements;
+  const std::string secure_scheme(chrome::kHttpsScheme);
+  replacements.SetSchemeStr(secure_scheme);
+  return instant_url.ReplaceComponents(replacements);
 }
 
 GURL GetLocalInstantURL(Profile* profile) {
   const TemplateURL* default_provider =
       GetDefaultSearchProviderTemplateURL(profile);
 
   if (default_provider &&
       (TemplateURLPrepopulateData::GetEngineType(default_provider->url()) ==
        SEARCH_ENGINE_GOOGLE)) {
     return GURL(chrome::kChromeSearchLocalGoogleNtpUrl);
@@ skipping 207 matching lines @@
   return instant_url.is_valid() &&
          (!IsInstantExtendedAPIEnabled() ||
           template_url->HasSearchTermsReplacementKey(instant_url));
 }
 
 void ResetInstantExtendedOptInStateGateForTest() {
   instant_extended_opt_in_state_gate = false;
 }
 
 }  // namespace chrome