[FRE] Add MobileFre.Progress histograms

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

Index: chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
index da70c9e365de017276a632de783be584799e1f8d..a5c82a276bd046c6fdd349afcc7f9d08a63cbf99 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
@@ -60,11 +60,6 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
 
     // UMA constants.
     private static final String UMA_SIGNIN_CHOICE = "MobileFre.SignInChoice";
-    private static final String UMA_SIGNIN_CHOICE_ENTRY_MAIN_INTENT = ".MainIntent";
-    private static final String UMA_SIGNIN_CHOICE_ENTRY_VIEW_INTENT = ".ViewIntent";
-    private static final String UMA_SIGNIN_CHOICE_ZERO_ACCOUNTS = ".ZeroAccounts";
-    private static final String UMA_SIGNIN_CHOICE_ONE_ACCOUNT = ".OneAccount";
-    private static final String UMA_SIGNIN_CHOICE_MANY_ACCOUNTS = ".ManyAccounts";
     private static final int SIGNIN_SETTINGS_DEFAULT_ACCOUNT = 0;
     private static final int SIGNIN_SETTINGS_ANOTHER_ACCOUNT = 1;
     private static final int SIGNIN_ACCEPT_DEFAULT_ACCOUNT = 2;
@@ -72,6 +67,18 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
     private static final int SIGNIN_NO_THANKS = 4;
     private static final int SIGNIN_OPTION_COUNT = 5;
 
+    private static final String FRE_ENTRY_MAIN_INTENT = ".MainIntent";
+    private static final String FRE_ENTRY_VIEW_INTENT = ".ViewIntent";
+
+    private static final String UMA_FRE_PROGRESS = "MobileFre.Progress";
+    private static final int FRE_PROGRESS_STARTED = 0;
+    private static final int FRE_PROGRESS_WELCOME_SHOWN = 1;
+    private static final int FRE_PROGRESS_DATA_SAVER_SHOWN = 2;
+    private static final int FRE_PROGRESS_SIGNIN_SHOWN = 3;
+    private static final int FRE_PROGRESS_COMPLETED_SIGNED_IN = 4;
+    private static final int FRE_PROGRESS_COMPLETED_NOT_SIGNED_IN = 5;
+    private static final int FRE_PROGRESS_TERMINATOR = 128;

[Bernhard Bauer, 2016/08/24 09:27:23]

Why 128? It's not only kind of arbitrary, but using an enum histogram will
allocate every single bucket up to the maximum value, so you'd be allocating way
more buckets than necessary.

[gogerald1, 2016/08/24 14:47:44]

Done. yes, enum histogram is not sparse,

+
     @VisibleForTesting
     static FirstRunGlue sGlue = new FirstRunGlueImpl();
 
@@ -89,6 +96,9 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
 
     private List<Callable<FirstRunPage>> mPages;
 
+    private List<Integer> mFreProgressSates;

[Bernhard Bauer, 2016/08/24 09:27:23]

Nit: ...States

[gogerald1, 2016/08/24 14:47:44]

Done.

+    private String mFreProgressEntryType;
+
     /**
      * The pager adapter, which provides the pages to the view pager widget.
      */
@@ -99,18 +109,24 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
      */
     private void createPageSequence() {
         mPages = new ArrayList<Callable<FirstRunPage>>();
+        mFreProgressSates = new ArrayList<Integer>();
 
         // An optional welcome page.
-        if (mShowWelcomePage) mPages.add(pageOf(ToSAndUMAFirstRunFragment.class));
+        if (mShowWelcomePage) {
+            mPages.add(pageOf(ToSAndUMAFirstRunFragment.class));
+            mFreProgressSates.add(FRE_PROGRESS_WELCOME_SHOWN);
+        }
 
         // An optional Data Saver page.
         if (mFreProperties.getBoolean(SHOW_DATA_REDUCTION_PAGE)) {
             mPages.add(pageOf(DataReductionProxyFirstRunFragment.class));
+            mFreProgressSates.add(FRE_PROGRESS_DATA_SAVER_SHOWN);
         }
 
         // An optional sign-in page.
         if (mFreProperties.getBoolean(SHOW_SIGNIN_PAGE)) {
             mPages.add(pageOf(AccountFirstRunFragment.class));
+            mFreProgressSates.add(FRE_PROGRESS_SIGNIN_SHOWN);
         }
     }
 
