[Android Crash Reporting] Allow uploading minidumps via the JobScheduler

chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadService.java

 // Copyright 2015 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.crash;
 
+import android.annotation.SuppressLint;
 import android.app.IntentService;
+import android.app.job.JobInfo;
+import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
+import android.os.Build;
+import android.os.PersistableBundle;
 import android.support.annotation.StringDef;
 
 import org.chromium.base.Log;
 import org.chromium.base.StreamUtil;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.metrics.RecordHistogram;
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
 import org.chromium.chrome.browser.preferences.privacy.PrivacyPreferencesManager;
 import org.chromium.components.minidump_uploader.CrashFileManager;
 import org.chromium.components.minidump_uploader.MinidumpUploadCallable;
+import org.chromium.components.minidump_uploader.MinidumpUploadJobService;
 import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager;
 
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
 
 /**
  * Service that is responsible for uploading crash minidumps to the Google crash server.
  */
 public class MinidumpUploadService extends IntentService {
     private static final String TAG = "MinidmpUploadService";
 
     // Intent actions
     @VisibleForTesting
     static final String ACTION_UPLOAD = "com.google.android.apps.chrome.crash.ACTION_UPLOAD";
 
     // Intent bundle keys
     @VisibleForTesting
     static final String FILE_TO_UPLOAD_KEY = "minidump_file";
     static final String UPLOAD_LOG_KEY = "upload_log";
 
     /**
+     * The job id for uploading minidumps. For more info on this constant, see
+     * https://developer.android.com/reference/android/app/job/JobInfo.Builder.html#JobInfo.Builder(int,%20android.content.ComponentName)
+     */
+    private static final int MINIDUMP_UPLOADING_JOB_ID = 142;

[gsennton, 2017/03/14 18:17:28]

I just realized it might be good to look for the IDs we are already using ;).
It looks like the different values for JobIds in Chrome are being set in
components/background_task_scheduler/android/java/src/org/chromium/components/background_task_scheduler/TaskIds.java

I think we should at least add a comment there describing which IDs we are using
for WebView+Chrome Minidump uploading.
(in the long run we might want to just use all the utility in
components/background_task_scheduler/ to implement our job-scheduler).

[Ilya Sherman, 2017/03/15 02:13:34]

Good call!  Moved the constants into that component.

And, yeah, I think after merging things to M58 I'll update the code to use that
component.  Looks much nicer than maintaining two similar-but-not-the-same code
paths =D

[gsennton, 2017/03/15 12:57:43]

Indeed, I wonder if there will be anything stopping us from using that code-path
for WebView as well. Will have to take a look ;)

