Commit 0dd5e73a authored by Kunihiko Sakamoto's avatar Kunihiko Sakamoto Committed by Commit Bot

Revert "Navi: Add check for Local NTP flag before testing experiment variations"

This reverts commit b495cf02.

Reason for revert: Broke win-google-rel build (crbug.com/943912)

Original change's description:
> Navi: Add check for Local NTP flag before testing experiment variations
> 
> Bug: 938093
> Change-Id: Ib09b75a213d46536caf180bacb6eaf480372af2c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1529386
> Reviewed-by: Hector Carmona <hcarmona@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: John Lee <johntlee@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#642230}

TBR=sky@chromium.org,hcarmona@chromium.org,johntlee@chromium.org

Change-Id: I886cfcebc0e1a4be203d5bed6f5feacd83ec683b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 938093,943912
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1530365Reviewed-by: default avatarKunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642380}
parent 1aa94916
...@@ -1089,7 +1089,7 @@ void ProfileManager::InitProfileUserPrefs(Profile* profile) { ...@@ -1089,7 +1089,7 @@ void ProfileManager::InitProfileUserPrefs(Profile* profile) {
// Enterprise users should not be included in any NUX/Navi flow. // Enterprise users should not be included in any NUX/Navi flow.
if (!base::IsMachineExternallyManaged()) { if (!base::IsMachineExternallyManaged()) {
profile->GetPrefs()->SetString(prefs::kNaviOnboardGroup, profile->GetPrefs()->SetString(prefs::kNaviOnboardGroup,
nux::GetOnboardingGroup(profile)); nux::GetOnboardingGroup());
} }
#endif // defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) #endif // defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD)
} }
......
...@@ -9,21 +9,15 @@ ...@@ -9,21 +9,15 @@
namespace nux { namespace nux {
const base::Feature kNuxOnboardingFeature{"NuxOnboarding", const base::Feature kNuxOnboardingFeature{"NuxOnboarding",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
// nux-ntp-background should not be added here until we can guarantee that
// kUseGoogleLocalNtp is enabled
const char kDefaultNewUserModules[] =
"nux-google-apps,nux-email,nux-set-as-default,signin-view";
const char kDefaultReturningUserModules[] = "nux-set-as-default";
// The value of these FeatureParam values should be a comma-delimited list // The value of these FeatureParam values should be a comma-delimited list
// of element names whitelisted in the MODULES_WHITELIST list, defined in // of element names whitelisted in the MODULES_WHITELIST list, defined in
// chrome/browser/resources/welcome/onboarding_welcome/welcome_app.js // chrome/browser/resources/welcome/onboarding_welcome/welcome_app.js
const base::FeatureParam<std::string> kNuxOnboardingNewUserModules{ const base::FeatureParam<std::string> kNuxOnboardingNewUserModules{
&kNuxOnboardingFeature, "new-user-modules", kDefaultNewUserModules}; &kNuxOnboardingFeature, "new-user-modules",
"nux-google-apps,nux-email,nux-set-as-default,signin-view"};
const base::FeatureParam<std::string> kNuxOnboardingReturningUserModules{ const base::FeatureParam<std::string> kNuxOnboardingReturningUserModules{
&kNuxOnboardingFeature, "returning-user-modules", &kNuxOnboardingFeature, "returning-user-modules", "nux-set-as-default"};
kDefaultReturningUserModules};
} // namespace nux } // namespace nux
...@@ -16,9 +16,6 @@ namespace nux { ...@@ -16,9 +16,6 @@ namespace nux {
extern const base::Feature kNuxOnboardingFeature; extern const base::Feature kNuxOnboardingFeature;
extern const char kDefaultNewUserModules[];
extern const char kDefaultReturningUserModules[];
extern const base::FeatureParam<std::string> kNuxOnboardingNewUserModules; extern const base::FeatureParam<std::string> kNuxOnboardingNewUserModules;
extern const base::FeatureParam<std::string> kNuxOnboardingReturningUserModules; extern const base::FeatureParam<std::string> kNuxOnboardingReturningUserModules;
extern const base::FeatureParam<bool> kNuxOnboardingShowEmailInterstitial; extern const base::FeatureParam<bool> kNuxOnboardingShowEmailInterstitial;
......
...@@ -13,10 +13,7 @@ ...@@ -13,10 +13,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h" #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/ntp_features.h"
#include "chrome/browser/search/search.h"
#include "chrome/browser/ui/webui/welcome/nux/constants.h" #include "chrome/browser/ui/webui/welcome/nux/constants.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
...@@ -42,28 +39,10 @@ const base::FeatureParam<std::string> ...@@ -42,28 +39,10 @@ const base::FeatureParam<std::string>
&kNuxOnboardingForceEnabled, "returning-user-modules", &kNuxOnboardingForceEnabled, "returning-user-modules",
"nux-set-as-default"}; "nux-set-as-default"};
// Our current running experiment of testing the nux-ntp-background module
// depends on the Local NTP feature/experiment being enabled. To avoid polluting
// our data with users who cannot use the nux-ntp-background module, we need
// to check to make sure the Local NTP feature is enabled before running
// any experiment or even reading any feature params from our experiment.
bool CanExperimentWithVariations(Profile* profile) {
return (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceLocalNtp) ||
base::FeatureList::IsEnabled(features::kUseGoogleLocalNtp)) &&
search::DefaultSearchProviderIsGoogle(profile);
}
// Must match study name in configs. // Must match study name in configs.
const char kNuxOnboardingStudyName[] = "NaviOnboarding"; const char kNuxOnboardingStudyName[] = "NaviOnboarding";
std::string GetOnboardingGroup(Profile* profile) { std::string GetOnboardingGroup() {
if (!CanExperimentWithVariations(profile)) {
// If we cannot run any variations, we bucket the users into a separate
// synthetic group that we will ignore data for.
return "NaviNoVariationSynthetic";
}
// We need to use |base::GetFieldTrialParamValue| instead of // We need to use |base::GetFieldTrialParamValue| instead of
// |base::FeatureParam| because our control group needs a custom value for // |base::FeatureParam| because our control group needs a custom value for
// this param. // this param.
...@@ -94,14 +73,9 @@ bool IsNuxOnboardingEnabled(Profile* profile) { ...@@ -94,14 +73,9 @@ bool IsNuxOnboardingEnabled(Profile* profile) {
std::string onboard_group = prefs->GetString(prefs::kNaviOnboardGroup); std::string onboard_group = prefs->GetString(prefs::kNaviOnboardGroup);
if (onboard_group.empty()) { if (onboard_group.empty()) {
// Users who onboarded before Navi or are part of an enterprise.
return false; return false;
} }
if (!CanExperimentWithVariations(profile)) {
return true; // Default Navi behavior.
}
// User will be tied to their original onboarding group, even after // User will be tied to their original onboarding group, even after
// experiment ends. // experiment ends.
ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial( ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
...@@ -128,14 +102,10 @@ base::DictionaryValue GetNuxOnboardingModules(Profile* profile) { ...@@ -128,14 +102,10 @@ base::DictionaryValue GetNuxOnboardingModules(Profile* profile) {
kNuxOnboardingForceEnabledNewUserModules.Get()); kNuxOnboardingForceEnabledNewUserModules.Get());
modules.SetString("returning-user", modules.SetString("returning-user",
kNuxOnboardingForceEnabledReturningUserModules.Get()); kNuxOnboardingForceEnabledReturningUserModules.Get());
} else if (CanExperimentWithVariations(profile)) { } else { // This means |nux::kNuxOnboardingFeature| is enabled.
modules.SetString("new-user", kNuxOnboardingNewUserModules.Get()); modules.SetString("new-user", kNuxOnboardingNewUserModules.Get());
modules.SetString("returning-user", modules.SetString("returning-user",
kNuxOnboardingReturningUserModules.Get()); kNuxOnboardingReturningUserModules.Get());
} else {
// Default behavior w/o checking feature flag.
modules.SetString("new-user", kDefaultNewUserModules);
modules.SetString("returning-user", kDefaultReturningUserModules);
} }
return modules; return modules;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment