Skip to content

Commit

Permalink
[fyfre] Implement long-term cohort tracking for FRE study
Browse files Browse the repository at this point in the history
Makes clients start registering with a study cohort (name of
the cohort obtained via a new param of the ForYouFreStudy feature)
when they attempt to run the FRE. The cohort and its name don't
affect the behaviour of the client by themselves, this is done
through the ForYouFre feature and its params.

The cohort registration is done through a synthetic trial, where
the group gets pulled from prefs, so that we can  annotate sessions
with the group name in subsequent startups to be able to track the
outcome of a specific first run config in the long run.

See http://go/for-you-fre-exp for more details.

Bug: 1402712
Change-Id: I4d392cc9aaaaf31385fadda569d621d086611900
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4244081
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1109150}
  • Loading branch information
Nicolas Dossou-Gbete authored and Chromium LUCI CQ committed Feb 23, 2023
1 parent e625065 commit 1b15ffb
Show file tree
Hide file tree
Showing 9 changed files with 227 additions and 8 deletions.
5 changes: 5 additions & 0 deletions chrome/browser/chrome_browser_field_trials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -179,4 +181,7 @@ void ChromeBrowserFieldTrials::RegisterSyntheticTrials() {
fre_consistency_trial_variation_id_);
}
#endif // BUILDFLAG(IS_ANDROID)
#if BUILDFLAG(ENABLE_DICE_SUPPORT)
FirstRunService::EnsureStickToFirstRunCohort();
#endif
}
1 change: 1 addition & 0 deletions chrome/browser/metrics/chrome_metrics_service_accessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 14 additions & 3 deletions chrome/browser/signin/signin_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -30,8 +30,19 @@ const base::FeatureParam<SigninPromoVariant> 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<std::string> 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.
Expand Down
11 changes: 8 additions & 3 deletions chrome/browser/signin/signin_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> kForYouFreCloseShouldProceed;

enum class SigninPromoVariant { kSignIn, kMakeYourOwn, kDoMore };
extern const base::FeatureParam<SigninPromoVariant>
kForYouFreSignInPromoVariant;
#endif
#endif

BASE_DECLARE_FEATURE(kForYouFreStudy);

extern const base::FeatureParam<std::string> kForYouFreStudyGroup;
#endif // BUILDFLAG(ENABLE_DICE_SUPPORT)
#endif // !BUILDFLAG(IS_CHROMEOS_ASH) && !BUILDFLAG(IS_ANDROID)

BASE_DECLARE_FEATURE(kProcessGaiaRemoveLocalAccountHeader);

Expand Down
73 changes: 73 additions & 0 deletions chrome/browser/ui/startup/first_run_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
25 changes: 25 additions & 0 deletions chrome/browser/ui/startup/first_run_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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:
Expand Down
97 changes: 95 additions & 2 deletions chrome/browser/ui/startup/first_run_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -220,6 +224,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();

Expand Down Expand Up @@ -397,7 +408,89 @@ IN_PROC_BROWSER_TEST_F(FirstRunServiceNotForYouBrowserTest,
EXPECT_TRUE(ShouldOpenFirstRun(profile()));
EXPECT_EQ(nullptr, CreateFirstRunService());
}
#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;
CreateFirstRunService()->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;
Expand Down
5 changes: 5 additions & 0 deletions chrome/common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions chrome/common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 1b15ffb

Please sign in to comment.