[Android] Histogram for Android restore

chrome/android/java/src/org/chromium/chrome/browser/ChromeBackupAgent.java

 // Copyright 2016 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.
 
 package org.chromium.chrome.browser;
 
 import android.app.backup.BackupAgent;
 import android.app.backup.BackupDataInput;
 import android.app.backup.BackupDataOutput;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.os.ParcelFileDescriptor;
+import android.support.annotation.IntDef;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.PathUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.ProcessInitException;
+import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.firstrun.FirstRunGlueImpl;
 import org.chromium.chrome.browser.firstrun.FirstRunSignInProcessor;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.init.AsyncInitTaskRunner;
 import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
 import org.chromium.chrome.browser.preferences.privacy.PrivacyPreferencesManager;
 import org.chromium.components.signin.AccountManagerHelper;
 import org.chromium.components.signin.ChromeSigninController;
 
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 /**
  * Backup agent for Chrome, using Android key/value backup.
  */
 
 public class ChromeBackupAgent extends BackupAgent {
+    /**
+     *

[Bernhard Bauer, 2017/03/16 11:36:36]

:)

[aberent, 2017/03/16 12:16:50]

Done.

+     */
     private static final String ANDROID_DEFAULT_PREFIX = "AndroidDefault.";
     private static final String NATIVE_PREF_PREFIX = "native.";
 
     private static final String TAG = "ChromeBackupAgent";
 
+    @VisibleForTesting
+    static final String HISTOGRAM_ANDROID_RESTORE_RESULT = "Android.RestoreResult";
+    // Restore status is used to pass the result of any restore to Chrome's first run, so that

[Bernhard Bauer, 2017/03/16 11:36:36]

Can you leave empty lines before comments?

[aberent, 2017/03/16 12:16:50]

Done here, and where I think it helps readability elsewhere in the file (most
but not every case).

Note, however, that git cl format is taking out blank lines at the start of
methods, so I can't do this everywhere.

[Bernhard Bauer, 2017/03/16 14:34:01]

That's fine, I wouldn't add them there either (or at the beginning of a block in
general). 

+    // it can be recorded as a histogram.
+    @Retention(RetentionPolicy.SOURCE)
+    @IntDef({NO_RESTORE, RESTORE_COMPLETED, RESTORE_AFTER_FIRST_RUN, BROWSER_STARTUP_FAILED,
+            NOT_SIGNED_IN, RESTORE_STATUS_RECORDED})
+    public @interface RestoreStatus {}
+
+    // Values must match those in histogram.xml AndroidRestoreResult.
+    static final int NO_RESTORE = 0;
+    static final int RESTORE_COMPLETED = 1;
+    static final int RESTORE_AFTER_FIRST_RUN = 2;
+    static final int BROWSER_STARTUP_FAILED = 3;
+    static final int NOT_SIGNED_IN = 4;
+    static final int RESTORE_HISTOGRAM_BOUNDARY = 5;
+    // Set RESTORE_STATUS_RECORDED when the histogram has been recorded; so that it is only recorded
+    // once.
+    public static final int RESTORE_STATUS_RECORDED = 5;
+
+    private static final String RESTORE_STATUS = "Android_restore_status";

[Bernhard Bauer, 2017/03/16 11:36:36]

Nit: The combination of uppercase initial and underscores looks a bit weird :)
Lowercase everything?

[aberent, 2017/03/16 12:16:50]

Done.

+
     // Lists of preferences that should be restored unchanged.
 
     static final String[] BACKUP_ANDROID_BOOL_PREFS = {
             FirstRunGlueImpl.CACHED_TOS_ACCEPTED_PREF,
             FirstRunStatus.FIRST_RUN_FLOW_COMPLETE,
             FirstRunStatus.LIGHTWEIGHT_FIRST_RUN_FLOW_COMPLETE,
             FirstRunSignInProcessor.FIRST_RUN_FLOW_SIGNIN_SETUP,
             PrivacyPreferencesManager.PREF_METRICS_REPORTING,
     };
 
