Skip to content

Commit

Permalink
Enable client side field trial for per-user metrics.
Browse files Browse the repository at this point in the history
The feature involves OOBE and cannot depend on the finch seed. Thus, a
client-side field trial is created to do a 50% dev/beta rollout.

Tested: I tested on an ekko device with 100% enabled to ensure that both features were enabled. I also tested with 100% disabled to ensure that both feature were disabled.

(cherry picked from commit 729a0ae)

Bug: 1186354
Change-Id: If1b140b8f65ccfe90b1249660941cd397ef86c3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3627157
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Commit-Queue: Jong Ahn <jongahn@chromium.org>
Reviewed-by: Greg Levin <glevin@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1003901}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3651742
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/branch-heads/5060@{#59}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Jong Ahn authored and Chromium LUCI CQ committed May 17, 2022
1 parent 3b1bb31 commit 34f09c4
Show file tree
Hide file tree
Showing 6 changed files with 294 additions and 0 deletions.
120 changes: 120 additions & 0 deletions chrome/browser/ash/login/consolidated_consent_field_trial.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// Copyright 2022 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/ash/login/consolidated_consent_field_trial.h"

#include "ash/constants/ash_features.h"
#include "base/feature_list.h"
#include "base/metrics/field_trial.h"
#include "base/strings/string_number_conversions.h"
#include "chrome/common/channel_info.h"
#include "chromeos/constants/chromeos_features.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "components/variations/variations_associated_data.h"
#include "components/version_info/version_info.h"

namespace ash::consolidated_consent_field_trial {

namespace {

// Probabilities for all field trial groups add up to kTotalProbability.
const base::FieldTrial::Probability kTotalProbability = 100;

// Creates the field trial.
scoped_refptr<base::FieldTrial> CreateFieldTrial() {
return base::FieldTrialList::FactoryGetFieldTrial(
kTrialName, kTotalProbability, kDisabledGroup,
base::FieldTrial::ONE_TIME_RANDOMIZED,
/*default_group_number=*/nullptr);
}

// Sets the feature state based on the trial group.
void SetFeatureState(base::FeatureList* feature_list,
base::FieldTrial* trial,
const std::string& group_name) {
const base::FeatureList::OverrideState feature_state =
group_name == kEnabledGroup ? base::FeatureList::OVERRIDE_ENABLE_FEATURE
: base::FeatureList::OVERRIDE_DISABLE_FEATURE;

// Both features need to be in the same state.
feature_list->RegisterFieldTrialOverride(
ash::features::kOobeConsolidatedConsent.name, feature_state, trial);
feature_list->RegisterFieldTrialOverride(ash::features::kPerUserMetrics.name,
feature_state, trial);
}

// Creates a trial if there is no group name saved and enables the features
// based on the randomly selected trial group. Returns the group name.
std::string CreateFreshTrial(base::FeatureList* feature_list) {
int enabled_percent;
int disabled_percent;
switch (chrome::GetChannel()) {
case version_info::Channel::UNKNOWN:
case version_info::Channel::CANARY:
case version_info::Channel::DEV:
case version_info::Channel::BETA:
enabled_percent = 50;
disabled_percent = 50;
break;
case version_info::Channel::STABLE:
// Disabled on stable until green signals seen in experiment.
enabled_percent = 0;
disabled_percent = 100;
break;
}
DCHECK_EQ(kTotalProbability, enabled_percent + disabled_percent);

// Set up the trial and groups.
scoped_refptr<base::FieldTrial> trial = CreateFieldTrial();
trial->AppendGroup(kEnabledGroup, enabled_percent);
trial->AppendGroup(kDisabledGroup, disabled_percent);

// Finalize the group choice and set the feature state.
const std::string& group_name = trial->GetGroupNameWithoutActivation();
SetFeatureState(feature_list, trial.get(), group_name);
return group_name;
}

// Creates a trial with a single group and sets the feature flag to the state
// for that group.
void CreateSubsequentRunTrial(base::FeatureList* feature_list,
const std::string& group_name) {
scoped_refptr<base::FieldTrial> trial = CreateFieldTrial();
trial->AppendGroup(group_name, kTotalProbability);
SetFeatureState(feature_list, trial.get(), group_name);
}

} // namespace

void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
registry->RegisterStringPref(kTrialGroupPrefName, std::string());
}

void Create(base::FeatureList* feature_list, PrefService* local_state) {
// Experiment is currently disabled on STABLE channel. Storing the pref would
// cause a skew in STABLE when this experiment is rolled out to stable channel
// as existing clients would be in the |kDisabled| group.
if (chrome::GetChannel() == version_info::Channel::STABLE)
return;

// Load the trial group from local state. Groups should be consistent once
// assigned for the device since the feature involves OOBE and modifies
// metrics opt-in/out model.
std::string trial_group = local_state->GetString(kTrialGroupPrefName);

if (trial_group.empty()) {
// No group assigned for the device yet. Assign a trial group.
trial_group = CreateFreshTrial(feature_list);

// Persist the assigned group for subsequent runs.
local_state->SetString(kTrialGroupPrefName, trial_group);
} else {
// Group already assigned. Toggle relevant features depending on
// |trial_group| assigned.
CreateSubsequentRunTrial(feature_list, trial_group);
}
}

} // namespace ash::consolidated_consent_field_trial
49 changes: 49 additions & 0 deletions chrome/browser/ash/login/consolidated_consent_field_trial.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2022 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_ASH_LOGIN_CONSOLIDATED_CONSENT_FIELD_TRIAL_H_
#define CHROME_BROWSER_ASH_LOGIN_CONSOLIDATED_CONSENT_FIELD_TRIAL_H_

