Update defaults for InstantExtended.

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"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search/instant_service.h"
@@ skipping 37 matching lines @@
 const char kStalePageTimeoutFlagName[] = "stale";
 const int kStalePageTimeoutDefault = 3 * 3600;  // 3 hours.
 const int kStalePageTimeoutDisabled = 0;
 
 const char kHideVerbatimFlagName[] = "hide_verbatim";
 const char kUseRemoteNTPOnStartupFlagName[] = "use_remote_ntp_on_startup";
 const char kShowNtpFlagName[] = "show_ntp";
 const char kRecentTabsOnNTPFlagName[] = "show_recent_tabs";
 const char kUseCacheableNTP[] = "use_cacheable_ntp";
 const char kPrefetchSearchResultsOnSRP[] = "prefetch_results_srp";
-const char kSuppressInstantExtendedOnSRPFlagName[] = "suppress_on_srp";
+const char kEnableQueryExtractionFlagName[] = "query_extraction";
 
 // Constants for the field trial name and group prefix.
 // Note in M30 and below this field trial was named "InstantExtended" and in
 // M31 was renamed to EmbeddedSearch for clarity and cleanliness.  Since we
 // can't easilly sync up Finch configs with the pushing of this change to
 // Dev & Canary, for now the code accepts both names.
 // TODO(dcblack): Remove the InstantExtended name once M31 hits the Beta
 // channel.
 const char kInstantExtendedFieldTrialName[] = "InstantExtended";
 const char kEmbeddedSearchFieldTrialName[] = "EmbeddedSearch";
 const char kGroupNumberPrefix[] = "Group";

[David Black, 2013/09/30 20:47:10]

remove

[samarth, 2013/10/02 00:18:00]

Done.

 
 // If the field trial's group name ends with this string its configuration will
 // be ignored and Instant Extended will not be enabled by default.
 const char kDisablingSuffix[] = "DISABLED";
 
-// Remember if we reported metrics about opt-in/out state.
-bool instant_extended_opt_in_state_gate = false;
-
 // Used to set the Instant support state of the Navigation entry.
 const char kInstantSupportStateKey[] = "instant_support_state";
 
 const char kInstantSupportEnabled[] = "Instant support enabled";
 const char kInstantSupportDisabled[] = "Instant support disabled";
 const char kInstantSupportUnknown[] = "Instant support unknown";
 
 InstantSupportState StringToInstantSupportState(const string16& value) {
   if (value == ASCIIToUTF16(kInstantSupportEnabled))
     return INSTANT_SUPPORT_YES;
@@ skipping 51 matching lines @@
   for (size_t i = 0; i < template_url->URLCount() - 1; ++i) {
     TemplateURLRef ref(template_url, i);
     search_url = TemplateURLRefToGURL(ref, kDisableStartMargin, false);
     if (search_url.is_valid() && MatchesOriginAndPath(url, search_url))
       return true;
   }
 
   return false;
 }
 
