[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.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.
  */
@@ skipping 30 matching lines @@
     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 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) {
         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 161 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 !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);
         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}.
@@ skipping 36 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 (MinidumpUploadService.shouldUseJobSchedulerForUploads()) {
+            MinidumpUploadService.scheduleUploadJob(context);
+        } else {
+            MinidumpUploadService.tryUploadCrashDump(context, renamedMinidumpFile);
+        }
     }
 }