Consolidate search terms extraction and Instant process determination.

chrome/browser/ui/search/search.cc

Index: chrome/browser/ui/search/search.cc
diff --git a/chrome/browser/ui/search/search.cc b/chrome/browser/ui/search/search.cc
index a39700e7ae2756498cc7bd4c91fbd83312943fc8..ea925d04e7d63ae0ba2dd1c6841b7c779731b04a 100644
--- a/chrome/browser/ui/search/search.cc
+++ b/chrome/browser/ui/search/search.cc
@@ -7,17 +7,17 @@
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/string_split.h"
-#include "base/string_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "chrome/browser/instant/instant_service.h"
+#include "chrome/browser/instant/instant_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search_engines/template_url_service.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/chrome_version_info.h"
+#include "chrome/common/url_constants.h"
 #include "content/public/browser/navigation_entry.h"
-
-#if !defined(OS_ANDROID)
-#include "chrome/browser/themes/theme_service.h"
-#include "chrome/browser/themes/theme_service_factory.h"
-#endif
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
 
 namespace {
 
@@ -25,12 +25,9 @@ namespace {
 // InstantExtended field trials are named in such a way that we can parse out
 // the experiment configuration from the trial's group name in order to give
 // us maximum flexability in running experiments.
-// Field trials should be named things like "Group7 espv:2 themes:0".
+// Field trial groups should be named things like "Group7 espv:2 instant:1".
 // The first token is always GroupN for some integer N, followed by a
 // space-delimited list of key:value pairs which correspond to these flags:
-const char kEnableOnThemesFlagName[] = "themes";
-const bool kEnableOnThemesDefault = false;
-
 const char kEmbeddedPageVersionFlagName[] = "espv";
 const int kEmbeddedPageVersionDefault = 1;
 
@@ -46,153 +43,272 @@ const char kGroupNumberPrefix[] = "Group";
 // be ignored and Instant Extended will not be enabled by default.
 const char kDisablingSuffix[] = "DISABLED";
 
+chrome::search::InstantExtendedDefault InstantExtendedDefaultFromInt64(
+    int64 default_value) {
+  switch (default_value) {
+    case 0: return chrome::search::INSTANT_DEFAULT_ON;
+    case 1: return chrome::search::INSTANT_USE_EXISTING;
+    case 2: return chrome::search::INSTANT_DEFAULT_OFF;
+    default: return chrome::search::INSTANT_USE_EXISTING;
+  }
+}
+
+TemplateURL* GetDefaultSearchProviderTemplateURL(Profile* profile) {
+  TemplateURLService* template_url_service =
+      TemplateURLServiceFactory::GetForProfile(profile);
+  if (template_url_service)
+    return template_url_service->GetDefaultSearchProvider();
+  return NULL;
+}
+
+GURL TemplateURLRefToGURL(const TemplateURLRef& ref) {
+  return GURL(
+      ref.ReplaceSearchTerms(TemplateURLRef::SearchTermsArgs(string16())));
+}
+
+bool MatchesOriginAndPath(const GURL& my_url, const GURL& other_url) {
+  return my_url.host() == other_url.host() &&
+         my_url.port() == other_url.port() &&
+         my_url.path() == other_url.path() &&
+         (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();
+  GURL instant_url(cl->GetSwitchValueASCII(switches::kInstantURL));
+  return instant_url.is_valid() && MatchesOriginAndPath(url, instant_url);
+}
+
+// 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) {
+  GURL search_url = TemplateURLRefToGURL(ref);
+
+  GURL::Replacements replacements;
+  replacements.SetSchemeStr(chrome::kHttpsScheme);
+  replacements.SetHostStr(search_url.host());
+  replacements.SetPortStr(search_url.port());
+  replacements.SetPathStr(search_url.path());
+
+  return instant_url.ReplaceComponents(replacements);
+}
+
+bool MatchesAnySearchURL(const GURL& url, TemplateURL* template_url) {
+  GURL search_url = TemplateURLRefToGURL(template_url->url_ref());
+  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);
+    if (search_url.is_valid() && MatchesOriginAndPath(url, search_url))
+      return true;
+  }
+
+  return false;
+}
+
 }  // namespace
 
 namespace chrome {
 namespace search {
 
-// static
 const char kInstantExtendedSearchTermsKey[] = "search_terms";
 
-InstantExtendedDefault InstantExtendedDefaultFromInt64(int64 default_value) {
-  switch (default_value) {
-    case 0: return INSTANT_FORCE_ON;
-    case 1: return INSTANT_USE_EXISTING;
-    case 2: return INSTANT_FORCE_OFF;
-    default: return INSTANT_USE_EXISTING;
-  }
-}
+const char kLocalOmniboxPopupURL[] =
+    "chrome://local-omnibox-popup/local-omnibox-popup.html";
 
 InstantExtendedDefault GetInstantExtendedDefaultSetting() {
-  InstantExtendedDefault default_setting = INSTANT_USE_EXISTING;
-
   FieldTrialFlags flags;
   if (GetFieldTrialInfo(
           base::FieldTrialList::FindFullName(kInstantExtendedFieldTrialName),
           &flags, NULL)) {
     uint64 trial_default = GetUInt64ValueForFlagWithDefault(
-        kInstantExtendedActivationName,
-        kInstantExtendedActivationDefault,
-        flags);
-    default_setting = InstantExtendedDefaultFromInt64(trial_default);
+                               kInstantExtendedActivationName,
+                               kInstantExtendedActivationDefault,
+                               flags);
+    return InstantExtendedDefaultFromInt64(trial_default);
   }
 
-  return default_setting;
+  return INSTANT_USE_EXISTING;
 }
 
-// Check whether or not the Extended API should be used on the given profile.
-bool IsInstantExtendedAPIEnabled(Profile* profile) {
+bool IsInstantExtendedAPIEnabled(const Profile* profile) {
   return EmbeddedSearchPageVersion(profile) != 0;
 }
 
 // Determine what embedded search page version to request from the user's
-// default search provider.  If 0, the embedded search UI should not be enabled.
-// Note that the profile object here isn't const because we need to determine
-// whether or not the user has a theme installed as part of this check, and
-// that logic requires a non-const profile for whatever reason.
-uint64 EmbeddedSearchPageVersion(Profile* profile) {
-  // Incognito windows do not currently use the embedded search API.
+// default search provider. If 0, the embedded search UI should not be enabled.
+uint64 EmbeddedSearchPageVersion(const Profile* profile) {
   if (!profile || profile->IsOffTheRecord())
     return 0;
 
-  // Check Finch field trials.
   FieldTrialFlags flags;
   if (GetFieldTrialInfo(
           base::FieldTrialList::FindFullName(kInstantExtendedFieldTrialName),
           &flags, NULL)) {
-    uint64 espv = GetUInt64ValueForFlagWithDefault(
-        kEmbeddedPageVersionFlagName,
-        kEmbeddedPageVersionDefault,
-        flags);
-
-    // Check for themes.
-    bool has_theme = false;
-#if !defined(OS_ANDROID)
-    has_theme =
-        !ThemeServiceFactory::GetForProfile(profile)->UsingDefaultTheme();
-#endif
-
-    bool enable_for_themes =
-        GetBoolValueForFlagWithDefault(kEnableOnThemesFlagName,
-                                       kEnableOnThemesDefault,
-                                       flags);
-    if (!has_theme || enable_for_themes)
-      return espv;
+    return GetUInt64ValueForFlagWithDefault(kEmbeddedPageVersionFlagName,
+                                            kEmbeddedPageVersionDefault,
+                                            flags);
   }
 
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableInstantExtendedAPI)) {
+  const CommandLine* cl = CommandLine::ForCurrentProcess();
+  if (cl->HasSwitch(switches::kEnableInstantExtendedAPI)) {
     // The user has manually flipped the about:flags switch - give the default
     // UI version.
     return kEmbeddedPageVersionDefault;
   }
-  return 0;
-}
 
-void EnableInstantExtendedAPIForTesting() {
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableInstantExtendedAPI);
+  return 0;
 }
 
-bool IsQueryExtractionEnabled(Profile* profile) {
+bool IsQueryExtractionEnabled(const Profile* profile) {
 #if defined(OS_IOS)
-  return CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableQueryExtraction);
+  const CommandLine* cl = CommandLine::ForCurrentProcess();
+  return cl->HasSwitch(switches::kEnableQueryExtraction);
 #else
-  if (!profile || profile->IsOffTheRecord())
-    return false;
-
   // On desktop, query extraction is controlled by the instant-extended-api
   // flag.
-  bool enabled = IsInstantExtendedAPIEnabled(profile);
-
-  // Running with --enable-query-extraction but not
-  // --enable-instant-extended-api is an error.
-  DCHECK(!(CommandLine::ForCurrentProcess()->HasSwitch(
-               switches::kEnableQueryExtraction) &&
-           !enabled));
-  return enabled;
+  return IsInstantExtendedAPIEnabled(profile);
 #endif
 }
 
+string16 GetSearchTermsFromNavigationEntry(
+    const content::NavigationEntry* entry) {
+  string16 search_terms;
+  if (entry)
+    entry->GetExtraData(kInstantExtendedSearchTermsKey, &search_terms);
+  return search_terms;
+}
+
+string16 GetSearchTerms(const content::WebContents* contents) {
+  string16 search_terms;
+
+  if (!contents)
+    return search_terms;

[dhollowa, 2013/02/13 17:42:28]

For these early returns, and anywhere you expect search_terms to be empty, it
would be clearer to use "return string16();".

[sreeram, 2013/02/13 17:45:05]

The reason why I kept it this way is to allow NRVO (i.e., all return paths
return the same local object). If you think it's not worthwhile trying to get
that optimization, I can switch over to string16(). I agree string16() is
clearer. The style guide does say not to worry too much about passing strings by
value vs reference, but when there's an opportunity, why not grab it, right?

+
+  Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
+  if (!IsQueryExtractionEnabled(profile))
+    return search_terms;
+
+  // 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.
+  const content::RenderProcessHost* process_host =
+      contents->GetRenderProcessHost();
+  if (!process_host)
+    return search_terms;
+
+  const InstantService* instant_service =
+      InstantServiceFactory::GetForProfile(profile);
+  if (!instant_service)
+    return search_terms;
+
+  if (!instant_service->IsInstantProcess(process_host->GetID()))
+    return search_terms;
+
+  // Check to see if search terms have already been extracted.
+  const content::NavigationEntry* entry =
+      contents->GetController().GetVisibleEntry();
+  if (!entry)
+    return search_terms;
+
+  search_terms = GetSearchTermsFromNavigationEntry(entry);
+  if (!search_terms.empty())
+    return search_terms;
+
+  // Otherwise, extract from the URL.
+  TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
+  if (!template_url)
+    return search_terms;
+
+  GURL url = entry->GetVirtualURL();
+
+  if (IsCommandLineInstantURL(url))
+    url = CoerceCommandLineURLToTemplateURL(url, template_url->url_ref());
+
+  if (url.SchemeIsSecure() && template_url->HasSearchTermsReplacementKey(url))
+    template_url->ExtractSearchTermsFromURL(url, &search_terms);
+
+  return search_terms;
+}
+
+bool ShouldAssignURLToInstantRenderer(const GURL& url, Profile* profile) {
+  TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
+  if (!template_url)
+    return false;
+
+  GURL effective_url = url;
+
+  if (IsCommandLineInstantURL(url)) {
+    const TemplateURLRef& instant_url_ref = template_url->instant_url_ref();
+    effective_url = CoerceCommandLineURLToTemplateURL(url, instant_url_ref);
+  }
+
+  return ShouldAssignURLToInstantRendererImpl(
+             effective_url,
+             IsInstantExtendedAPIEnabled(profile),
+             template_url);
+}
+
+void EnableInstantExtendedAPIForTesting() {
+  CommandLine* cl = CommandLine::ForCurrentProcess();
+  cl->AppendSwitch(switches::kEnableInstantExtendedAPI);
+}
+
 void EnableQueryExtractionForTesting() {
 #if defined(OS_IOS)
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableQueryExtraction);
+  CommandLine* cl = CommandLine::ForCurrentProcess();
+  cl->AppendSwitch(switches::kEnableQueryExtraction);
 #else
-  // On desktop, query extraction is controlled by the instant-extended-api
-  // flag.
-  CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableInstantExtendedAPI);
+  EnableInstantExtendedAPIForTesting();
 #endif
 }
 
