[Android Crash Reporting] Allow uploading minidumps via the JobScheduler

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

Index: chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadService.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadService.java b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadService.java
index 148b8a6097a18854e69ec18e84ee524609ea4d42..7c89a18a94a39f5b068c94ebeae4d36b9ccf81e0 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadService.java
@@ -4,9 +4,13 @@
 
 package org.chromium.chrome.browser.crash;
 
+import android.annotation.SuppressLint;
 import android.app.IntentService;
+import android.app.job.JobInfo;
 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;
@@ -14,10 +18,12 @@ 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;
@@ -68,6 +74,43 @@ public class MinidumpUploadService extends IntentService {
     }
 
     /**
+     * @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 MinidumpUploadService.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_USAGE_AND_CRASH_REPORTING_PERMITTED_BY_USER,
+                permissionManager.isUsageAndCrashReportingPermittedByUser() ? 1 : 0);
+        permissions.putInt(ChromeMinidumpUploaderDelegate.IS_UPLOAD_ENABLED_FOR_TESTS,
+                permissionManager.isUploadEnabledForTests() ? 1 : 0);
+        MinidumpUploadJobService.scheduleUpload(context, ChromeMinidumpUploadJobService.class,
+                JobInfo.NETWORK_TYPE_ANY, permissions);

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

Not sure I like the mismatch between the network type here, and the
isNetworkAvailable in the ChromeDelegate. A failure from isNetworkAvailable
means increasing the try-number of a minidump by 1, right? So we could easily
end up never uploading any minidumps because we run all our tries on a data
connection?

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

I believe that matches what we currently do in Chrome.  At least this way we
have exponential backoff -- already an improvement! =)

(There's no way, afaict, to schedule the job to run on "WiFi or Ethernet",
metered or not, which is what Chrome currently expects.  For M59, I'm planning
to update the scheduling constraints to simply be unmetered networks.)

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

Right, because updating to only using unmetered networks is too scary of a
change for 58?

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

Affirmative =)

+    }
+
+    /**
      * Stores the successes and failures from uploading crash to UMA,
      */
     public static void storeBreakpadUploadStatsInUma(ChromePreferenceManager pref) {
@@ -249,6 +292,7 @@ public class MinidumpUploadService extends IntentService {
      */
     public static void tryUploadCrashDump(Context context, File minidumpFile)
             throws SecurityException {
+        assert !MinidumpUploadService.shouldUseJobSchedulerForUploads();

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

Should we add this assert in tryUploadAllCrashDumps (or w/e it is called) as
well?

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

Done.  tryUploadAllCrashDumps calls this method internally, but I suppose the
extra clarity doesn't hurt =)

         CrashFileManager fileManager = new CrashFileManager(context.getCacheDir());
         Intent intent = new Intent(context, MinidumpUploadService.class);
         intent.setAction(ACTION_UPLOAD);
@@ -305,6 +349,11 @@ public class MinidumpUploadService extends IntentService {
             Log.w(TAG, "Could not rename the file " + minidumpFile.getName() + " for re-upload");
             return;
         }
-        MinidumpUploadService.tryUploadCrashDump(context, renamedMinidumpFile);
+
+        if (MinidumpUploadService.shouldUseJobSchedulerForUploads()) {
+            MinidumpUploadService.scheduleUploadJob(context);
+        } else {
+            MinidumpUploadService.tryUploadCrashDump(context, renamedMinidumpFile);
+        }
     }
 }