[Android Crash Reporting] Allow uploading minidumps via the JobScheduler

chrome/android/javatests/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnableTest.java

Index: chrome/android/javatests/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnableTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnableTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnableTest.java
index cd851953dff3bf5634e50b2c5a3716357c21a30d..190dded006c116dba57faa7e5d7c94d154a0b873 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnableTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/crash/LogcatExtractionRunnableTest.java
@@ -4,13 +4,19 @@
 
 package org.chromium.chrome.browser.crash;
 
+import android.annotation.TargetApi;
+import android.app.job.JobInfo;
+import android.app.job.JobScheduler;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
+import android.os.Build;
 import android.support.test.filters.MediumTest;
 
 import org.chromium.base.StreamUtil;
 import org.chromium.base.test.util.AdvancedMockContext;
+import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.components.background_task_scheduler.TaskIds;
 import org.chromium.components.minidump_uploader.CrashFileManager;
 import org.chromium.components.minidump_uploader.CrashTestCase;
 
@@ -19,9 +25,10 @@ import java.io.File;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
-import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.Map;
 
 /**
  * Unittests for {@link LogcatExtractionRunnable}.
@@ -29,83 +36,189 @@ import java.util.concurrent.atomic.AtomicInteger;
 public class LogcatExtractionRunnableTest extends CrashTestCase {
     private File mCrashDir;
 
+    private static final String BOUNDARY = "boundary";
+    private static final String MINIDUMP_CONTENTS = "important minidump contents";
+    private static final List<String> LOGCAT =
+            Arrays.asList("some random log content", "some more deterministic log content");
+
+    private static class TestLogcatExtractionRunnable extends LogcatExtractionRunnable {
+        TestLogcatExtractionRunnable(Context context, File minidump) {
+            super(context, minidump);
+        }
+
+        @Override
+        protected List<String> getLogcat() {
+            return LOGCAT;
+        }
+    };
+
+    @TargetApi(Build.VERSION_CODES.LOLLIPOP)
+    private static class TestJobScheduler extends JobScheduler {
+        private final JobScheduler mRealScheduler;

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

I don't quite see why you have a real JobScheduler in here - you aren't actually
checking that the real jobscheduler is working? (and doing so would probably be
bad since we don't know how long the jobscheduler will wait before running a
job).

[Ilya Sherman, 2017/03/16 01:36:57]

Hmm, good point. Simplified.

+
+        TestJobScheduler(JobScheduler realScheduler) {
+            mRealScheduler = realScheduler;
+        }
+
+        @Override
+        public void cancel(int jobId) {
+            mRealScheduler.cancel(jobId);
+        }
+
+        @Override
+        public void cancelAll() {
+            mRealScheduler.cancelAll();
+        }
+
+        @Override
+        public List<JobInfo> getAllPendingJobs() {
+            return mRealScheduler.getAllPendingJobs();
+        }
+
+        @Override
+        public JobInfo getPendingJob(int jobId) {
+            return mRealScheduler.getPendingJob(jobId);
+        }
+
+        @Override
+        public int schedule(JobInfo job) {
+            assertEquals(TaskIds.CHROME_MINIDUMP_UPLOADING_JOB_ID, job.getId());
+            assertEquals(ChromeMinidumpUploadJobService.class.getName(),
+                    job.getService().getClassName());
+            return mRealScheduler.schedule(job);
+        }
+    };
+
+    // Responsible for verifying that the correct intent is fired after the logcat is extracted.
+    private class TestContext extends AdvancedMockContext {
+        int mNumServiceStarts;
+
+        TestContext(Context realContext) {
+            super(realContext);
+        }
+
+        @Override
+        public ComponentName startService(Intent intent) {
+            assertFalse("Should only start a service directly when the job scheduler is disabled.",
+                    ChromeFeatureList.isEnabled(
+                            ChromeFeatureList.UPLOAD_CRASH_REPORTS_USING_JOB_SCHEDULER));
+            ++mNumServiceStarts;
+            assertEquals(1, mNumServiceStarts);
+            assertEquals(
+                    MinidumpUploadService.class.getName(), intent.getComponent().getClassName());
+            assertEquals(MinidumpUploadService.ACTION_UPLOAD, intent.getAction());
+            assertEquals(new File(mCrashDir, "test.dmp.try0").getAbsolutePath(),
+                    intent.getStringExtra(MinidumpUploadService.FILE_TO_UPLOAD_KEY));
+            return super.startService(intent);
+        }
+
+        @Override
+        public Object getSystemService(String name) {
+            if (Context.JOB_SCHEDULER_SERVICE.equals(name)) {
+                assertTrue("Should only access the JobScheduler when it is enabled.",
+                        ChromeFeatureList.isEnabled(
+                                ChromeFeatureList.UPLOAD_CRASH_REPORTS_USING_JOB_SCHEDULER));
+                return new TestJobScheduler((JobScheduler) super.getSystemService(name));
+            }
+
+            return super.getSystemService(name);
+        }
+    };
+
     @Override
     protected void setUp() throws Exception {
         super.setUp();
         mCrashDir = new CrashFileManager(mCacheDir).getCrashDirectory();
     }
 
-    @MediumTest
-    public void testSimpleExtraction() {
-        // Set up the test.
-        final List<String> logcat = new ArrayList<String>();
-        logcat.add("some random log content");
-        logcat.add("some more deterministic log content");
-
-        final File minidump = new File(mCrashDir, "test.dmp");
-        final String boundary = "boundary";
-        final String minidumpContents = "important minidump contents";
+    @Override
+    protected void tearDown() {
+        ChromeFeatureList.setTestFeatures(null);
+    }
+
+    /**
+     * Sets whether to upload minidumps using the JobScheduler API. Minidumps can either be uploaded
+     * via a JobScheduler, or via a direct Intent service.
+     * @param enable Whether to enable the JobScheduler API.
+     */
+    private void setJobSchedulerEnabled(boolean enable) {
+        Map<String, Boolean> features = new HashMap<>();
+        features.put(ChromeFeatureList.UPLOAD_CRASH_REPORTS_USING_JOB_SCHEDULER, enable);
+        ChromeFeatureList.setTestFeatures(features);
+    }
+
+    /**
+     * Creates a simple fake minidump file for testing.
+     * @param filename The name of the file to create.
+     */
+    private File createMinidump(String filename) throws IOException {
+        File minidump = new File(mCrashDir, filename);
         FileWriter writer = null;
         try {
             writer = new FileWriter(minidump);
-            writer.write(boundary + "\n");
-            writer.write(minidumpContents + "\n");
-        } catch (IOException e) {
-            fail(e.toString());
+            writer.write(BOUNDARY + "\n");
+            writer.write(MINIDUMP_CONTENTS + "\n");
         } finally {
             StreamUtil.closeQuietly(writer);
         }
+        return minidump;
+    }
 