-string16 GetSearchTermsFromNavigationEntry(
-    const content::NavigationEntry* entry) {
-  string16 search_terms;
-  entry->GetExtraData(kInstantExtendedSearchTermsKey, &search_terms);
-  return search_terms;
-}
+bool ShouldAssignURLToInstantRendererImpl(const GURL& url,
+                                          bool extended_api_enabled,
+                                          TemplateURL* template_url) {
+  if (!url.is_valid())
+    return false;
+
+  if (url.SchemeIs(chrome::kChromeSearchScheme))
+    return true;
 
-bool IsForcedInstantURL(const GURL& url) {
-  CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (!command_line->HasSwitch(switches::kInstantURL))
+  if (extended_api_enabled && url == GURL(kLocalOmniboxPopupURL))
+    return true;
+
+  if (extended_api_enabled && !url.SchemeIsSecure())
+    return false;
+
+  if (extended_api_enabled && !template_url->HasSearchTermsReplacementKey(url))
+    return false;
+
+  GURL instant_url = TemplateURLRefToGURL(template_url->instant_url_ref());
+  if (!instant_url.is_valid())
     return false;
 
-  GURL instant_url(command_line->GetSwitchValueASCII(switches::kInstantURL));
-  return url.scheme() == instant_url.scheme() &&
-         url.host() == instant_url.host() &&
-         url.port() == instant_url.port() &&
-         url.path() == instant_url.path();
+  if (MatchesOriginAndPath(url, instant_url))
+    return true;
+
+  if (extended_api_enabled && MatchesAnySearchURL(url, template_url))
+    return true;
+
+  return false;
 }
 
 bool GetFieldTrialInfo(const std::string& group_name,
                        FieldTrialFlags* flags,
                        uint64* group_number) {
   if (EndsWith(group_name, kDisablingSuffix, true) ||
-      !StartsWithASCII(group_name, kGroupNumberPrefix, true)) {
+      !StartsWithASCII(group_name, kGroupNumberPrefix, true))
     return false;
-  }
 
   // We have a valid trial that starts with "Group" and isn't disabled.
   // First extract the flags.
@@ -200,8 +316,7 @@ bool GetFieldTrialInfo(const std::string& group_name,
 
   size_t first_space = group_name.find(" ");
   if (first_space != std::string::npos) {
-    // There is a flags section of the group name.  Split that out and parse
-    // it.
+    // There is a flags section of the group name. Split that out and parse it.
     group_prefix = group_name.substr(0, first_space);
     if (!base::SplitStringIntoKeyValuePairs(group_name.substr(first_space),
                                             ':', ' ', flags)) {
@@ -213,11 +328,10 @@ bool GetFieldTrialInfo(const std::string& group_name,
 
   // Now extract the group number, making sure we get a non-zero value.
   uint64 temp_group_number = 0;
-  if (!base::StringToUint64(group_prefix.substr(strlen(kGroupNumberPrefix)),
-                            &temp_group_number) ||
-      temp_group_number == 0) {
+  std::string group_suffix = group_prefix.substr(strlen(kGroupNumberPrefix));
+  if (!base::StringToUint64(group_suffix, &temp_group_number) ||
+      temp_group_number == 0)
     return false;
-  }
 
   if (group_number)
     *group_number = temp_group_number;
@@ -227,10 +341,9 @@ bool GetFieldTrialInfo(const std::string& group_name,
 
 // Given a FieldTrialFlags object, returns the string value of the provided
 // flag.
-std::string GetStringValueForFlagWithDefault(
-    const std::string& flag,
-    const std::string& default_value,
-    FieldTrialFlags& flags) {
+std::string GetStringValueForFlagWithDefault(const std::string& flag,
+                                             const std::string& default_value,
+                                             const FieldTrialFlags& flags) {
   FieldTrialFlags::const_iterator i;
   for (i = flags.begin(); i != flags.end(); i++) {
     if (i->first == flag)
@@ -241,19 +354,21 @@ std::string GetStringValueForFlagWithDefault(
 
 // Given a FieldTrialFlags object, returns the uint64 value of the provided
 // flag.
-uint64 GetUInt64ValueForFlagWithDefault(
-    const std::string& flag, uint64 default_value, FieldTrialFlags& flags) {
+uint64 GetUInt64ValueForFlagWithDefault(const std::string& flag,
+                                        uint64 default_value,
+                                        const FieldTrialFlags& flags) {
   uint64 value;
-  if (!base::StringToUint64(GetStringValueForFlagWithDefault(flag, "", flags),
-                            &value))
-    return default_value;
-  return value;
+  std::string str_value = GetStringValueForFlagWithDefault(flag, "", flags);
+  if (base::StringToUint64(str_value, &value))
+    return value;
+  return default_value;
 }
 
 // Given a FieldTrialFlags object, returns the boolean value of the provided
 // flag.
-bool GetBoolValueForFlagWithDefault(
-    const std::string& flag, bool default_value, FieldTrialFlags& flags) {
+bool GetBoolValueForFlagWithDefault(const std::string& flag,
+                                    bool default_value,
+                                    const FieldTrialFlags& flags) {
   return !!GetUInt64ValueForFlagWithDefault(flag, default_value ? 1 : 0, flags);
 }