-void RecordInstantExtendedOptInState() {
-  if (instant_extended_opt_in_state_gate)
-    return;
-
-  instant_extended_opt_in_state_gate = true;
-  OptInState state = INSTANT_EXTENDED_NOT_SET;
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kDisableInstantExtendedAPI))
-    state = INSTANT_EXTENDED_OPT_OUT;
-  else if (command_line->HasSwitch(switches::kEnableInstantExtendedAPI))
-    state = INSTANT_EXTENDED_OPT_IN;
-
-  UMA_HISTOGRAM_ENUMERATION("InstantExtended.NewOptInState", state,
-                            INSTANT_EXTENDED_OPT_IN_STATE_ENUM_COUNT);
-}
-
 // Returns true if |contents| is rendered inside the Instant process for
 // |profile|.
 bool IsRenderedInInstantProcess(const content::WebContents* contents,
                                 Profile* profile) {
   const content::RenderProcessHost* process_host =
       contents->GetRenderProcessHost();
   if (!process_host)
     return false;
 
   const InstantService* instant_service =
@@ skipping 38 matching lines @@
 
   const TemplateURLRef& instant_url_ref = template_url->instant_url_ref();
   const GURL instant_url =
       TemplateURLRefToGURL(instant_url_ref, kDisableStartMargin, false);
   if (!instant_url.is_valid())
     return false;
 
   if (MatchesOriginAndPath(url, instant_url))
     return true;
 
-  return !ShouldSuppressInstantExtendedOnSRP() &&
-      MatchesAnySearchURL(url, template_url);
+  return IsQueryExtractionEnabled() && MatchesAnySearchURL(url, template_url);
 }
 
 string16 GetSearchTermsImpl(const content::WebContents* contents,
                             const content::NavigationEntry* entry) {
   if (!contents || !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
@@ skipping 20 matching lines @@
 
 }  // namespace
 
 // Negative start-margin values prevent the "es_sm" parameter from being used.
 const int kDisableStartMargin = -1;
 
 bool IsInstantExtendedAPIEnabled() {
 #if defined(OS_IOS) || defined(OS_ANDROID)
   return false;
 #else
-  RecordInstantExtendedOptInState();
   return EmbeddedSearchPageVersion() != kEmbeddedPageVersionDisabled;

[David Black, 2013/09/30 20:47:10]

Do we actually want to retain the ability to turn off 1993 via an espv=0 field
trial param?  Seems dangerous.

[samarth, 2013/10/02 00:18:00]

Good point.  I'd kept it in to give us the flexibility.  But on further thought,
in the M32 timescale, the non-instant-extended code will have bitrotted enough
that we should probably not trust it.  Removed this.

 #endif  // defined(OS_IOS) || defined(OS_ANDROID)
 }
 
 // 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.
 uint64 EmbeddedSearchPageVersion() {
-  RecordInstantExtendedOptInState();
-
-  // Check the command-line/about:flags setting first, which should have
-  // precedence and allows the trial to not be reported (if it's never queried).
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kDisableInstantExtendedAPI))
-    return kEmbeddedPageVersionDisabled;
-  if (command_line->HasSwitch(switches::kEnableInstantExtendedAPI)) {
-    // The user has set the about:flags switch to Enabled - give the default
-    // UI version.
-    return kEmbeddedPageVersionDefault;
-  }
-
   FieldTrialFlags flags;
-  uint64 group_num = 0;
-  if (GetFieldTrialInfo(&flags, &group_num)) {
-    if (group_num == 0)
-      return kEmbeddedPageVersionDisabled;
+  if (GetFieldTrialInfo(&flags)) {
     return GetUInt64ValueForFlagWithDefault(kEmbeddedPageVersionFlagName,
                                             kEmbeddedPageVersionDefault,
                                             flags);
   }
-  return kEmbeddedPageVersionDisabled;
+  return kEmbeddedPageVersionDefault;
 }
 
 bool IsQueryExtractionEnabled() {
-  return EmbeddedSearchPageVersion() != kEmbeddedPageVersionDisabled &&
-      !ShouldSuppressInstantExtendedOnSRP();
+#if defined(OS_IOS) || defined(OS_ANDROID)
+  return true;
+#else
+  if (!IsInstantExtendedAPIEnabled())
+    return false;
+
+  const CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kEnableQueryExtraction))
+    return true;
+
+  FieldTrialFlags flags;
+  if (GetFieldTrialInfo(&flags)) {
+    return GetBoolValueForFlagWithDefault(
+        kEnableQueryExtractionFlagName, false, flags);
+  }
+
+  return false;
+#endif  // defined(OS_IOS) || defined(OS_ANDROID)
 }
 
 string16 GetSearchTermsFromURL(Profile* profile, const GURL& url) {
   string16 search_terms;
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
   if (template_url && IsSuitableURLForInstant(url, template_url))
     template_url->ExtractSearchTermsFromURL(url, &search_terms);
   return search_terms;
 }
 