#include "base/feature_list.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"

namespace ash::consolidated_consent_field_trial {

// String local state preference with the name of the assigned trial group.
// Empty if no group has been assigned yet.
inline constexpr char kTrialGroupPrefName[] = "per_user_metrics.trial_group";

// The field trial name.
inline constexpr char kTrialName[] = "ConsolidatedScreenAndPerUserMetrics";

// Group names for the trial.
inline constexpr char kEnabledGroup[] = "Enabled";
inline constexpr char kDisabledGroup[] = "Disabled";

// Registers the local state pref to hold the trial group.
//
// If the device does not belong to an experiment group, an experiment group
// will be assigned and the experiment group will be stored in local state. This
// is to keep states consistent across sessions even if the variation seed
// changes since the feature modifies the metrics opt-in/out model.
void RegisterLocalStatePrefs(PrefRegistrySimple* registry);

// Enables client-side rollout for kOobeConsolidatedConsent and kPerUserMetrics
// features. These two features must be coupled together as they are dependent
// on each other.
//
// kOobeConsolidatedConsent is an OOBE feature and cannot depend on the
// variation seed being available. Thus, a client-side trial is created to
// control these two features.
//
// See crbug/1186354 for OOBE consolidated screen and crbug/1181504 for
// per-user metrics collection.
//
// The rollout plan for this feature is 50% for dev/beta.
void Create(base::FeatureList* feature_list, PrefService* local_state);

} // namespace ash::consolidated_consent_field_trial

#endif // CHROME_BROWSER_ASH_LOGIN_CONSOLIDATED_CONSENT_FIELD_TRIAL_H_
115 changes: 115 additions & 0 deletions chrome/browser/ash/login/consolidated_consent_field_trial_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// Copyright 2022 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/ash/login/consolidated_consent_field_trial.h"

#include "ash/constants/ash_features.h"
#include "base/feature_list.h"
#include "base/metrics/field_trial.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/common/channel_info.h"
#include "components/prefs/testing_pref_service.h"
#include "components/version_info/version_info.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace ash::consolidated_consent_field_trial {

using ::testing::Eq;

class ConsolidatedConsentFieldTrialTest : public testing::Test {
protected:
ConsolidatedConsentFieldTrialTest() {}

void SetUp() override { RegisterLocalStatePrefs(pref_service_.registry()); }

PrefService* local_state() { return &pref_service_; }

TestingPrefServiceSimple pref_service_;
};

TEST_F(ConsolidatedConsentFieldTrialTest,
FieldTrialCreatesProperlyWithNoExistingPref) {
base::FeatureList feature_list;

// Assert that pref has not been assigned.
EXPECT_THAT(local_state()->GetString(kTrialGroupPrefName),
::testing::IsEmpty());

Create(&feature_list, local_state());

// Ensure that the correct state is preserved so on the next run, the trial
// does not change.
const std::string expected_pref_value =
local_state()->GetString(kTrialGroupPrefName);

// Experiment should not be active on STABLE channel. This check is here in
// case there are any test that run in the stable channel.
if (chrome::GetChannel() != version_info::Channel::STABLE) {
EXPECT_TRUE(expected_pref_value == kEnabledGroup ||
expected_pref_value == kDisabledGroup);

auto* trial = base::FieldTrialList::Find(kTrialName);

// Ensure that the trial group assigned is the same as the group stored in
// the pref and has not been activated.
EXPECT_EQ(expected_pref_value, trial->GetGroupNameWithoutActivation());
EXPECT_FALSE(base::FieldTrialList::IsTrialActive(kTrialName));

EXPECT_TRUE(feature_list.IsFeatureOverridden(
ash::features::kOobeConsolidatedConsent.name));
EXPECT_TRUE(
feature_list.IsFeatureOverridden(ash::features::kPerUserMetrics.name));
} else {
EXPECT_THAT(base::FieldTrialList::Find(kTrialName), ::testing::IsNull());
}
}

class ConsolidatedConsentFieldTrialExistingGroupTest
: public ConsolidatedConsentFieldTrialTest,
public ::testing::WithParamInterface<const char*> {};