@@ skipping 149 matching lines @@
 
     @Override
     public void onRestore(BackupDataInput data, int appVersionCode, ParcelFileDescriptor newState)
             throws IOException {
         // TODO(aberent) Check that this is not running on the UI thread. Doing so, however, makes
         // testing difficult since the test code runs on the UI thread.
 
         // Check that the user hasn't already seen FRE (not sure if this can ever happen, but if it
         // does then restoring the backup will overwrite the user's choices).
         SharedPreferences sharedPrefs = ContextUtils.getAppSharedPreferences();
-        if (sharedPrefs.getBoolean(FirstRunStatus.FIRST_RUN_FLOW_COMPLETE, false)
-                || sharedPrefs.getBoolean(
-                           FirstRunStatus.LIGHTWEIGHT_FIRST_RUN_FLOW_COMPLETE, false)) {
+        if (FirstRunStatus.getFirstRunFlowComplete()
+                || FirstRunStatus.getLightweightFirstRunFlowComplete()) {
+            setRestoreStatus(RESTORE_AFTER_FIRST_RUN);
             Log.w(TAG, "Restore attempted after first run");
             return;
         }
 
         final ArrayList<String> backupNames = new ArrayList<>();
         final ArrayList<byte[]> backupValues = new ArrayList<>();
 
         String restoredUserName = null;
         while (data.readNextHeader()) {
             String key = data.getKey();
@@ skipping 40 matching lines @@
         boolean browserStarted =
                 ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() {
                     @Override
                     public Boolean call() {
                         // Start the browser if necessary.
                         return initializeBrowser(backupAgent);
                     }
                 });
         if (!browserStarted) {
             // Something went wrong starting Chrome, skip the restore.
+            setRestoreStatus(BROWSER_STARTUP_FAILED);
             return;
         }
         // If the user hasn't signed in, or can't sign in, then don't restore anything.
         if (restoredUserName == null || !accountExistsOnDevice(restoredUserName)) {
+            setRestoreStatus(NOT_SIGNED_IN);
             Log.i(TAG, "Chrome was not signed in with a known account name, not restoring");
             return;
         }
         // Restore the native preferences on the UI thread
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
             @Override
             public void run() {
                 ArrayList<String> nativeBackupNames = new ArrayList<>();
                 boolean[] nativeBackupValues = new boolean[backupNames.size()];
                 int count = 0;
@@ skipping 27 matching lines @@
 
         // Because FirstRunSignInProcessor.FIRST_RUN_FLOW_SIGNIN_COMPLETE is not restored Chrome
         // will sign in the user on first run to the account in FIRST_RUN_FLOW_SIGNIN_ACCOUNT_NAME
         // if any. If the rest of FRE has been completed this will happen silently.
         editor.putString(
                 FirstRunSignInProcessor.FIRST_RUN_FLOW_SIGNIN_ACCOUNT_NAME, restoredUserName);
         editor.apply();
 
         // The silent first run will change things, so there is no point in trying to prevent
         // additional backups at this stage. Don't write anything to |newState|.
+        setRestoreStatus(RESTORE_COMPLETED);
         Log.i(TAG, "Restore complete");
     }
 
     @VisibleForTesting
     AsyncInitTaskRunner createAsyncInitTaskRunner(final CountDownLatch latch) {
         return new AsyncInitTaskRunner() {
 
             @Override
             protected void onSuccess() {
                 latch.countDown();
             }
 
             @Override
             protected void onFailure() {
                 // Ignore failure. Problems with the variation seed can be ignored, and other
                 // problems will either recover or be repeated when Chrome is started synchronously.
                 latch.countDown();
             }
         };
     }
 
+    /**
+     * Get the saved result of any restore that may have happened.
+     *
+     * @return the restore status, a RestoreStatus value.
+     */
+    @VisibleForTesting
+    @RestoreStatus
+    static int getRestoreStatus() {
+        return ContextUtils.getAppSharedPreferences().getInt(RESTORE_STATUS, NO_RESTORE);
+    }
+
+    /**
+     * Set the restore status

[Bernhard Bauer, 2017/03/16 11:36:36]

Nit: I don't think this comments adds any value.

[aberent, 2017/03/16 12:16:50]

Updated comment.

+     *
+     * @param status the status.
+     */
+    @VisibleForTesting
+    static void setRestoreStatus(@RestoreStatus int status) {
+        ContextUtils.getAppSharedPreferences().edit().putInt(RESTORE_STATUS, status).apply();
+    }
+
+    /**
+     * Record the restore histogram. To be called from Chrome itself once it is running.
+     */
+    static void recordRestoreHistogram() {
+        int restoreStatus = getRestoreStatus();
+        // Ensure restore status is only recorded once
+        if (restoreStatus != RESTORE_STATUS_RECORDED) {
+            RecordHistogram.recordEnumeratedHistogram(
+                    HISTOGRAM_ANDROID_RESTORE_RESULT, restoreStatus, RESTORE_HISTOGRAM_BOUNDARY);
+            setRestoreStatus(RESTORE_STATUS_RECORDED);
+        }
+    }
+
     @VisibleForTesting
     protected native String[] nativeGetBoolBackupNames();
 
     @VisibleForTesting
     protected native boolean[] nativeGetBoolBackupValues();
 
     @VisibleForTesting
     protected native void nativeSetBoolBackupPrefs(String[] name, boolean[] value);
 }