[Android Crash Reporting] Allow uploading minidumps via the JobScheduler

components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadJobService.java

Index: components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadJobService.java
diff --git a/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadJobService.java b/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadJobService.java
index c275030846f7b104471f8a6dc2c0397644de1848..00e8411d1895758643e6639f206af9df0f9a1c4f 100644
--- a/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadJobService.java
+++ b/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadJobService.java
@@ -4,33 +4,89 @@
 package org.chromium.components.minidump_uploader;
 
 import android.annotation.TargetApi;
+import android.app.job.JobInfo;
 import android.app.job.JobParameters;
+import android.app.job.JobScheduler;
 import android.app.job.JobService;
+import android.content.ComponentName;
+import android.content.Context;
 import android.os.Build;
+import android.os.PersistableBundle;
+
+import org.chromium.base.Log;
 
 /**
  * Class that interacts with the Android JobScheduler to upload Minidumps at appropriate times.
  */
 @TargetApi(Build.VERSION_CODES.LOLLIPOP)
 public abstract class MinidumpUploadJobService extends JobService {
-    Object mRunningLock = new Object();
-    boolean mRunningJob = false;
-    MinidumpUploader mMinidumpUploader;
+    private static final String TAG = "MinidumpJobService";
+
+    // 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,
+    // android.content.ComponentName)
+    private static final int MINIDUMP_UPLOADING_JOB_ID = 42;

[gsennton, 2017/03/13 17:57:17]

I think we want different numbers for Chrome and WebView here - otherwise
(Mono)Chrome and (Monochrome-)WebView will interrupt each others' jobs.

[Ilya Sherman, 2017/03/14 02:18:55]

Done.  Good point -- I completely forgot about Monochrome!

+
+    // Initial back-off time for upload-job. This is set to a fairly high number (30 minutes) to
+    // increase the chance of performing uploads in batches if the initial upload fails.
+    private static final int JOB_BACKOFF_TIME_IN_MS = 1000 * 60 * 30;
+
+    // Back-off policy for upload-job.
+    private static final int JOB_BACKOFF_POLICY = JobInfo.BACKOFF_POLICY_EXPONENTIAL;
+
+    private final Object mRunningLock = new Object();
+    private boolean mRunningJob = false;
+    private MinidumpUploader mMinidumpUploader;
+
+    /**
+     * Schedules uploading of all pending minidumps.
+     * @param context The application context, in which to schedule the crash report uploads.
+     * @param jobServiceClass The concrete class to use as the job service for uploading minidumps.
+     *     Should be a subclass of this class.
+     * @param requiredNetworkType The network type required for upload. Should be set to one of the
+     *     NETWORK_TYPE constants defined in the Android JobInfo class.
+     * @param extras Any serialized data that should be passed to the upload job, in case the job
+     *     runs after the calling process is destroyed.
+     */
+    public static void scheduleUpload(Context context,
+            Class<? extends MinidumpUploadJobService> jobServiceClass, int requiredNetworkType,
+            PersistableBundle extras) {
+        Log.i(TAG, "Scheduling upload of all pending minidumps.");
+        JobScheduler scheduler =
+                (JobScheduler) context.getSystemService(Context.JOB_SCHEDULER_SERVICE);
+        JobInfo uploadJob =
+                new JobInfo
+                        .Builder(MINIDUMP_UPLOADING_JOB_ID,
+                                new ComponentName(context, jobServiceClass))
+                        .setRequiredNetworkType(requiredNetworkType)
+                        // Minimum delay when a job is retried (a retry will happen when
+                        // there are minidumps left after trying to upload all minidumps -
+                        // this could e.g. happen if we add more minidumps at the same time
+                        // as uploading old ones).
+                        .setBackoffCriteria(JOB_BACKOFF_TIME_IN_MS, JOB_BACKOFF_POLICY)
+                        .setExtras(extras)
+                        .build();
+        if (scheduler.schedule(uploadJob) == JobScheduler.RESULT_FAILURE) {
+            throw new RuntimeException("couldn't schedule " + uploadJob);
+        }
+    }
 
     @Override
     public boolean onStartJob(JobParameters params) {
         // Ensure we only run one job at a time.
         synchronized (mRunningLock) {
+            // TODO(isherman): We never read mRunningJob -- is it obsolete?

[Ilya Sherman, 2017/03/13 03:12:09]

Gustav, I forget whether we've discussed this before.  Does mRunningJob still
make sense?  It doesn't seem to be read (outside of the assertion below, I
suppose...)

[gsennton, 2017/03/13 17:57:17]

It's asserting that we aren't running several jobs at the same time (only on
debug-builds though since we are using an assertion). It's just a sanity check.

[Ilya Sherman, 2017/03/14 02:18:55]

Okay, as long as it's intended just a sanity-check, that's ok.  (Though, I don't
think this code is actually reachable in debug builds, at least not without
jumping through a lot of hoops -- crash reporting tends to be disabled in debug
builds.  At least, that's true for Chrome -- I'm less sure about Webview =])

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

Yeah, it's only enabled for WebView with a certain command-line flag turned on
(something like --enable-crash-uploading).

             assert !mRunningJob;
             mRunningJob = true;
         }
-        mMinidumpUploader = createMinidumpUploader();
+        mMinidumpUploader = createMinidumpUploader(params.getExtras());
         mMinidumpUploader.uploadAllMinidumps(createJobFinishedCallback(params));
         return true; // true = processing work on a separate thread, false = done already.
     }
 
     @Override
     public boolean onStopJob(JobParameters params) {
+        Log.i(TAG, "Canceling pending uploads due to change in networking status.");
         boolean reschedule = mMinidumpUploader.cancelUploads();
         synchronized (mRunningLock) {
             mRunningJob = false;
@@ -49,6 +105,9 @@ public abstract class MinidumpUploadJobService extends JobService {
         return new MinidumpUploader.UploadsFinishedCallback() {
             @Override
             public void uploadsFinished(boolean reschedule) {
+                if (reschedule) {
+                    Log.i(TAG, "Some minidumps remain un-uploaded; rescheduling.");
+                }
                 synchronized (mRunningLock) {
                     mRunningJob = false;
                 }
@@ -58,7 +117,8 @@ public abstract class MinidumpUploadJobService extends JobService {
     }
 
     /**
+     * @param extras Any extra data persisted for this job.
      * @return The minidump uploader that jobs should use.
      */
-    protected abstract MinidumpUploader createMinidumpUploader();
+    protected abstract MinidumpUploader createMinidumpUploader(PersistableBundle extras);
 }