[Signin][Android] Move logSigninStartUserAction to SigninMetricsUtils

This CL moves logSigninStartUserAction to SigninMetricsUtils to group
the signin metrics logging methods in one place.

Bug: 1163476
Change-Id: I515a2c04f4bac3541a0a3bcee4097c3dabab95cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611715Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Tanmoy Mollik <triploblastic@chromium.org>
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844008}

chrome/android/java/src/org/chromium/chrome/browser/firstrun/SigninFirstRunFragment.java

@@ -51,8 +51,8 @@ public class SigninFirstRunFragment extends SigninFragmentBase implements FirstR
         RecordHistogram.recordCountHistogram(
                 "Signin.AndroidDeviceAccountsNumberWhenEnteringFRE", Math.min(numAccounts, 2));
         RecordUserAction.record("MobileFre.SignInShown");
-        RecordUserAction.record("Signin_Signin_FromStartPage");
         SigninMetricsUtils.logSigninStartAccessPoint(SigninAccessPoint.START_PAGE);
+        SigninMetricsUtils.logSigninUserActionForAccessPoint(SigninAccessPoint.START_PAGE);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/signin/SigninFragment.java

@@ -12,7 +12,6 @@ import androidx.annotation.IntDef;
 import androidx.annotation.Nullable;
 
 import org.chromium.base.metrics.RecordHistogram;
-import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.SyncFirstSetupCompleteSource;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
@@ -120,8 +119,8 @@ public class SigninFragment extends SigninFragmentBase {
                 getSigninArguments().getInt(ARGUMENT_PERSONALIZED_PROMO_ACTION, PromoAction.NONE);
 
         SigninMetricsUtils.logSigninStartAccessPoint(mSigninAccessPoint);
+        SigninMetricsUtils.logSigninUserActionForAccessPoint(mSigninAccessPoint);
         recordSigninStartedHistogramAccountInfo();
-        recordSigninStartedUserAction();
     }
 
     @Override
@@ -241,29 +240,4 @@ public class SigninFragment extends SigninFragmentBase {
         RecordHistogram.recordEnumeratedHistogram(
                 histogram, mSigninAccessPoint, SigninAccessPoint.MAX);
     }
-
-    private void recordSigninStartedUserAction() {
-        switch (mSigninAccessPoint) {
-            case SigninAccessPoint.AUTOFILL_DROPDOWN:
-                RecordUserAction.record("Signin_Signin_FromAutofillDropdown");
-                break;
-            case SigninAccessPoint.BOOKMARK_MANAGER:
-                RecordUserAction.record("Signin_Signin_FromBookmarkManager");
-                break;
-            case SigninAccessPoint.RECENT_TABS:
-                RecordUserAction.record("Signin_Signin_FromRecentTabs");
-                break;
-            case SigninAccessPoint.SETTINGS:
-                RecordUserAction.record("Signin_Signin_FromSettings");
-                break;
-            case SigninAccessPoint.SIGNIN_PROMO:
-                RecordUserAction.record("Signin_Signin_FromSigninPromo");
-                break;
-            case SigninAccessPoint.NTP_CONTENT_SUGGESTIONS:
-                RecordUserAction.record("Signin_Signin_FromNTPContentSuggestions");
-                break;
-            default:
-                assert false : "Invalid access point.";
-        }
-    }
 }

chrome/browser/android/signin/signin_metrics_utils.cc

@@ -5,6 +5,7 @@
 #include "base/android/jni_android.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/signin/services/android/jni_headers/SigninMetricsUtils_jni.h"
+#include "components/signin/public/base/signin_metrics.h"
 
 static void JNI_SigninMetricsUtils_LogProfileAccountManagementMenu(
     JNIEnv* env,
@@ -14,3 +15,11 @@ static void JNI_SigninMetricsUtils_LogProfileAccountManagementMenu(
       static_cast<ProfileMetrics::ProfileAndroidAccountManagementMenu>(metric),
       static_cast<signin::GAIAServiceType>(gaia_service_type));
 }
+
+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);
+}

chrome/browser/signin/services/android/java/src/org/chromium/chrome/browser/signin/services/SigninMetricsUtils.java

@@ -54,10 +54,18 @@ public class SigninMetricsUtils {
                 "Signin.SigninStartedAccessPoint", accessPoint, SigninAccessPoint.MAX);
     }
 
+    /**
+     * Logs signin user action for a given {@link SigninAccessPoint}.
+     */
+    public static void logSigninUserActionForAccessPoint(@SigninAccessPoint int accessPoint) {
+        SigninMetricsUtilsJni.get().logSigninUserActionForAccessPoint(accessPoint);
+    }
+
     @VisibleForTesting
     @NativeMethods
     public interface Natives {
         void logProfileAccountManagementMenu(int metric, int gaiaServiceType);
+        void logSigninUserActionForAccessPoint(int accessPoint);
     }
 
     private SigninMetricsUtils() {}

components/signin/public/base/signin_metrics.cc

@@ -133,11 +133,12 @@ void RecordSigninUserActionForAccessPoint(AccessPoint access_point) {
     case AccessPoint::ACCESS_POINT_FORCED_SIGNIN:
     case AccessPoint::ACCESS_POINT_ACCOUNT_RENAMED:
     case AccessPoint::ACCESS_POINT_WEB_SIGNIN:
-    case AccessPoint::ACCESS_POINT_SAFETY_CHECK:
       NOTREACHED() << "Access point " << static_cast<int>(access_point)
-                   << " is only used on Android, where"
-                   << " RecordSigninUserActionForAccessPoint is not used"
-                   << " for logging user actions.";
+                   << " is not supposed to log signin user actions.";
+      break;
+    case AccessPoint::ACCESS_POINT_SAFETY_CHECK:
+      VLOG(1) << "Signin_Signin_From* user action is not recorded "
+              << "for access point " << static_cast<int>(access_point);
       break;
     case AccessPoint::ACCESS_POINT_MAX:
       NOTREACHED();