Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2354)

Unified Diff: chrome/browser/ui/search/search.cc

Issue 11743035: Disable the embedded search field trials for users with themes. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 7 years, 12 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/search/search.cc
diff --git a/chrome/browser/ui/search/search.cc b/chrome/browser/ui/search/search.cc
index 1205dfb9c67950c58dd4f1d2f0ac7c8a6dd8ea86..35f14c3185b6cde1edb39edaeb9959c2ab40e14d 100644
--- a/chrome/browser/ui/search/search.cc
+++ b/chrome/browser/ui/search/search.cc
@@ -7,37 +7,142 @@
#include "base/command_line.h"
#include "base/metrics/field_trial.h"
#include "base/string_number_conversions.h"
+#include "base/string_split.h"
#include "base/string_util.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/themes/theme_service.h"
+#include "chrome/browser/themes/theme_service_factory.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/chrome_version_info.h"
namespace chrome {
namespace search {
-bool IsInstantExtendedAPIEnabled(const Profile* profile) {
+// Configuration options for Embedded Search.
+// 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" -
dhollowa 2013/01/04 17:11:49 nit: period "." at end of sentence, not "-".
David Black 2013/01/04 19:55:32 Done.
+// 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;
+
+// Constants for the field trial name and group prefix.
+const char kInstantExtendedFieldTrialName[] = "InstantExtended";
+const char kGroupNumberPrefix[] = "Group";
+
+// 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";
+
+// Given a field trial group name in the above format, parses out the group
+// number and configuration flags. Will return a group number of 0 on error.
+void GetFieldTrialInfo(const std::string& group_name,
+ FieldTrialFlags* flags,
+ uint64* group_number) {
+ if (!EndsWith(group_name, kDisablingSuffix, true) &&
+ StartsWithASCII(group_name, kGroupNumberPrefix, true)) {
+ // We have a valid trial that starts with "Group" and isn't disabled.
+ size_t first_space = group_name.find(" ");
+ std::string group_prefix = group_name;
+ 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);
+ base::SplitStringIntoKeyValuePairs(
+ group_name.substr(first_space), ':', ' ', flags);
+ }
+ if (!base::StringToUint64(group_prefix.substr(strlen(kGroupNumberPrefix)),
+ group_number)) {
+ // Could not parse group number.
+ *group_number = 0;
+ }
+ }
+}
+
+// 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) {
+ FieldTrialFlags::const_iterator i;
+ for (i = flags.begin(); i != flags.end(); i++) {
+ if (i->first == flag)
+ return i->second;
+ }
+ return default_value;
+}
+
+// Given a FieldTrialFlags object, returns the uint64 value of the provided
+// flag.
+uint64 GetUInt64ValueForFlagWithDefault(
+ const std::string& flag, uint64 default_value, FieldTrialFlags& flags) {
+ uint64 value;
+ if (!base::StringToUint64(GetStringValueForFlagWithDefault(flag, "", flags),
+ &value))
+ return default_value;
+ return value;
+}
+
+// Given a FieldTrialFlags object, returns the boolean value of the provided
+// flag.
+bool GetBoolValueForFlagWithDefault(
+ const std::string& flag, bool default_value, FieldTrialFlags& flags) {
+ return GetUInt64ValueForFlagWithDefault(flag, default_value ? 1 : 0, flags);
+}
+
+// Check whether or not the Extended API should be used on the given profile.
+bool IsInstantExtendedAPIEnabled(Profile* profile) {
return EmbeddedSearchPageVersion(profile) != 0;
}
-uint64 EmbeddedSearchPageVersion(const Profile* profile) {
+// 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.
if (!profile || profile->IsOffTheRecord())
return 0;
// Check Finch field trials.
- base::FieldTrial* trial = base::FieldTrialList::Find("InstantExtended");
- if (trial && StartsWithASCII(trial->group_name(), "Group", true)) {
- uint64 group_number;
- if (base::StringToUint64(trial->group_name().substr(5), &group_number)) {
- return group_number;
- }
+ FieldTrialFlags flags;
+ uint64 group_number = 0;
+ base::FieldTrial* trial =
+ base::FieldTrialList::Find(kInstantExtendedFieldTrialName);
+ if (trial) {
+ std::string group_name = trial->group_name();
+ GetFieldTrialInfo(group_name, &flags, &group_number);
+ }
+
+ if (group_number > 0) {
+ uint64 espv = GetUInt64ValueForFlagWithDefault(
+ kEmbeddedPageVersionFlagName,
+ kEmbeddedPageVersionDefault,
+ flags);
+
+ // Check for themes.
+ bool has_theme =
+ !ThemeServiceFactory::GetForProfile(profile)->UsingDefaultTheme();
+ bool enable_for_themes =
+ GetBoolValueForFlagWithDefault(kEnableOnThemesFlagName,
+ kEnableOnThemesDefault,
+ flags);
+ if (!has_theme || enable_for_themes)
+ return espv;
}
if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableInstantExtendedAPI)) {
// The user has manually flipped the about:flags switch - give the default
// UI version.
- return 1;
+ return kEmbeddedPageVersionDefault;
}
return 0;
}
@@ -47,7 +152,7 @@ void EnableInstantExtendedAPIForTesting() {
switches::kEnableInstantExtendedAPI);
}
-bool IsQueryExtractionEnabled(const Profile* profile) {
+bool IsQueryExtractionEnabled(Profile* profile) {
return IsInstantExtendedAPIEnabled(profile) ||
CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableInstantExtendedAPI);

Powered by Google App Engine
This is Rietveld 408576698