Skip to content

Commit

Permalink
Remove personalized promos user actions.
Browse files Browse the repository at this point in the history
User actions Signin_Sign{WithDefault|NotDefault|
NewAccountNoExistingAccount|SigninNewAccountExistingAccount}_From* were
added for the personalized sign-in promos and they are no longer used to
understand how user interact with these promos. This CL removes them.

Fixed: 1334221

Change-Id: Idd4de0c4fd1ac7bf9f8158b8b5f75d8369e781b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3695197
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Auto-Submit: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Nohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1013459}
  • Loading branch information
Mihai Sardarescu authored and Chromium LUCI CQ committed Jun 13, 2022
1 parent eca4595 commit 30501ff
Show file tree
Hide file tree
Showing 17 changed files with 188 additions and 638 deletions.
3 changes: 1 addition & 2 deletions chrome/browser/android/signin/signin_metrics_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,5 @@ static void JNI_SigninMetricsUtils_LogSigninUserActionForAccessPoint(
JNIEnv* env,
jint access_point) {
signin_metrics::RecordSigninUserActionForAccessPoint(
static_cast<signin_metrics::AccessPoint>(access_point),
signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO);
static_cast<signin_metrics::AccessPoint>(access_point));
}
3 changes: 1 addition & 2 deletions chrome/browser/signin/dice_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ void DiceTabHelper::InitializeSigninFlow(
if (reason == signin_metrics::Reason::kSigninPrimaryAccount) {
sync_signin_flow_status_ = SyncSigninFlowStatus::kStarted;
signin_metrics::LogSigninAccessPointStarted(access_point, promo_action);
signin_metrics::RecordSigninUserActionForAccessPoint(access_point,
promo_action);
signin_metrics::RecordSigninUserActionForAccessPoint(access_point);
base::RecordAction(base::UserMetricsAction("Signin_SigninPage_Loading"));
}
}
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/signin/signin_ui_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,7 @@ void EnableSyncFromMultiAccountPromo(Profile* profile,

signin_metrics::LogSigninAccessPointStarted(access_point,
existing_account_promo_action);
signin_metrics::RecordSigninUserActionForAccessPoint(
access_point, existing_account_promo_action);
signin_metrics::RecordSigninUserActionForAccessPoint(access_point);
GetSigninUiDelegate()->ShowTurnSyncOnUI(
profile, access_point, existing_account_promo_action,
signin_metrics::Reason::kSigninPrimaryAccount, account.account_id,
Expand Down
24 changes: 0 additions & 24 deletions chrome/browser/signin/signin_ui_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,6 @@ TEST_F(SigninUiUtilTest, EnableSyncWithExistingAccount) {
ExpectOneSigninStartedHistograms(histogram_tester, expected_promo_action);
EXPECT_EQ(1, user_action_tester.GetActionCount(
"Signin_Signin_FromBookmarkBubble"));
if (is_default_promo_account) {
EXPECT_EQ(1, user_action_tester.GetActionCount(
"Signin_SigninWithDefault_FromBookmarkBubble"));
} else {
EXPECT_EQ(1, user_action_tester.GetActionCount(
"Signin_SigninNotDefault_FromBookmarkBubble"));
}
}
}

Expand Down Expand Up @@ -319,14 +312,6 @@ TEST_F(SigninUiUtilTest, EnableSyncWithAccountThatNeedsReauth) {
EXPECT_EQ(1, user_action_tester.GetActionCount(
"Signin_Signin_FromBookmarkBubble"));

if (is_default_promo_account) {
EXPECT_EQ(1, user_action_tester.GetActionCount(
"Signin_SigninWithDefault_FromBookmarkBubble"));
} else {
EXPECT_EQ(1, user_action_tester.GetActionCount(
"Signin_SigninNotDefault_FromBookmarkBubble"));
}

// Verify that the active tab has the correct DICE sign-in URL.
TabStripModel* tab_strip = browser()->tab_strip_model();
content::WebContents* active_contents = tab_strip->GetActiveWebContents();
Expand Down Expand Up @@ -355,9 +340,6 @@ TEST_F(SigninUiUtilTest, EnableSyncForNewAccountWithNoTab) {
PROMO_ACTION_NEW_ACCOUNT_NO_EXISTING_ACCOUNT);
EXPECT_EQ(
1, user_action_tester.GetActionCount("Signin_Signin_FromBookmarkBubble"));
EXPECT_EQ(1,
user_action_tester.GetActionCount(
"Signin_SigninNewAccountNoExistingAccount_FromBookmarkBubble"));

// Verify that the active tab has the correct DICE sign-in URL.
content::WebContents* active_contents =
Expand Down Expand Up @@ -387,9 +369,6 @@ TEST_F(SigninUiUtilTest, EnableSyncForNewAccountWithNoTabWithExisting) {
signin_metrics::PromoAction::PROMO_ACTION_NEW_ACCOUNT_EXISTING_ACCOUNT);
EXPECT_EQ(
1, user_action_tester.GetActionCount("Signin_Signin_FromBookmarkBubble"));
EXPECT_EQ(1,
user_action_tester.GetActionCount(
"Signin_SigninNewAccountExistingAccount_FromBookmarkBubble"));
}

TEST_F(SigninUiUtilTest, EnableSyncForNewAccountWithOneTab) {
Expand All @@ -408,9 +387,6 @@ TEST_F(SigninUiUtilTest, EnableSyncForNewAccountWithOneTab) {
PROMO_ACTION_NEW_ACCOUNT_NO_EXISTING_ACCOUNT);
EXPECT_EQ(
1, user_action_tester.GetActionCount("Signin_Signin_FromBookmarkBubble"));
EXPECT_EQ(1,
user_action_tester.GetActionCount(
"Signin_SigninNewAccountNoExistingAccount_FromBookmarkBubble"));

// Verify that the active tab has the correct DICE sign-in URL.
content::WebContents* active_contents =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@ public interface OnDismissListener {
private @Nullable ImpressionTracker mImpressionTracker;
private final @AccessPoint int mAccessPoint;
private final String mImpressionUserActionName;
private final String mSigninWithDefaultUserActionName;
private final String mSigninNotDefaultUserActionName;
private final String mSigninNewAccountUserActionName;
private final @Nullable String mSyncPromoDismissedPreferenceTracker;
private final @StringRes int mTitleStringId;
private final @StringRes int mDescriptionStringId;
Expand Down Expand Up @@ -273,12 +270,6 @@ public SigninPromoController(
switch (mAccessPoint) {
case SigninAccessPoint.BOOKMARK_MANAGER:
mImpressionUserActionName = "Signin_Impression_FromBookmarkManager";
mSigninWithDefaultUserActionName = "Signin_SigninWithDefault_FromBookmarkManager";
mSigninNotDefaultUserActionName = "Signin_SigninNotDefault_FromBookmarkManager";
// On Android, the promo does not have a button to add and account when there is
// already an account on the device. Always use the NoExistingAccount variant.
mSigninNewAccountUserActionName =
"Signin_SigninNewAccountNoExistingAccount_FromBookmarkManager";
mSyncPromoDismissedPreferenceTracker =
ChromePreferenceKeys.SIGNIN_PROMO_BOOKMARKS_DECLINED;
mTitleStringId = R.string.sync_promo_title_bookmarks;
Expand All @@ -295,14 +286,6 @@ public SigninPromoController(
break;
case SigninAccessPoint.NTP_CONTENT_SUGGESTIONS:
mImpressionUserActionName = "Signin_Impression_FromNTPContentSuggestions";
mSigninWithDefaultUserActionName =
"Signin_SigninWithDefault_FromNTPContentSuggestions";
mSigninNotDefaultUserActionName =
"Signin_SigninNotDefault_FromNTPContentSuggestions";
// On Android, the promo does not have a button to add and account when there is
// already an account on the device. Always use the NoExistingAccount variant.
mSigninNewAccountUserActionName =
"Signin_SigninNewAccountNoExistingAccount_FromNTPContentSuggestions";
mSyncPromoDismissedPreferenceTracker =
ChromePreferenceKeys.SIGNIN_PROMO_NTP_PROMO_DISMISSED;
if (ChromeFeatureList.isEnabled(
Expand All @@ -324,12 +307,6 @@ public SigninPromoController(
break;
case SigninAccessPoint.RECENT_TABS:
mImpressionUserActionName = "Signin_Impression_FromRecentTabs";
mSigninWithDefaultUserActionName = "Signin_SigninWithDefault_FromRecentTabs";
mSigninNotDefaultUserActionName = "Signin_SigninNotDefault_FromRecentTabs";
// On Android, the promo does not have a button to add and account when there is
// already an account on the device. Always use the NoExistingAccount variant.
mSigninNewAccountUserActionName =
"Signin_SigninNewAccountNoExistingAccount_FromRecentTabs";
mSyncPromoDismissedPreferenceTracker = null;
if (ChromeFeatureList.isEnabled(
ChromeFeatureList.SYNC_ANDROID_PROMOS_WITH_ALTERNATIVE_TITLE)) {
Expand All @@ -348,12 +325,6 @@ public SigninPromoController(
break;
case SigninAccessPoint.SETTINGS:
mImpressionUserActionName = "Signin_Impression_FromSettings";
mSigninWithDefaultUserActionName = "Signin_SigninWithDefault_FromSettings";
mSigninNotDefaultUserActionName = "Signin_SigninNotDefault_FromSettings";
// On Android, the promo does not have a button to add and account when there is
// already an account on the device. Always use the NoExistingAccount variant.
mSigninNewAccountUserActionName =
"Signin_SigninNewAccountNoExistingAccount_FromSettings";
mSyncPromoDismissedPreferenceTracker =
ChromePreferenceKeys.SIGNIN_PROMO_SETTINGS_PERSONALIZED_DISMISSED;
if (ChromeFeatureList.isEnabled(
Expand Down Expand Up @@ -542,20 +513,17 @@ private void setupHotState(PersonalizedSigninPromoView view) {

private void signinWithNewAccount(Context context) {
recordShowCountHistogram(UserAction.CONTINUED);
RecordUserAction.record(mSigninNewAccountUserActionName);
mSyncConsentActivityLauncher.launchActivityForPromoAddAccountFlow(context, mAccessPoint);
}

private void signinWithDefaultAccount(Context context) {
recordShowCountHistogram(UserAction.CONTINUED);
RecordUserAction.record(mSigninWithDefaultUserActionName);
mSyncConsentActivityLauncher.launchActivityForPromoDefaultFlow(
context, mAccessPoint, mProfileData.getAccountEmail());
}

private void signinWithNotDefaultAccount(Context context) {
recordShowCountHistogram(UserAction.CONTINUED);
RecordUserAction.record(mSigninNotDefaultUserActionName);
mSyncConsentActivityLauncher.launchActivityForPromoChooseAccountFlow(
context, mAccessPoint, mProfileData.getAccountEmail());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,7 @@ void SigninInterceptFirstRunExperienceDialog::DoTurnOnSync() {
const signin_metrics::PromoAction promo_action =
signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO;
signin_metrics::LogSigninAccessPointStarted(access_point, promo_action);
signin_metrics::RecordSigninUserActionForAccessPoint(access_point,
promo_action);
signin_metrics::RecordSigninUserActionForAccessPoint(access_point);

// TurnSyncOnHelper deletes itself once done.
new TurnSyncOnHelper(browser_->profile(), access_point, promo_action,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,7 @@ void ProfilePickerDiceSignInProvider::OnProfileCreated(
// by the instance of DiceTurnSyncOnHelper constructed later on in
// ProfilePickerSignedInFlowController).
signin_metrics::RecordSigninUserActionForAccessPoint(
signin_metrics::AccessPoint::ACCESS_POINT_USER_MANAGER,
signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO);
signin_metrics::AccessPoint::ACCESS_POINT_USER_MANAGER);
signin_metrics::LogSigninAccessPointStarted(
signin_metrics::AccessPoint::ACCESS_POINT_USER_MANAGER,
signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO);
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/ui/webui/signin/inline_login_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,7 @@ void InlineLoginHandler::ContinueHandleInitializeMessage() {
signin_metrics::LogSigninAccessPointStarted(
access_point,
signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO);
signin_metrics::RecordSigninUserActionForAccessPoint(
access_point,
signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO);
signin_metrics::RecordSigninUserActionForAccessPoint(access_point);
base::RecordAction(base::UserMetricsAction("Signin_SigninPage_Loading"));
params.Set("isLoginPrimaryAccount", true);
}
Expand Down
Loading

0 comments on commit 30501ff

Please sign in to comment.