From e23daf4b677c2b1257900e24804513b400b8d5ee Mon Sep 17 00:00:00 2001 From: "sreeram@chromium.org" Date: Tue, 26 Jul 2011 10:10:46 +0000 Subject: Introduce a field trial for Instant. Add a field trial for Instant, opting in 10% of undecided users into the experiment. "Undecided" means that users who have played with the Instant checkbox (in Preferences -> Basics) will be excluded from the field trial. When Instant is turned on in this manner, it's restricted to search (i.e., other non-search URLs are not previewed). If the user is selected into the experiment, the checkbox state in preferences will accurately reflect that Instant is on, even if the user didn't enable it explicitly. Although the experiment enables the "Restrict Instant to Search" functionality, it's not reflected in about:flags. I consider this deficiency to be minor, since (i) most users won't check about:flags anyway, and (ii) about:flags is already inconsistent in other ways (for example, it also doesn't show if you added --restrict-instant-to-search manually). Similarly for --preload-instant-search, which preloads the default search engine's homepage when the omnibox gets keyboard focus. We respect group policy. If prefs::kInstantEnabled is a managed pref, the field trial will not affect the user. We store the actual random number generated for the user in a pref so that the same user gets the same experience across browser restarts (i.e., one-time randomization). This also allows us to change (through a Chrome update) the percentages allocated to different groups and thus allocate users to groups differently. If we only stored the final determination of the user's group in the pref, changing the group percentages would have no effect. The code doesn't use base::FieldTrial, because this experiment needs one-time randomization, and base::FieldTrial only supports it if the user has opted-in to UMA (see http://codereview.chromium.org/7360001/ for some context). There is no explicit expiry date for this experiment. The experiment can be disabled by updating the field trial percentages in instant_field_trial.cc and shipping an update to Chrome (not much different from how base::FieldTrial also requires new builds to expire experiments). BUG=none TEST=Delete profile and restart Chrome enough times to see that Instant is ON by default about 10% of the time; fiddling with the preferences checkbox gives expected results. Review URL: http://codereview.chromium.org/7337007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@94073 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/autocomplete/autocomplete.cc | 9 +++- chrome/browser/autocomplete/autocomplete.h | 7 +++- .../browser/autocomplete/autocomplete_unittest.cc | 6 ++- .../autocomplete/search_provider_unittest.cc | 6 ++- chrome/browser/instant/instant_controller.cc | 30 +++++++++---- chrome/browser/instant/instant_field_trial.cc | 47 +++++++++++++++++++++ chrome/browser/instant/instant_field_trial.h | 49 ++++++++++++++++++++++ .../browser/resources/options/browser_options.html | 2 + .../browser/resources/options/browser_options.js | 33 +++++++++++++++ .../ui/webui/options/browser_options_handler.cc | 11 +++++ .../ui/webui/options/browser_options_handler.h | 3 ++ chrome/chrome_browser.gypi | 2 + chrome/common/pref_names.cc | 4 ++ chrome/common/pref_names.h | 1 + chrome/test/data/webui/options.js | 2 + chrome/test/functional/instant.py | 1 + 16 files changed, 199 insertions(+), 14 deletions(-) create mode 100644 chrome/browser/instant/instant_field_trial.cc create mode 100644 chrome/browser/instant/instant_field_trial.h diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 48e7dd6..6a87b83 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -25,6 +25,7 @@ #include "chrome/browser/autocomplete/shortcuts_provider.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/external_protocol/external_protocol_handler.h" +#include "chrome/browser/instant/instant_field_trial.h" #include "chrome/browser/net/url_fixer_upper.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" @@ -793,7 +794,8 @@ AutocompleteController::AutocompleteController( AutocompleteControllerDelegate* delegate) : delegate_(delegate), done_(true), - in_start_(false) { + in_start_(false), + profile_(profile) { search_provider_ = new SearchProvider(this, profile); providers_.push_back(search_provider_); // TODO(mrossetti): Remove the following and permanently modify the @@ -839,6 +841,7 @@ void AutocompleteController::SetProfile(Profile* profile) { (*i)->SetProfile(profile); input_.Clear(); // Ensure we don't try to do a "minimal_changes" query on a // different profile. + profile_ = profile; } void AutocompleteController::Start( @@ -880,7 +883,9 @@ void AutocompleteController::Start( if (matches_requested == AutocompleteInput::ALL_MATCHES && (text.length() < 6)) { base::TimeTicks end_time = base::TimeTicks::Now(); - std::string name = "Omnibox.QueryTime." + base::IntToString(text.length()); + std::string name = "Omnibox.QueryTime." + + InstantFieldTrial::GetGroupName(profile_) + + base::IntToString(text.length()); base::Histogram* counter = base::Histogram::FactoryGet( name, 1, 1000, 50, base::Histogram::kUmaTargetedHistogramFlag); counter->Add(static_cast((end_time - start_time).InMilliseconds())); diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index 2a19545..487e8bc 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -599,13 +599,14 @@ class AutocompleteController : public ACProviderListener { AutocompleteController(Profile* profile, AutocompleteControllerDelegate* delegate); #ifdef UNIT_TEST - explicit AutocompleteController(const ACProviders& providers) + AutocompleteController(const ACProviders& providers, Profile* profile) : delegate_(NULL), providers_(providers), keyword_provider_(NULL), search_provider_(NULL), done_(true), - in_start_(false) { + in_start_(false), + profile_(profile) { } #endif ~AutocompleteController(); @@ -728,6 +729,8 @@ class AutocompleteController : public ACProviderListener { // notifications until Start() has been invoked on all providers. bool in_start_; + Profile* profile_; + DISALLOW_COPY_AND_ASSIGN(AutocompleteController); }; diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc index 2b2a17b..be819c7 100644 --- a/chrome/browser/autocomplete/autocomplete_unittest.cc +++ b/chrome/browser/autocomplete/autocomplete_unittest.cc @@ -153,7 +153,8 @@ void AutocompleteProviderTest::ResetControllerWithTestProviders( providers_.push_back(providerB); // Reset the controller to contain our new providers. - AutocompleteController* controller = new AutocompleteController(providers_); + AutocompleteController* controller = + new AutocompleteController(providers_, &profile_); controller_.reset(controller); providerA->set_listener(controller); providerB->set_listener(controller); @@ -201,7 +202,8 @@ void AutocompleteProviderTest:: search_provider->AddRef(); providers_.push_back(search_provider); - AutocompleteController* controller = new AutocompleteController(providers_); + AutocompleteController* controller = + new AutocompleteController(providers_, &profile_); controller_.reset(controller); } diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index 99b07e0..2f5a3a9 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -153,6 +153,10 @@ void SearchProviderTest::SetUp() { provider_ = new SearchProvider(this, &profile_); URLFetcher::set_factory(&test_factory_); + + // Prevent the Instant field trial from kicking in. + PrefService* service = profile_.GetPrefs(); + service->SetBoolean(prefs::kInstantEnabledOnce, true); } void SearchProviderTest::OnProviderUpdate(bool updated_matches) { @@ -623,7 +627,7 @@ TEST_F(SearchProviderTest, UpdateKeywordDescriptions) { ACProviders providers; SearchProvider* provider = provider_.release(); providers.push_back(provider); - AutocompleteController controller(providers); + AutocompleteController controller(providers, &profile_); controller.set_search_provider(provider); provider->set_listener(&controller); controller.Start(ASCIIToUTF16("k t"), string16(), false, false, true, diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc index 4e99b3d..fad2793 100644 --- a/chrome/browser/instant/instant_controller.cc +++ b/chrome/browser/instant/instant_controller.cc @@ -7,9 +7,11 @@ #include "base/command_line.h" #include "base/message_loop.h" #include "base/metrics/histogram.h" +#include "base/rand_util.h" #include "build/build_config.h" #include "chrome/browser/autocomplete/autocomplete_match.h" #include "chrome/browser/instant/instant_delegate.h" +#include "chrome/browser/instant/instant_field_trial.h" #include "chrome/browser/instant/instant_loader.h" #include "chrome/browser/instant/instant_loader_manager.h" #include "chrome/browser/instant/promo_counter.h" @@ -49,9 +51,10 @@ InstantController::InstantController(Profile* profile, last_transition_type_(PageTransition::LINK), ALLOW_THIS_IN_INITIALIZER_LIST(destroy_factory_(this)) { PrefService* service = profile->GetPrefs(); - if (service) { - // kInstantWasEnabledOnce was added after instant, set it now to make sure - // it is correctly set. + if (service && + InstantFieldTrial::GetGroup(profile) == InstantFieldTrial::INACTIVE) { + // kInstantEnabledOnce was added after instant, set it now to make sure it + // is correctly set. service->SetBoolean(prefs::kInstantEnabledOnce, true); } } @@ -73,6 +76,9 @@ void InstantController::RegisterUserPrefs(PrefService* prefs) { prefs->RegisterInt64Pref(prefs::kInstantEnabledTime, false, PrefService::UNSYNCABLE_PREF); + prefs->RegisterIntegerPref(prefs::kInstantFieldTrialRandomDraw, + base::RandInt(0, 9999), + PrefService::UNSYNCABLE_PREF); PromoCounter::RegisterUserPrefs(prefs, prefs::kInstantPromo); } @@ -100,7 +106,8 @@ void InstantController::RecordMetrics(Profile* profile) { // static bool InstantController::IsEnabled(Profile* profile) { PrefService* prefs = profile->GetPrefs(); - return prefs->GetBoolean(prefs::kInstantEnabled); + return prefs->GetBoolean(prefs::kInstantEnabled) || + InstantFieldTrial::IsExperimentGroup(profile); } // static @@ -113,11 +120,11 @@ void InstantController::Enable(Profile* profile) { if (!service) return; + service->SetBoolean(prefs::kInstantEnabledOnce, true); service->SetBoolean(prefs::kInstantEnabled, true); service->SetBoolean(prefs::kInstantConfirmDialogShown, true); service->SetInt64(prefs::kInstantEnabledTime, base::Time::Now().ToInternalValue()); - service->SetBoolean(prefs::kInstantEnabledOnce, true); } // static @@ -135,6 +142,12 @@ void InstantController::Disable(Profile* profile) { delta.InMinutes(), 1, 60 * 24 * 10, 50); } + if (InstantFieldTrial::IsExperimentGroup(profile)) { + UMA_HISTOGRAM_COUNTS( + "Instant.FieldTrialOptOut." + InstantFieldTrial::GetGroupName(profile), + 1); + } + service->SetBoolean(prefs::kInstantEnabledOnce, true); service->SetBoolean(prefs::kInstantEnabled, false); } @@ -377,8 +390,10 @@ void InstantController::OnAutocompleteLostFocus( void InstantController::OnAutocompleteGotFocus( TabContentsWrapper* tab_contents) { CommandLine* cl = CommandLine::ForCurrentProcess(); - if (!cl->HasSwitch(switches::kPreloadInstantSearch)) + if (!cl->HasSwitch(switches::kPreloadInstantSearch) && + !InstantFieldTrial::IsExperimentGroup(tab_contents->profile())) { return; + } if (is_active_) return; @@ -705,7 +720,8 @@ InstantController::PreviewCondition InstantController::GetPreviewConditionFor( return PREVIEW_CONDITION_BLACKLISTED; const CommandLine* cl = CommandLine::ForCurrentProcess(); - if (cl->HasSwitch(switches::kRestrictInstantToSearch) && + if ((cl->HasSwitch(switches::kRestrictInstantToSearch) || + InstantFieldTrial::IsExperimentGroup(tab_contents_->profile())) && match.type != AutocompleteMatch::SEARCH_WHAT_YOU_TYPED && match.type != AutocompleteMatch::SEARCH_HISTORY && match.type != AutocompleteMatch::SEARCH_SUGGEST && diff --git a/chrome/browser/instant/instant_field_trial.cc b/chrome/browser/instant/instant_field_trial.cc new file mode 100644 index 0000000..90ff542 --- /dev/null +++ b/chrome/browser/instant/instant_field_trial.cc @@ -0,0 +1,47 @@ +// Copyright (c) 2011 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/instant/instant_field_trial.h" + +#include "chrome/browser/prefs/pref_service.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/common/pref_names.h" + +// static +InstantFieldTrial::Group InstantFieldTrial::GetGroup(Profile* profile) { + if (profile->IsOffTheRecord()) + return INACTIVE; + + const PrefService* prefs = profile->GetPrefs(); + if (!prefs || + prefs->GetBoolean(prefs::kInstantEnabledOnce) || + prefs->IsManagedPreference(prefs::kInstantEnabled)) { + return INACTIVE; + } + + const int random = prefs->GetInteger(prefs::kInstantFieldTrialRandomDraw); + return random < 4500 ? CONTROL1 : // 45% + random < 9000 ? CONTROL2 : // 45% + random < 9500 ? EXPERIMENT1 // 5% + : EXPERIMENT2; // 5% +} + +// static +bool InstantFieldTrial::IsExperimentGroup(Profile* profile) { + Group group = GetGroup(profile); + return group == EXPERIMENT1 || group == EXPERIMENT2; +} + +// static +std::string InstantFieldTrial::GetGroupName(Profile* profile) { + switch (GetGroup(profile)) { + case INACTIVE: return "InstantInactive"; + case CONTROL1: return "InstantControl1"; + case CONTROL2: return "InstantControl2"; + case EXPERIMENT1: return "InstantExperiment1"; + case EXPERIMENT2: return "InstantExperiment2"; + } + NOTREACHED(); + return "InstantUnknown"; +} diff --git a/chrome/browser/instant/instant_field_trial.h b/chrome/browser/instant/instant_field_trial.h new file mode 100644 index 0000000..e4149bc --- /dev/null +++ b/chrome/browser/instant/instant_field_trial.h @@ -0,0 +1,49 @@ +// Copyright (c) 2011 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. + +#ifndef CHROME_BROWSER_INSTANT_INSTANT_FIELD_TRIAL_H_ +#define CHROME_BROWSER_INSTANT_INSTANT_FIELD_TRIAL_H_ + +#include + +#include "base/basictypes.h" + +class Profile; + +// This class manages the Instant field trial. Each user is in exactly one of +// three field trial groups: Inactive, Control or Experiment. +// - Inactive users are those who have played with the Instant option in the +// Preferences page, or those for whom group policy provides an override, or +// those with an incognito profile, etc. The field trial is inactive for such +// users, so their Instant preference setting is respected. +// - Control and Experiment are all the other users, i.e., those who have never +// touched the Preferences option. Some percentage of these users are chosen +// into the Experiment group and get Instant enabled automatically. The rest +// fall into the Control group; for them, Instant remains disabled by default. +// - Control and Experiment are further split into two subgroups each, in order +// to detect bias between them (when analyzing metrics). The subgroups are +// treated identically for all other purposes. +class InstantFieldTrial { + public: + enum Group { + INACTIVE, + CONTROL1, + CONTROL2, + EXPERIMENT1, + EXPERIMENT2, + }; + static Group GetGroup(Profile* profile); + + // Check if the group is either of the two experiment subgroups. + static bool IsExperimentGroup(Profile* profile); + + // Returns a string describing the user's group. Can be added to histogram + // names, to split histograms by field trial groups. + static std::string GetGroupName(Profile* profile); + + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(InstantFieldTrial); +}; + +#endif // CHROME_BROWSER_INSTANT_INSTANT_FIELD_TRIAL_H_ diff --git a/chrome/browser/resources/options/browser_options.html b/chrome/browser/resources/options/browser_options.html index d03df9b..e23b0b8 100644 --- a/chrome/browser/resources/options/browser_options.html +++ b/chrome/browser/resources/options/browser_options.html @@ -94,6 +94,8 @@