-        // The testContext is responsible for verifying that the correct intent is fired after the
-        // logcat is extracted.
-        final AtomicInteger numServiceStarts = new AtomicInteger(0);
-        Context testContext = new AdvancedMockContext(getInstrumentation().getTargetContext()) {
-            @Override
-            public ComponentName startService(Intent intent) {
-                assertEquals(1, numServiceStarts.incrementAndGet());
-                assertEquals(MinidumpUploadService.class.getName(),
-                        intent.getComponent().getClassName());
-                assertEquals(MinidumpUploadService.ACTION_UPLOAD, intent.getAction());
-                assertEquals(new File(mCrashDir, "test.dmp.try0").getAbsolutePath(),
-                        intent.getStringExtra(MinidumpUploadService.FILE_TO_UPLOAD_KEY));
-                return super.startService(intent);
-            }
-        };
-
-        // Extract the logcat!
-        LogcatExtractionRunnable runnable = new LogcatExtractionRunnable(testContext, minidump) {
-            @Override
-            protected List<String> getLogcat() {
-                return logcat;
-            }
-        };
-        runnable.run();
-
-        // Verify the file contents.
+    /**
+     * Verifies that the contents of the {@param filename} are the expected ones.
+     * @param filename The name of the file containing the concatenated logcat and minidump output.
+     */
+    private void verifyMinidumpWithLogcat(String filename) throws IOException {
         BufferedReader input = null;
         try {
-            File logfile = new File(mCrashDir, "test.dmp.try0");
-            assertTrue("Logfile should exist!", logfile.exists());
+            File minidumpWithLogcat = new File(mCrashDir, filename);
+            assertTrue("Should have created a file containing the logcat and minidump contents",
+                    minidumpWithLogcat.exists());
 
-            input = new BufferedReader(new FileReader(logfile));
-            assertEquals("The first line should be the boundary line.", boundary, input.readLine());
+            input = new BufferedReader(new FileReader(minidumpWithLogcat));
+            assertEquals("The first line should be the boundary line.", BOUNDARY, input.readLine());
             assertEquals("The second line should be the content dispoistion.",
                     MinidumpLogcatPrepender.LOGCAT_CONTENT_DISPOSITION, input.readLine());
             assertEquals("The third line should be the content type.",
                     MinidumpLogcatPrepender.LOGCAT_CONTENT_TYPE, input.readLine());
             assertEquals("The fourth line should be blank, for padding.", "", input.readLine());
-            for (String expected : logcat) {
-                assertEquals(expected, input.readLine());
+            for (String expected : LOGCAT) {
+                assertEquals("The logcat contents should match", expected, input.readLine());
             }
-            assertEquals("The logcat should be followed by the boundary line.", boundary,
+            assertEquals("The logcat should be followed by the boundary line.", BOUNDARY,
                     input.readLine());
             assertEquals(
-                    "The minidump contents should follow.", minidumpContents, input.readLine());
+                    "The minidump contents should follow.", MINIDUMP_CONTENTS, input.readLine());
             assertNull("There should be nothing else in the file", input.readLine());
-        } catch (IOException e) {
-            fail(e.toString());
         } finally {
             StreamUtil.closeQuietly(input);
         }
     }
+
+    @MediumTest
+    public void testSimpleExtraction_SansJobScheduler() throws IOException {
+        setJobSchedulerEnabled(false);
+        final File minidump = createMinidump("test.dmp");
+        Context testContext = new TestContext(getInstrumentation().getTargetContext());
+
+        LogcatExtractionRunnable runnable = new TestLogcatExtractionRunnable(testContext, minidump);
+        runnable.run();
+
+        verifyMinidumpWithLogcat("test.dmp.try0");
+    }
+
+    @MediumTest
+    public void testSimpleExtraction_WithJobScheduler() throws IOException {
+        // The JobScheduler API is only available as of Android L.
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) return;
+
+        setJobSchedulerEnabled(true);
+        final File minidump = createMinidump("test.dmp");
+        Context testContext = new TestContext(getInstrumentation().getTargetContext());
+
+        LogcatExtractionRunnable runnable = new TestLogcatExtractionRunnable(testContext, minidump);
+        runnable.run();
+
+        verifyMinidumpWithLogcat("test.dmp.try0");
+    }
 }