@@ skipping 81 matching lines @@
   return IsInstantURL(entry->GetVirtualURL(), profile) &&
       GetSearchTermsImpl(contents, entry).empty();
 }
 
 bool IsSuggestPrefEnabled(Profile* profile) {
   return profile && !profile->IsOffTheRecord() && profile->GetPrefs() &&
          profile->GetPrefs()->GetBoolean(prefs::kSearchSuggestEnabled);
 }
 
 GURL GetInstantURL(Profile* profile, int start_margin) {
   if (!IsInstantExtendedAPIEnabled() || !IsSuggestPrefEnabled(profile))

[David Black, 2013/09/30 20:47:10]

This IsSuggestPrefEnabled check here (and another one below) seems obsolete and
maybe very wrong?  Thoughts?

[samarth, 2013/10/02 00:18:00]

Yeah it's arguable whether this makes sense anymore.  The original reasoning was
that with if you don't have suggest enabled, we shouldn't do instant.  With
these checks now, we won't show you the remote NTP if you don't have suggest
enabled.

It's kinda weird, but I don't want to change this here.  How about you file a
bug about this?  I want to make sure there's no privacy issues I'm forgetting
about.

     return GURL();
 
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
   if (!template_url)
     return GURL();
 
   GURL instant_url =
       TemplateURLRefToGURL(template_url->instant_url_ref(), start_margin, true);
   if (!instant_url.is_valid() ||
       !template_url->HasSearchTermsReplacementKey(instant_url))
     return GURL();
 
   // 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()).
   if (instant_url.SchemeIsSecure() ||
       google_util::StartsWithCommandLineGoogleBaseURL(instant_url))
     return instant_url;
   GURL::Replacements replacements;
   const std::string secure_scheme(content::kHttpsScheme);
   replacements.SetSchemeStr(secure_scheme);
   return instant_url.ReplaceComponents(replacements);
 }
 
 GURL GetLocalInstantURL(Profile* profile) {
   return GURL(chrome::kChromeSearchLocalNtpUrl);
 }
 
 bool ShouldPreferRemoteNTPOnStartup() {
-  // Check the command-line/about:flags setting first, which should have
-  // precedence and allows the trial to not be reported (if it's never queried).
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kDisableInstantExtendedAPI) ||
-      command_line->HasSwitch(switches::kEnableLocalFirstLoadNTP)) {
-    return false;
-  }
-  if (command_line->HasSwitch(switches::kDisableLocalFirstLoadNTP))
-    return true;
-
   FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
+  if (GetFieldTrialInfo(&flags)) {
     return GetBoolValueForFlagWithDefault(kUseRemoteNTPOnStartupFlagName, true,
                                           flags);
   }
   return false;

[David Black, 2013/09/30 20:47:10]

As per your comment on https://chromiumcodereview.appspot.com/18543006/ please
change this to true.

[samarth, 2013/10/02 00:18:00]

Done.

 }
 
 bool ShouldHideTopVerbatimMatch() {
   FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
+  if (GetFieldTrialInfo(&flags)) {
     return GetBoolValueForFlagWithDefault(kHideVerbatimFlagName, false, flags);
   }
   return false;
 }
 
 bool ShouldUseCacheableNTP() {
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kUseCacheableNewTabPage))
     return true;
 
   FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
+  if (GetFieldTrialInfo(&flags)) {
     return GetBoolValueForFlagWithDefault(kUseCacheableNTP, false, flags);
   }
   return false;
 }
 
 bool ShouldShowInstantNTP() {
   // If using the cacheable NTP, load the NTP directly instead of preloading its
   // contents using InstantNTP.
   if (ShouldUseCacheableNTP())
     return false;
 
   FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
+  if (GetFieldTrialInfo(&flags)) {
     return GetBoolValueForFlagWithDefault(kShowNtpFlagName, true, flags);
   }
   return true;
 }
 
 bool ShouldShowRecentTabsOnNTP() {
   FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
+  if (GetFieldTrialInfo(&flags)) {
     return GetBoolValueForFlagWithDefault(
         kRecentTabsOnNTPFlagName, false, flags);
   }
 
   return false;
 }
 
