diff --git a/chrome/browser/chrome_browser_field_trials.cc b/chrome/browser/chrome_browser_field_trials.cc index dec9cdb8e7923a..b9d34431b8fda1 100644 --- a/chrome/browser/chrome_browser_field_trials.cc +++ b/chrome/browser/chrome_browser_field_trials.cc @@ -20,10 +20,12 @@ #include "chrome/browser/metrics/chrome_browser_sampling_trials.h" #include "chrome/browser/metrics/chrome_metrics_service_accessor.h" #include "chrome/browser/metrics/chrome_metrics_service_client.h" +#include "chrome/browser/ui/startup/first_run_service.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "components/metrics/metrics_pref_names.h" #include "components/metrics/persistent_histograms.h" +#include "components/signin/public/base/signin_buildflags.h" #include "components/version_info/version_info.h" #if BUILDFLAG(IS_ANDROID) @@ -179,4 +181,7 @@ void ChromeBrowserFieldTrials::RegisterSyntheticTrials() { fre_consistency_trial_variation_id_); } #endif // BUILDFLAG(IS_ANDROID) +#if BUILDFLAG(ENABLE_DICE_SUPPORT) + FirstRunService::EnsureStickToFirstRunCohort(); +#endif } diff --git a/chrome/browser/metrics/chrome_metrics_service_accessor.h b/chrome/browser/metrics/chrome_metrics_service_accessor.h index 47a78ddcaa925d..f78d1c9ba31a5b 100644 --- a/chrome/browser/metrics/chrome_metrics_service_accessor.h +++ b/chrome/browser/metrics/chrome_metrics_service_accessor.h @@ -136,6 +136,7 @@ class ChromeMetricsServiceAccessor : public metrics::MetricsServiceAccessor { friend class OptimizationGuideKeyedService; friend class WebUITabStripFieldTrial; friend class feed::FeedServiceDelegateImpl; + friend class FirstRunService; friend class browser_sync::DeviceInfoSyncClientImpl; friend class feed::WebFeedSubscriptionCoordinator; friend class HttpsFirstModeService; diff --git a/chrome/browser/signin/signin_features.cc b/chrome/browser/signin/signin_features.cc index 3d493dda0c9671..d26c0ebea0b27e 100644 --- a/chrome/browser/signin/signin_features.cc +++ b/chrome/browser/signin/signin_features.cc @@ -11,7 +11,7 @@ // Enables the new style, "For You" First Run Experience BASE_FEATURE(kForYouFre, "ForYouFre", base::FEATURE_DISABLED_BY_DEFAULT); -#if !BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(ENABLE_DICE_SUPPORT) // Whether the browser should be opened when the user closes the FRE window. If // false, we just exit Chrome and the user will get straight to the browser on // the next process launch. @@ -30,8 +30,19 @@ const base::FeatureParam kForYouFreSignInPromoVariant{ &kForYouFre, /*name=*/"signin_promo_variant", /*default_value=*/SigninPromoVariant::kSignIn, /*options=*/&kSignInPromoVariantOptions}; -#endif -#endif + +// Feature that indicates that we should put the client in a study group to +// be able to look at metrics in the long term. Does not affect the client's +// behavior by itself, instead this is done through the `kForYouFre` feature. +BASE_FEATURE(kForYouFreStudy, + "ForYouFreStudy", + base::FEATURE_DISABLED_BY_DEFAULT); +// String that refers to the study group in which this install was enrolled. +// Used to implement the sticky experiment tracking. +const base::FeatureParam kForYouFreStudyGroup{ + &kForYouFreStudy, /*name=*/"group_name", /*default_value=*/""}; +#endif // BUILDFLAG(ENABLE_DICE_SUPPORT) +#endif // !BUILDFLAG(IS_CHROMEOS_ASH) && !BUILDFLAG(IS_ANDROID) // Enables the client-side processing of the HTTP response header // Google-Accounts-RemoveLocalAccount. diff --git a/chrome/browser/signin/signin_features.h b/chrome/browser/signin/signin_features.h index dd1d61bd1c5893..1c6f04a92fbfdb 100644 --- a/chrome/browser/signin/signin_features.h +++ b/chrome/browser/signin/signin_features.h @@ -7,18 +7,23 @@ #include "base/feature_list.h" #include "base/metrics/field_trial_params.h" +#include "components/signin/public/base/signin_buildflags.h" #if !BUILDFLAG(IS_CHROMEOS_ASH) && !BUILDFLAG(IS_ANDROID) BASE_DECLARE_FEATURE(kForYouFre); -#if !BUILDFLAG(IS_CHROMEOS_LACROS) +#if BUILDFLAG(ENABLE_DICE_SUPPORT) extern const base::FeatureParam kForYouFreCloseShouldProceed; enum class SigninPromoVariant { kSignIn, kMakeYourOwn, kDoMore }; extern const base::FeatureParam kForYouFreSignInPromoVariant; -#endif -#endif + +BASE_DECLARE_FEATURE(kForYouFreStudy); + +extern const base::FeatureParam kForYouFreStudyGroup; +#endif // BUILDFLAG(ENABLE_DICE_SUPPORT) +#endif // !BUILDFLAG(IS_CHROMEOS_ASH) && !BUILDFLAG(IS_ANDROID) BASE_DECLARE_FEATURE(kProcessGaiaRemoveLocalAccountHeader); diff --git a/chrome/browser/ui/startup/first_run_service.cc b/chrome/browser/ui/startup/first_run_service.cc index 4215094855bb94..47a8efc5feef9f 100644 --- a/chrome/browser/ui/startup/first_run_service.cc +++ b/chrome/browser/ui/startup/first_run_service.cc @@ -17,6 +17,7 @@ #include "base/notreached.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/first_run/first_run.h" +#include "chrome/browser/metrics/chrome_metrics_service_accessor.h" #include "chrome/browser/policy/chrome_browser_policy_connector.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_metrics.h" @@ -34,6 +35,7 @@ #include "components/signin/public/base/consent_level.h" #include "components/signin/public/base/signin_pref_names.h" #include "components/signin/public/identity_manager/identity_manager.h" +#include "components/variations/synthetic_trials.h" #include "content/public/browser/browser_context.h" #if BUILDFLAG(IS_CHROMEOS_LACROS) @@ -233,8 +235,68 @@ void OnFirstRunHasExited(ResumeTaskCallback original_intent_callback, // static void FirstRunService::RegisterLocalStatePrefs(PrefRegistrySimple* registry) { registry->RegisterBooleanPref(prefs::kFirstRunFinished, false); + registry->RegisterStringPref(prefs::kFirstRunStudyGroup, ""); } +#if BUILDFLAG(ENABLE_DICE_SUPPORT) +// static +void FirstRunService::EnsureStickToFirstRunCohort() { + PrefService* local_state = g_browser_process->local_state(); + if (!local_state) { + return; // Can be null in unit tests; + } + + if (!IsFirstRunMarkedFinishedInPrefs()) { + // This user did not see the FRE. Their first run either happened before the + // feature was enabled, or it's happening right now. In the former case we + // don't enroll them, and in the latter, they will be enrolled right before + // starting the FRE. + return; + } + + auto enrolled_study_group = + local_state->GetString(prefs::kFirstRunStudyGroup); + if (enrolled_study_group.empty()) { + // The user was not enrolled or exited the study at some point. + return; + } + + RegisterSyntheticFieldTrial(enrolled_study_group); +} + +// static +void FirstRunService::JoinFirstRunCohort() { + PrefService* local_state = g_browser_process->local_state(); + if (!local_state) { + return; // Can be null in unit tests; + } + + // The First Run experience depends on experiment groups (see params + // associated with the `kForYouFre` feature). To measure the long terms impact + // of this one-shot experience, we save an associated group name to prefs so + // we can report it as a synthetic trial for understanding the effects for + // each specific configuration, disambiguating it from other clients who had + // a different experience (or did not see the FRE for some reason). + std::string active_study_group = kForYouFreStudyGroup.Get(); + if (active_study_group.empty()) { + return; // No active study, no need to sign up. + } + + local_state->SetString(prefs::kFirstRunStudyGroup, active_study_group); + RegisterSyntheticFieldTrial(active_study_group); +} + +// static +void FirstRunService::RegisterSyntheticFieldTrial( + const std::string& group_name) { + DCHECK(!group_name.empty()); + + ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial( + "ForYouFreSynthetic", group_name, + variations::SyntheticTrialAnnotationMode::kCurrentLog); +} +#endif // BUILDFLAG(ENABLE_DICE_SUPPORT) + FirstRunService::FirstRunService(Profile* profile) : profile_(profile) {} FirstRunService::~FirstRunService() = default; @@ -391,6 +453,17 @@ KeyedService* FirstRunServiceFactory::BuildServiceInstanceFor( } #if BUILDFLAG(ENABLE_DICE_SUPPORT) + if (base::FeatureList::IsEnabled(kForYouFreStudy)) { + // We use this point to register for the study as it can give us a good + // counterfactual, before checking the state of the feature itself. The + // service is created on demand so we are in a code path that will require + // the FRE to be shown. + // Besides being suppressed by enterprise policy, if the FRE doesn't run, it + // would be related to handling some corner cases, and should not impact our + // metrics too much. + FirstRunService::JoinFirstRunCohort(); + } + if (!base::FeatureList::IsEnabled(kForYouFre)) { return nullptr; } diff --git a/chrome/browser/ui/startup/first_run_service.h b/chrome/browser/ui/startup/first_run_service.h index f1a309e262249a..b394c8614b2635 100644 --- a/chrome/browser/ui/startup/first_run_service.h +++ b/chrome/browser/ui/startup/first_run_service.h @@ -13,6 +13,7 @@ #include "build/chromeos_buildflags.h" #include "chrome/browser/profiles/profile_keyed_service_factory.h" #include "components/keyed_service/core/keyed_service.h" +#include "components/signin/public/base/signin_buildflags.h" class PrefRegistrySimple; class Profile; @@ -44,6 +45,13 @@ class FirstRunService : public KeyedService { static void RegisterLocalStatePrefs(PrefRegistrySimple* registry); +#if BUILDFLAG(ENABLE_DICE_SUPPORT) + // Ensures that the user's experiment group is appropriately reported + // to track the effect of the first run experience over time. Should be called + // once per browser process startup. + static void EnsureStickToFirstRunCohort(); +#endif + explicit FirstRunService(Profile* profile); ~FirstRunService() override; @@ -73,6 +81,23 @@ class FirstRunService : public KeyedService { private: friend class FirstRunServiceFactory; +#if BUILDFLAG(ENABLE_DICE_SUPPORT) + // Enrolls this client with a synthetic field trial based on the Finch params. + // Should be called when the FRE is launched, then the client needs to + // register again on each process startup by calling + // `RegisterSyntheticFieldTrial()`. + static void JoinFirstRunCohort(); + + // Reports to the launch study for the First Run rollout. + // Notes: + // - This is declared here so it can have access to some private functions + // that need to be friended to be used. + // - The function is Dice-only as on Lacros (where this build flag is not set) + // the ForYouFre feature rollout will not go through this study process. The + // feature only guards an internal refactoring that does not have a + // user-visible effect. If will only have a killswitch. + static void RegisterSyntheticFieldTrial(const std::string& group_name); +#endif // Asynchronously attempts to complete the first run silently. // By the time `callback` is run (if non-null), either: diff --git a/chrome/browser/ui/startup/first_run_service_browsertest.cc b/chrome/browser/ui/startup/first_run_service_browsertest.cc index fb63633b5758dd..8bc90225bde012 100644 --- a/chrome/browser/ui/startup/first_run_service_browsertest.cc +++ b/chrome/browser/ui/startup/first_run_service_browsertest.cc @@ -33,6 +33,7 @@ #include "chrome/common/pref_names.h" #include "chrome/common/webui_url_constants.h" #include "chrome/test/base/in_process_browser_test.h" +#include "components/metrics/metrics_service.h" #include "components/policy/core/common/mock_configuration_policy_provider.h" #include "components/policy/core/common/policy_namespace.h" #include "components/policy/core/common/policy_service.h" @@ -42,10 +43,12 @@ #include "components/signin/public/identity_manager/identity_manager.h" #include "components/signin/public/identity_manager/identity_test_environment.h" #include "components/signin/public/identity_manager/primary_account_mutator.h" +#include "components/variations/synthetic_trials_active_group_id_provider.h" #include "content/public/browser/web_contents.h" #include "content/public/test/browser_test.h" #include "google_apis/gaia/core_account_id.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "ui/views/controls/webview/webview.h" #if BUILDFLAG(IS_CHROMEOS_LACROS) @@ -105,8 +108,9 @@ void SetIsFirstRun(bool is_first_run) { // browser that the test fixtures rely on. // So are manipulating flags here instead of during `SetUpX` methods on // purpose. - if (first_run::IsChromeFirstRun() == is_first_run) + if (first_run::IsChromeFirstRun() == is_first_run) { return; + } if (is_first_run) { // This switch is added by InProcessBrowserTest @@ -215,6 +219,13 @@ IN_PROC_BROWSER_TEST_F(FirstRunServiceBrowserTest, profiles::testing::WaitForPickerWidgetCreated(); EXPECT_FALSE(GetFirstRunFinishedPrefValue()); + // We don't expect synthetic trials to be registered here, since no group + // is configured with the feature. For the positive test case, see + // `FirstRunServiceCohortBrowserTest.GroupRegisteredAfterFre`. + PrefService* local_state = g_browser_process->local_state(); + EXPECT_FALSE(local_state->HasPrefPath(prefs::kFirstRunStudyGroup)); + EXPECT_FALSE(variations::HasSyntheticTrial("ForYouFreSynthetic")); + ProfilePicker::Hide(); run_loop.Run(); @@ -385,7 +396,89 @@ IN_PROC_BROWSER_TEST_F(FirstRunServiceNotForYouBrowserTest, EXPECT_TRUE(ShouldOpenFirstRun(profile())); EXPECT_EQ(nullptr, fre_service()); } -#endif + +class FirstRunServiceCohortBrowserTest : public FirstRunServiceBrowserTest { + public: + static constexpr char kStudyTestGroupName1[] = "test_group_1"; + static constexpr char kStudyTestGroupName2[] = "test_group_2"; + + FirstRunServiceCohortBrowserTest() { + variations::SyntheticTrialsActiveGroupIdProvider::GetInstance() + ->ResetForTesting(); + + scoped_feature_list_.InitWithFeaturesAndParameters( + { + {kForYouFreStudy, {{"group_name", kStudyTestGroupName1}}}, + {kForYouFre, {}}, + }, + {}); + } + + private: + base::test::ScopedFeatureList scoped_feature_list_; +}; + +IN_PROC_BROWSER_TEST_F(FirstRunServiceCohortBrowserTest, + PRE_GroupRegisteredAfterFre) { + EXPECT_TRUE(ShouldOpenFirstRun(browser()->profile())); + + // We don't expect the synthetic trial to be registered before the FRE runs. + PrefService* local_state = g_browser_process->local_state(); + EXPECT_FALSE(local_state->HasPrefPath(prefs::kFirstRunStudyGroup)); + EXPECT_FALSE(variations::HasSyntheticTrial("ForYouFreSynthetic")); + + base::RunLoop run_loop; + fre_service()->OpenFirstRunIfNeeded( + FirstRunService::EntryPoint::kOther, + ExpectProceed(true).Then(run_loop.QuitClosure())); + + // Opening the FRE triggers recording of the group. + EXPECT_EQ(kStudyTestGroupName1, + local_state->GetString(prefs::kFirstRunStudyGroup)); + EXPECT_TRUE(variations::HasSyntheticTrial("ForYouFreSynthetic")); + EXPECT_TRUE(variations::IsInSyntheticTrialGroup("ForYouFreSynthetic", + kStudyTestGroupName1)); + + profiles::testing::WaitForPickerWidgetCreated(); + ProfilePicker::Hide(); + profiles::testing::WaitForPickerClosed(); + run_loop.Run(); +} +IN_PROC_BROWSER_TEST_F(FirstRunServiceCohortBrowserTest, + GroupRegisteredAfterFre) { + EXPECT_FALSE(ShouldOpenFirstRun(browser()->profile())); + + PrefService* local_state = g_browser_process->local_state(); + EXPECT_EQ(kStudyTestGroupName1, + local_state->GetString(prefs::kFirstRunStudyGroup)); + EXPECT_TRUE(variations::IsInSyntheticTrialGroup("ForYouFreSynthetic", + kStudyTestGroupName1)); +} + +IN_PROC_BROWSER_TEST_F(FirstRunServiceCohortBrowserTest, + PRE_PRE_GroupViaPrefs) { + // Setting the pref, we expect it to get picked up in an upcoming startup. + PrefService* local_state = g_browser_process->local_state(); + local_state->SetString(prefs::kFirstRunStudyGroup, kStudyTestGroupName2); + + EXPECT_FALSE(variations::HasSyntheticTrial("ForYouFreSynthetic")); +} +IN_PROC_BROWSER_TEST_F(FirstRunServiceCohortBrowserTest, PRE_GroupViaPrefs) { + // The synthetic group should not be registered yet since we didn't go through + // the FRE. + EXPECT_FALSE(variations::HasSyntheticTrial("ForYouFreSynthetic")); + + // Setting this should make the next run finally register the synthetic trial. + PrefService* local_state = g_browser_process->local_state(); + local_state->SetBoolean(prefs::kFirstRunFinished, true); +} +IN_PROC_BROWSER_TEST_F(FirstRunServiceCohortBrowserTest, GroupViaPrefs) { + // The registered group is read from the prefs, not from the feature param. + EXPECT_TRUE(variations::IsInSyntheticTrialGroup("ForYouFreSynthetic", + kStudyTestGroupName2)); +} + +#endif // BUILDFLAG(ENABLE_DICE_SUPPORT) struct PolicyTestParam { const std::string test_suffix; diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 0ea376baddf8e4..b3f85efb885569 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -1630,6 +1630,11 @@ const char kWebRtcTextLogCollectionAllowed[] = // Boolean that indicates that the first run experience has been finished (or // skipped by some policy) for this browser install. const char kFirstRunFinished[] = "browser.first_run_finished"; + +// String that refers to the study group in which this install was enrolled. +// Used to implement the sticky experiment tracking. +// TODO(crbug.com/1418017): Clean up experiment setup. +const char kFirstRunStudyGroup[] = "browser.first_run_study_group"; #endif #if !BUILDFLAG(IS_ANDROID) diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index b84ac2c7891521..ea59ea46c2e8c4 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -538,6 +538,7 @@ extern const char kWebRtcTextLogCollectionAllowed[]; #if BUILDFLAG(IS_CHROMEOS_LACROS) || BUILDFLAG(ENABLE_DICE_SUPPORT) extern const char kFirstRunFinished[]; +extern const char kFirstRunStudyGroup[]; #endif #if !BUILDFLAG(IS_ANDROID)