@@ -150,6 +166,10 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
         mPager.setId(R.id.fre_pager);
         setContentView(mPager);
 
+        mFreProgressEntryType = mFreProperties.getBoolean(FirstRunActivity.COMING_FROM_CHROME_ICON)
+                ? FRE_ENTRY_MAIN_INTENT
+                : FRE_ENTRY_VIEW_INTENT;
+
         mProfileDataCache = new ProfileDataCache(FirstRunActivity.this, null);
         mProfileDataCache.setProfile(Profile.getLastUsedProfile());
         new FirstRunFlowSequencer(this, mFreProperties, isMetricsReportingOptIn()) {
@@ -180,9 +200,15 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
                 stopProgressionIfNotAcceptedTermsOfService();
                 mPager.setAdapter(mPagerAdapter);
 
+                RecordHistogram.recordEnumeratedHistogram(UMA_FRE_PROGRESS + mFreProgressEntryType,
+                        mFreProgressSates.get(0), FRE_PROGRESS_TERMINATOR);
+
                 skipPagesIfNecessary();
             }
         }.start();
+
+        RecordHistogram.recordEnumeratedHistogram(UMA_FRE_PROGRESS + mFreProgressEntryType,

[Bernhard Bauer, 2016/08/24 09:27:23]

Would it be worth to extract this to a shared method?

[gogerald1, 2016/08/24 14:47:44]

Done. Might worth

+                FRE_PROGRESS_STARTED, FRE_PROGRESS_TERMINATOR);
     }
 
     @Override
@@ -284,19 +310,11 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
             RecordHistogram.recordEnumeratedHistogram(
                     UMA_SIGNIN_CHOICE, choice, SIGNIN_OPTION_COUNT);
 
-            String entryType = mFreProperties.getBoolean(FirstRunActivity.COMING_FROM_CHROME_ICON)
-                    ? UMA_SIGNIN_CHOICE_ENTRY_MAIN_INTENT : UMA_SIGNIN_CHOICE_ENTRY_VIEW_INTENT;
-            int numAccounts = sGlue.numberOfAccounts(getApplicationContext());
-            String numAccountsString;
-            if (numAccounts == 0) {
-                numAccountsString = UMA_SIGNIN_CHOICE_ZERO_ACCOUNTS;
-            } else if (numAccounts == 1) {
-                numAccountsString = UMA_SIGNIN_CHOICE_ONE_ACCOUNT;
-            } else {
-                numAccountsString = UMA_SIGNIN_CHOICE_MANY_ACCOUNTS;
-            }
-            RecordHistogram.recordEnumeratedHistogram(
-                    UMA_SIGNIN_CHOICE + entryType + numAccountsString, choice, SIGNIN_OPTION_COUNT);
+            RecordHistogram.recordEnumeratedHistogram(UMA_FRE_PROGRESS + mFreProgressEntryType,
+                    FRE_PROGRESS_COMPLETED_SIGNED_IN, FRE_PROGRESS_TERMINATOR);
+        } else {
+            RecordHistogram.recordEnumeratedHistogram(UMA_FRE_PROGRESS + mFreProgressEntryType,
+                    FRE_PROGRESS_COMPLETED_NOT_SIGNED_IN, FRE_PROGRESS_TERMINATOR);
         }
 
         mFreProperties.putString(RESULT_SIGNIN_ACCOUNT_NAME, mResultSignInAccountName);
@@ -398,6 +416,8 @@ public class FirstRunActivity extends AppCompatActivity implements FirstRunPageD
             return false;
         }
         mPager.setCurrentItem(position, false);
+        RecordHistogram.recordEnumeratedHistogram(UMA_FRE_PROGRESS + mFreProgressEntryType,
+                mFreProgressSates.get(position), FRE_PROGRESS_TERMINATOR);
         return true;
     }