TEST_P(ConsolidatedConsentFieldTrialExistingGroupTest,
FieldTrialUsesLocalStateGroupIfExists) {
base::FeatureList feature_list;
const std::string group_name = GetParam();

// assert that pref has not been assigned.
EXPECT_THAT(local_state()->GetString(kTrialGroupPrefName), Eq(""));

// Set pref to enabled group.
local_state()->SetString(kTrialGroupPrefName, group_name);
Create(&feature_list, local_state());

// Experiment should not be active on STABLE channel. This check is here in
// case there are any test that run in the stable channel.
if (chrome::GetChannel() != version_info::Channel::STABLE) {
// Pref should not change.
const std::string expected_pref_value =
local_state()->GetString(kTrialGroupPrefName);
EXPECT_EQ(expected_pref_value, group_name);

auto* trial = base::FieldTrialList::Find(kTrialName);

// Field trial should have the same value as the pref.
EXPECT_EQ(group_name, trial->GetGroupNameWithoutActivation());

EXPECT_FALSE(base::FieldTrialList::IsTrialActive(kTrialName));
EXPECT_TRUE(feature_list.IsFeatureOverridden(
ash::features::kOobeConsolidatedConsent.name));
EXPECT_TRUE(
feature_list.IsFeatureOverridden(ash::features::kPerUserMetrics.name));

// TODO(jongahn): Figure out how to check feature state without
// |feature_list| being initialized or how to initialize |feature_list|.
} else {
EXPECT_THAT(base::FieldTrialList::Find(kTrialName), ::testing::IsNull());
}
}

INSTANTIATE_TEST_SUITE_P(FieldTrialUsesLocalStateGroupIfExists,
ConsolidatedConsentFieldTrialExistingGroupTest,
testing::ValuesIn({kDisabledGroup, kEnabledGroup}));

} // namespace ash::consolidated_consent_field_trial
2 changes: 2 additions & 0 deletions chrome/browser/ash/preferences.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "chrome/browser/ash/drive/file_system_util.h"
#include "chrome/browser/ash/input_method/input_method_persistence.h"
#include "chrome/browser/ash/input_method/input_method_syncer.h"
#include "chrome/browser/ash/login/consolidated_consent_field_trial.h"
#include "chrome/browser/ash/login/login_pref_names.h"
#include "chrome/browser/ash/login/session/user_session_manager.h"
#include "chrome/browser/ash/profiles/profile_helper.h"
Expand Down Expand Up @@ -154,6 +155,7 @@ void Preferences::RegisterPrefs(PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(::prefs::kConsumerAutoUpdateToggle, true);

RegisterLocalStatePrefs(registry);
ash::consolidated_consent_field_trial::RegisterLocalStatePrefs(registry);
}

// static
Expand Down
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 @@ -39,6 +39,7 @@

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "ash/services/multidevice_setup/public/cpp/first_run_field_trial.h"
#include "chrome/browser/ash/login/consolidated_consent_field_trial.h"
#endif

namespace {
Expand Down Expand Up @@ -82,6 +83,10 @@ void ChromeBrowserFieldTrials::SetUpFeatureControllingFieldTrials(
bool has_seed,
const base::FieldTrial::EntropyProvider* low_entropy_provider,
base::FeatureList* feature_list) {
#if BUILDFLAG(IS_CHROMEOS_ASH)
ash::consolidated_consent_field_trial::Create(feature_list, local_state_);
#endif

// Only create the fallback trials if there isn't already a variations seed
// being applied. This should occur during first run when first-run variations
// isn't supported. It's assumed that, if there is a seed, then it either
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1823,6 +1823,8 @@ source_set("chromeos") {
"../ash/login/chrome_restart_request.h",
"../ash/login/configuration_keys.cc",
"../ash/login/configuration_keys.h",
"../ash/login/consolidated_consent_field_trial.cc",
"../ash/login/consolidated_consent_field_trial.h",
"../ash/login/demo_mode/demo_extensions_external_loader.cc",
"../ash/login/demo_mode/demo_extensions_external_loader.h",
"../ash/login/demo_mode/demo_mode_resources_remover.cc",
Expand Down Expand Up @@ -4384,6 +4386,7 @@ source_set("unit_tests") {
"../ash/lock_screen_apps/lock_screen_profile_creator_impl_unittest.cc",
"../ash/lock_screen_apps/state_controller_unittest.cc",
"../ash/login/auth/cryptohome_authenticator_unittest.cc",
"../ash/login/consolidated_consent_field_trial_unittest.cc",
"../ash/login/demo_mode/demo_extensions_external_loader_unittest.cc",
"../ash/login/demo_mode/demo_mode_resources_remover_unittest.cc",
"../ash/login/demo_mode/demo_resources_unittest.cc",
Expand Down

0 comments on commit 34f09c4

Please sign in to comment.