-bool ShouldSuppressInstantExtendedOnSRP() {
-  FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
-    return GetBoolValueForFlagWithDefault(
-        kSuppressInstantExtendedOnSRPFlagName, false, flags);
-  }
-
-  return false;
-}
-
 bool MatchesOriginAndPath(const GURL& my_url, const GURL& other_url) {
   return MatchesOrigin(my_url, other_url) && my_url.path() == other_url.path();
 }
 
 GURL GetEffectiveURLForInstant(const GURL& url, Profile* profile) {
   CHECK(ShouldAssignURLToInstantRenderer(url, profile))
       << "Error granting Instant access.";
 
   if (url.SchemeIs(chrome::kChromeSearchScheme))
     return url;
@@ skipping 19 matching lines @@
     }
   }
 
   effective_url = effective_url.ReplaceComponents(replacements);
   return effective_url;
 }
 
 int GetInstantLoaderStalenessTimeoutSec() {
   int timeout_sec = kStalePageTimeoutDefault;
   FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
+  if (GetFieldTrialInfo(&flags)) {
     timeout_sec = GetUInt64ValueForFlagWithDefault(kStalePageTimeoutFlagName,
                                                    kStalePageTimeoutDefault,
                                                    flags);
   }
 
   // Require a minimum 5 minute timeout.
   if (timeout_sec < 0 || (timeout_sec > 0 && timeout_sec < 300))
     timeout_sec = kStalePageTimeoutDefault;
 
   // Randomize by upto 15% either side.
@@ skipping 64 matching lines @@
 InstantSupportState GetInstantSupportStateFromNavigationEntry(
     const content::NavigationEntry& entry) {
   string16 value;
   if (!entry.GetExtraData(kInstantSupportStateKey, &value))
     return INSTANT_SUPPORT_UNKNOWN;
 
   return StringToInstantSupportState(value);
 }
 
 bool ShouldPrefetchSearchResultsOnSRP() {
-  // Check the command-line/about:flags setting first, which should have
-  // precedence and allows the trial to not be reported (if it's never queried).
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kDisableInstantExtendedAPI) ||
-      command_line->HasSwitch(switches::kEnableInstantExtendedAPI)) {
-    return false;
-  }
-
   FieldTrialFlags flags;
-  if (GetFieldTrialInfo(&flags, NULL)) {
+  if (GetFieldTrialInfo(&flags)) {
     return GetBoolValueForFlagWithDefault(kPrefetchSearchResultsOnSRP, false,
                                           flags);
   }
   return false;
 }
 
-void EnableInstantExtendedAPIForTesting() {
+void EnableQueryExtractionForTesting() {
   CommandLine* cl = CommandLine::ForCurrentProcess();
-  cl->AppendSwitch(switches::kEnableInstantExtendedAPI);
+  cl->AppendSwitch(switches::kEnableQueryExtraction);
 }
 
-void DisableInstantExtendedAPIForTesting() {
-  CommandLine* cl = CommandLine::ForCurrentProcess();
-  cl->AppendSwitch(switches::kDisableInstantExtendedAPI);
-}
-
-bool GetFieldTrialInfo(FieldTrialFlags* flags,
-                       uint64* group_number) {
+bool GetFieldTrialInfo(FieldTrialFlags* flags) {
   // Get the group name.  If the EmbeddedSearch trial doesn't exist, look for
   // the older InstantExtended name.
   std::string group_name = base::FieldTrialList::FindFullName(
       kEmbeddedSearchFieldTrialName);
   if (group_name.empty()) {
     group_name = base::FieldTrialList::FindFullName(
         kInstantExtendedFieldTrialName);
   }
 
   if (EndsWith(group_name, kDisablingSuffix, true))
     return false;
 
-  // We have a valid trial that isn't disabled.
-  // First extract the flags.
+  // We have a valid trial that isn't disabled. Extract the flags.
   std::string group_prefix(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.
     group_prefix = group_name.substr(0, first_space);
     if (!base::SplitStringIntoKeyValuePairs(group_name.substr(first_space),
                                             ':', ' ', flags)) {
       // Failed to parse the flags section. Assume the whole group name is
       // invalid.
       return false;
     }
   }
 
-  // Now extract the group number, making sure we get a non-zero value.
-  uint64 temp_group_number = 0;
-  if (StartsWithASCII(group_name, kGroupNumberPrefix, true)) {
-    std::string group_suffix = group_prefix.substr(strlen(kGroupNumberPrefix));
-    if (!base::StringToUint64(group_suffix, &temp_group_number))
-      return false;
-    if (group_number)
-      *group_number = temp_group_number;
-  } else {
-    // Instant Extended is not enabled.
-    if (group_number)
-      *group_number = 0;
-  }
-
   return true;
 }
 
 // Given a FieldTrialFlags object, returns the string value of the provided
 // flag.
 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++) {
@@ skipping 32 matching lines @@
     return GURL();
 
   TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
   if (!template_url)
     return GURL();
 
   return TemplateURLRefToGURL(template_url->new_tab_url_ref(),
                               kDisableStartMargin, false);
 }
 
-void ResetInstantExtendedOptInStateGateForTest() {
-  instant_extended_opt_in_state_gate = false;
-}
-
 }  // namespace chrome