+
+    /**
      * The number of times we will try to upload a crash.
      */
     public static final int MAX_TRIES_ALLOWED = 3;
 
     /**
      * Histogram related constants.
      */
     private static final String HISTOGRAM_NAME_PREFIX = "Tab.AndroidCrashUpload_";
     private static final int HISTOGRAM_MAX = 2;
     private static final int FAILURE = 0;
     private static final int SUCCESS = 1;
 
     @StringDef({BROWSER, RENDERER, GPU, OTHER})
     public @interface ProcessType {}
     static final String BROWSER = "Browser";
     static final String RENDERER = "Renderer";
     static final String GPU = "GPU";
     static final String OTHER = "Other";
 
     static final String[] TYPES = {BROWSER, RENDERER, GPU, OTHER};
 
     public MinidumpUploadService() {
         super(TAG);
         setIntentRedelivery(true);
     }
 
     /**
+     * @return Whether to use the JobSchduler API to upload crash reports, rather than directly
+     *     creating a service for uploading.
+     */
+    public static boolean shouldUseJobSchedulerForUploads() {
+        // The JobScheduler API is only available as of Android L.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) {
+            return false;
+        }
+
+        return ChromeFeatureList.isEnabled(
+                ChromeFeatureList.UPLOAD_CRASH_REPORTS_USING_JOB_SCHEDULER);
+    }
+
+    /**
+     * Schedules uploading of all pending minidumps, using the JobScheduler API.
+     */
+    @SuppressLint("NewApi")
+    public static void scheduleUploadJob(Context context) {
+        assert shouldUseJobSchedulerForUploads();
+
+        CrashReportingPermissionManager permissionManager = PrivacyPreferencesManager.getInstance();
+        PersistableBundle permissions = new PersistableBundle();
+        // Note: putBoolean is only available in API level 22, so massage the data into ints
+        // instead.
+        permissions.putInt(ChromeMinidumpUploaderDelegate.IS_CLIENT_IN_METRICS_SAMPLE,
+                permissionManager.isClientInMetricsSample() ? 1 : 0);
+        permissions.putInt(ChromeMinidumpUploaderDelegate.IS_CRASH_UPLOAD_DISABLED_BY_COMMAND_LINE,
+                permissionManager.isCrashUploadDisabledByCommandLine() ? 1 : 0);
+        permissions.putInt(ChromeMinidumpUploaderDelegate.IS_UPLOAD_ENABLED_FOR_TESTS,
+                permissionManager.isUploadEnabledForTests() ? 1 : 0);
+
+        JobInfo.Builder builder =
+                new JobInfo
+                        .Builder(MINIDUMP_UPLOADING_JOB_ID,
+                                new ComponentName(context, ChromeMinidumpUploadJobService.class))
+                        .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY)
+                        .setExtras(permissions);
+        MinidumpUploadJobService.scheduleUpload(context, builder);
+    }
+
+    /**
      * Stores the successes and failures from uploading crash to UMA,
      */
     public static void storeBreakpadUploadStatsInUma(ChromePreferenceManager pref) {
         for (String type : TYPES) {
             for (int success = pref.getCrashSuccessUploadCount(type); success > 0; success--) {
                 RecordHistogram.recordEnumeratedHistogram(
                         HISTOGRAM_NAME_PREFIX + type, SUCCESS, HISTOGRAM_MAX);
             }
             for (int fail = pref.getCrashFailureUploadCount(type); fail > 0; fail--) {
                 RecordHistogram.recordEnumeratedHistogram(
@@ skipping 18 matching lines @@
             Log.w(TAG, "Cannot upload crash data since minidump is absent.");
             return;
         }
         File minidumpFile = new File(minidumpFileName);
         if (!minidumpFile.isFile()) {
             Log.w(TAG, "Cannot upload crash data since specified minidump "
                     + minidumpFileName + " is not present.");
             return;
         }
         int tries = CrashFileManager.readAttemptNumber(minidumpFileName);
-        // -1 means no attempt number was read.
-        if (tries == -1) {
-            tries = 0;
-        }
 
         // Since we do not rename a file after reaching max number of tries,
         // files that have maxed out tries will NOT reach this.
         if (tries >= MAX_TRIES_ALLOWED || tries < 0) {
             // Reachable only if the file naming is incorrect by current standard.
             // Thus we log an error instead of recording failure to UMA.
             Log.e(TAG, "Giving up on trying to upload " + minidumpFileName
                     + " after failing to read a valid attempt number.");
             return;
         }
 
         String logfileName = intent.getStringExtra(UPLOAD_LOG_KEY);
         File logfile = new File(logfileName);
 
         // Try to upload minidump
         MinidumpUploadCallable minidumpUploadCallable =
                 createMinidumpUploadCallable(minidumpFile, logfile);
         @MinidumpUploadCallable.MinidumpUploadStatus int uploadStatus =
                 minidumpUploadCallable.call();
 
         if (uploadStatus == MinidumpUploadCallable.UPLOAD_SUCCESS) {
             // Only update UMA stats if an intended and successful upload.
-            incrementCrashSuccessUploadCount(getNewNameAfterSuccessfulUpload(minidumpFileName));
+            incrementCrashSuccessUploadCount(minidumpFileName);
         } else if (uploadStatus == MinidumpUploadCallable.UPLOAD_FAILURE) {
             // Unable to upload minidump. Incrementing try number and restarting.
 
             // Only create another attempt if we have successfully renamed
             // the file.
             String newName = CrashFileManager.tryIncrementAttemptNumber(minidumpFile);
             if (newName != null) {
                 if (++tries < MAX_TRIES_ALLOWED) {
                     // TODO(nyquist): Do this as an exponential backoff.
                     MinidumpUploadRetry.scheduleRetry(
@@ skipping 59 matching lines @@
             StreamUtil.closeQuietly(fileReader);
         }
         return OTHER;
     }
 
     /**
      * Increment the count of success/failure by 1 and distinguish between different types of
      * crashes by looking into the file.
      * @param fileName is the name of a minidump file that contains the type of crash.
      */
-    private void incrementCrashSuccessUploadCount(String fileName) {
+    public static void incrementCrashSuccessUploadCount(String fileName) {
         ChromePreferenceManager.getInstance().incrementCrashSuccessUploadCount(
-                getCrashType(fileName));
+                getCrashType(getNewNameAfterSuccessfulUpload(fileName)));

[gsennton, 2017/03/14 18:17:28]

Hiding this call in here seems like an implementation detail that could easily
be missed/overseen? (I might be wrong though ;))

[Ilya Sherman, 2017/03/15 02:13:34]

That's a very good point, and I actually had a bug for incrementing the failure
count.  The current state is awkward because MinidumpUploadCallable renames
successful crashes, but not failed ones.  I added thorough documentation,
though, in hopes that it will help (a) clarify and (b) alert the reader to be
careful.  Do you think that's sufficient for now?

[gsennton, 2017/03/15 12:57:43]

Yeah, that looks fine ;).

     }
 
-    private void incrementCrashFailureUploadCount(String fileName) {
+    public static void incrementCrashFailureUploadCount(String fileName) {
         ChromePreferenceManager.getInstance().incrementCrashFailureUploadCount(
                 getCrashType(fileName));
     }
 
     /**
      * Factory method for creating minidump callables.
      *
      * This may be overridden for tests.
      *
      * @param minidumpFile the File to upload.
@@ skipping 11 matching lines @@
      *
      * Note that this method is asynchronous. All that is guaranteed is that an upload attempt will
      * be enqueued.
      *
      * @param context The application context, in which to initiate the crash report upload.
      * @throws A security excpetion if the caller doesn't have permission to start the upload
      *         service. This can only happen on KitKat and below, due to a framework bug.
      */
     public static void tryUploadCrashDump(Context context, File minidumpFile)
             throws SecurityException {
+        assert !shouldUseJobSchedulerForUploads();
         CrashFileManager fileManager = new CrashFileManager(context.getCacheDir());
         Intent intent = new Intent(context, MinidumpUploadService.class);
         intent.setAction(ACTION_UPLOAD);
         intent.putExtra(FILE_TO_UPLOAD_KEY, minidumpFile.getAbsolutePath());
         intent.putExtra(UPLOAD_LOG_KEY, fileManager.getCrashUploadLogFile().getAbsolutePath());
         context.startService(intent);
     }
 
     /**
      * Attempts to upload all minidump files using the given {@link android.content.Context}.
      *
      * Note that this method is asynchronous. All that is guaranteed is that
      * upload attempts will be enqueued.
      *
      * This method is safe to call from the UI thread.
      *
      * @param context Context of the application.
      */
     public static void tryUploadAllCrashDumps(Context context) {
+        assert !shouldUseJobSchedulerForUploads();
         CrashFileManager fileManager = new CrashFileManager(context.getCacheDir());
         File[] minidumps = fileManager.getAllMinidumpFiles(MAX_TRIES_ALLOWED);
         Log.i(TAG, "Attempting to upload accumulated crash dumps.");
         for (File minidump : minidumps) {
-            MinidumpUploadService.tryUploadCrashDump(context, minidump);
+            tryUploadCrashDump(context, minidump);
         }
     }
 
     /**
      * Attempts to upload the crash report with the given local ID.
      *
      * Note that this method is asynchronous. All that is guaranteed is that
      * upload attempts will be enqueued.
      *
      * This method is safe to call from the UI thread.
@@ skipping 12 matching lines @@
         File minidumpFile = fileManager.getCrashFileWithLocalId(localId);
         if (minidumpFile == null) {
             Log.w(TAG, "Could not find a crash dump with local ID " + localId);
             return;
         }
         File renamedMinidumpFile = CrashFileManager.trySetForcedUpload(minidumpFile);
         if (renamedMinidumpFile == null) {
             Log.w(TAG, "Could not rename the file " + minidumpFile.getName() + " for re-upload");
             return;
         }
-        MinidumpUploadService.tryUploadCrashDump(context, renamedMinidumpFile);
+
+        if (shouldUseJobSchedulerForUploads()) {
+            scheduleUploadJob(context);
+        } else {
+            tryUploadCrashDump(context, renamedMinidumpFile);
+        }
     }
 }