[Android WebView] Split out WebView-parts of MinidumpUploaderTest

android_webview/javatests/src/org/chromium/android_webview/test/crash/WebViewMinidumpUploaderTest.java

Index: android_webview/javatests/src/org/chromium/android_webview/test/crash/WebViewMinidumpUploaderTest.java
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/crash/WebViewMinidumpUploaderTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/crash/WebViewMinidumpUploaderTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..5bd262a64e5935399d06fe9f17458cd687355b5b
--- /dev/null
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/crash/WebViewMinidumpUploaderTest.java
@@ -0,0 +1,242 @@
+// Copyright 2017 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.android_webview.test.crash;
+
+import android.content.Context;
+import android.os.ParcelFileDescriptor;
+import android.support.test.filters.MediumTest;
+
+import org.chromium.android_webview.PlatformServiceBridge;
+import org.chromium.android_webview.crash.CrashReceiverService;
+import org.chromium.android_webview.crash.MinidumpUploader;
+import org.chromium.android_webview.crash.MinidumpUploaderImpl;
+import org.chromium.components.minidump_uploader.CrashFileManager;
+import org.chromium.components.minidump_uploader.MinidumpUploadCallable;
+import org.chromium.components.minidump_uploader.MinidumpUploadCallableTest;
+import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+
+/**
+ * WebView-specific instrumentation tests for MinidumpUploader.
+ * These tests target the WebView-specific parts of MinidumpUploaderImpl, and the combined use of
+ * CrashReceiverService and MinidumpUploaderImpl.
+ */
+public class WebViewMinidumpUploaderTest extends MinidumpUploaderTest {

[Ilya Sherman, 2017/02/28 04:00:04]

FWIW, I'd prefer not to use inheritance like this for tests, and instead to
factor out some test utility classes and methods if need be.  Inheritance can
easily hide a lot of complexity; and it's nice if tests are as simple and
self-contained as possible.  (Inheriting from really generic instrumentation
frameworks is a bit different, as they are designed with very simple APIs in
mind.)

[gsennton, 2017/02/28 16:30:05]

Alright, makes sense :) (I'll update this to do so).

+    /**
+     * MinidumpUploaderImpl sub-class that uses WebView's MinidumpUploaderImpl implementation of
+     * CrashReportingPermissionManager.isUsageAndCrashReportingPermittedByUser().
+     */
+    private static class WebViewUserConsentMinidumpUploaderImpl extends MinidumpUploaderImpl {
+        boolean mUserConsent;
+        WebViewUserConsentMinidumpUploaderImpl(
+                Context context, boolean cleanOutMinidumps, boolean userConsent) {
+            super(context, cleanOutMinidumps);
+            mUserConsent = userConsent;
+        }
+        @Override
+        public PlatformServiceBridge createPlatformServiceBridge() {
+            return new TestPlatformServiceBridge(true /* canUseGms */, mUserConsent);
+        }
+        @Override
+        public MinidumpUploadCallable createMinidumpUploadCallable(
+                File minidumpFile, File logfile) {
+            return new MinidumpUploadCallable(minidumpFile, logfile,
+                    new MinidumpUploadCallableTest.TestHttpURLConnectionFactory(),
+                    createWebViewCrashReportingManager());
+        }
+        @Override
+        protected CrashReportingPermissionManager createWebViewCrashReportingManager() {
+            final CrashReportingPermissionManager realPermissionManager =
+                    super.createWebViewCrashReportingManager();
+            return new MockCrashReportingPermissionManager() {
+                {
+                    // This setup ensures we depend on isUsageAndCrashReportingPermittedByUser().
+                    mIsInSample = true;
+                    mIsPermitted = true;
+                    mIsCommandLineDisabled = false;
+                    mIsNetworkAvailable = true;
+                    mIsEnabledForTests = false;
+                }
+                @Override
+                public boolean isUsageAndCrashReportingPermittedByUser() {
+                    // Ensure that we use the real implementation of
+                    // isUsageAndCrashReportingPermittedByUser.
+                    boolean userPermitted =
+                            realPermissionManager.isUsageAndCrashReportingPermittedByUser();
+                    assertEquals(mUserConsent, userPermitted);
+                    return userPermitted;
+                }
+            };
+        }
+    }
+
+    /**
+     * Ensure that when PlatformServiceBridge returns true we do upload minidumps.
+     */
+    @MediumTest
+    public void testPlatformServicesBridgeIsUsedUserConsent() throws IOException {
+        testPlatformServicesBridgeIsUsed(true);
+    }
+
+    /**
+     * Ensure that when PlatformServiceBridge returns false we do not upload minidumps.
+     */
+    @MediumTest
+    public void testPlatformServicesBridgeIsUsedNoUserConsent() throws IOException {
+        testPlatformServicesBridgeIsUsed(false);
+    }
+
+    private void testPlatformServicesBridgeIsUsed(final boolean userConsent) throws IOException {
+        MinidumpUploader minidumpUploader =
+                new WebViewUserConsentMinidumpUploaderImpl(getInstrumentation().getTargetContext(),
+                        false /* cleanOutMinidumps */, userConsent);
+
+        File firstFile = createMinidumpFileInCrashDir("1_abc.dmp0");
+        File secondFile = createMinidumpFileInCrashDir("12_abcd.dmp0");
+        File expectedFirstFile = new File(
+                mCrashDir, firstFile.getName().replace(".dmp", userConsent ? ".up" : ".skipped"));
+        File expectedSecondFile = new File(
+                mCrashDir, secondFile.getName().replace(".dmp", userConsent ? ".up" : ".skipped"));
+
+        MinidumpUploaderTest.uploadMinidumpsSync(minidumpUploader, false /* expectReschedule */);
+
+        assertFalse(firstFile.exists());
+        assertTrue(expectedFirstFile.exists());
+        assertFalse(secondFile.exists());
+        assertTrue(expectedSecondFile.exists());
+    }
+
+    /**
+     * Ensures that the minidump copying works together with the minidump uploading.
+     */
+    @MediumTest
+    public void testCopyAndUploadWebViewMinidump() throws FileNotFoundException, IOException {
+        final CrashFileManager fileManager = new CrashFileManager(
+                CrashReceiverService.getWebViewCrashDir(getInstrumentation().getTargetContext()));
+        // Note that these minidump files are set up directly in the cache dir - not in the WebView
+        // crash dir. This is to ensure the CrashFileManager doesn't see these minidumps without us
+        // first copying them.
+        File minidumpToCopy =
+                new File(getInstrumentation().getTargetContext().getCacheDir(), "toCopy.dmp");
+        setUpMinidumpFile(minidumpToCopy, BOUNDARY, "browser");
+        final String expectedFileContent = readEntireFile(minidumpToCopy);
+
+        File[] uploadedFiles = copyAndUploadMinidumpsSync(
+                fileManager, new File[][] {{minidumpToCopy}}, new int[] {0});
+
+        // CrashReceiverService will rename the minidumps to some globally unique file name
+        // meaning that we have to check the contents of the minidump rather than the file
+        // name.
+        assertEquals(expectedFileContent, readEntireFile(uploadedFiles[0]));

[gsennton, 2017/02/23 14:48:48]

This is the only change I did except just moving code here: 
try {
    assertEquals(expectedFileContent, readEntireFile(uploadedFiles[0]));
} catch (IOException e) {
    throw new RuntimeException(e);
}

====>

assertEquals(expectedFileContent, readEntireFile(uploadedFiles[0]));

+        File webviewTmpDir =
+                CrashReceiverService.getWebViewTmpCrashDir(getInstrumentation().getTargetContext());
+        assertEquals(0, webviewTmpDir.listFiles().length);
+    }
+
+    /**
+     * Ensure we can copy and upload several batches of files (i.e. emulate several copying-calls in
+     * a row without the copying-service being destroyed in between).
+     */
+    @MediumTest
+    public void testCopyAndUploadSeveralMinidumpBatches() throws IOException {
+        final CrashFileManager fileManager = new CrashFileManager(
+                CrashReceiverService.getWebViewCrashDir(getInstrumentation().getTargetContext()));
+        // Note that these minidump files are set up directly in the cache dir - not in the WebView
+        // crash dir. This is to ensure the CrashFileManager doesn't see these minidumps without us
+        // first copying them.
+        File firstMinidumpToCopy =
+                new File(getInstrumentation().getTargetContext().getCacheDir(), "firstToCopy.dmp");
+        File secondMinidumpToCopy =
+                new File(getInstrumentation().getTargetContext().getCacheDir(), "secondToCopy.dmp");
+        setUpMinidumpFile(firstMinidumpToCopy, BOUNDARY, "browser");
+        setUpMinidumpFile(secondMinidumpToCopy, BOUNDARY, "renderer");
+        final String expectedFirstFileContent = readEntireFile(firstMinidumpToCopy);
+        final String expectedSecondFileContent = readEntireFile(secondMinidumpToCopy);
+
+        File[] uploadedFiles = copyAndUploadMinidumpsSync(fileManager,
+                new File[][] {{firstMinidumpToCopy}, {secondMinidumpToCopy}}, new int[] {0, 0});
+
+        // CrashReceiverService will rename the minidumps to some globally unique file name
+        // meaning that we have to check the contents of the minidump rather than the file
+        // name.
+        final String actualFileContent0 = readEntireFile(uploadedFiles[0]);
+        final String actualFileContent1 = readEntireFile(uploadedFiles[1]);
+        if (expectedFirstFileContent.equals(actualFileContent0)) {

[gsennton, 2017/02/23 14:48:48]

Same as above here - remove try-catch-rethrow of IOException.

+            assertEquals(expectedSecondFileContent, actualFileContent1);
+        } else {
+            assertEquals(expectedFirstFileContent, actualFileContent1);
+            assertEquals(expectedSecondFileContent, actualFileContent0);
+        }
+    }
+
+    private static String readEntireFile(File file) throws IOException {
+        FileInputStream fileInputStream = new FileInputStream(file);
+        try {
+            byte[] data = new byte[(int) file.length()];
+            fileInputStream.read(data);
+            return new String(data);
+        } finally {
+            fileInputStream.close();
+        }
+    }
+
+    /**
+     * Copy and upload {@param minidumps} by one array at a time - i.e. the minidumps in a single
+     * array in {@param minidumps} will all be copied in the same call into CrashReceiverService.
+     * @param fileManager the CrashFileManager to use when copying/renaming minidumps.
+     * @param minidumps an array of arrays of minidumps to copy and upload, by copying one array at
+     * a time.
+     * @param uids an array of uids declaring the uids used when calling into CrashReceiverService.
+     * @return the uploaded files.
+     */
+    private File[] copyAndUploadMinidumpsSync(CrashFileManager fileManager, File[][] minidumps,
+            int[] uids) throws FileNotFoundException {
+        CrashReceiverService crashReceiverService = new CrashReceiverService();
+        assertEquals(minidumps.length, uids.length);
+        // Ensure the upload service minidump directory is empty before we start copying files.
+        File[] initialMinidumps =
+                fileManager.getAllMinidumpFiles(MinidumpUploaderImpl.MAX_UPLOAD_TRIES_ALLOWED);
+        assertEquals(0, initialMinidumps.length);
+
+        // Open file descriptors to the files and then delete the files.
+        ParcelFileDescriptor[][] fileDescriptors = new ParcelFileDescriptor[minidumps.length][];
+        int numMinidumps = 0;
+        for (int n = 0; n < minidumps.length; n++) {
+            File[] currentMinidumps = minidumps[n];
+            numMinidumps += currentMinidumps.length;
+            fileDescriptors[n] = new ParcelFileDescriptor[currentMinidumps.length];
+            for (int m = 0; m < currentMinidumps.length; m++) {
+                fileDescriptors[n][m] = ParcelFileDescriptor.open(
+                        currentMinidumps[m], ParcelFileDescriptor.MODE_READ_ONLY);
+                assertTrue(currentMinidumps[m].delete());
+            }
+            crashReceiverService.performMinidumpCopyingSerially(
+                    getInstrumentation().getTargetContext(), uids[n] /* uid */, fileDescriptors[n],
+                    false /* scheduleUploads */);
+        }
+
+        final CrashReportingPermissionManager permManager =
+                new MockCrashReportingPermissionManager() {
+                    { mIsEnabledForTests = true; }
+                };
+        MinidumpUploader minidumpUploader =
+                new TestMinidumpUploaderImpl(getInstrumentation().getTargetContext(), permManager,
+                        false /* cleanOutMinidumps */);
+
+        MinidumpUploaderTest.uploadMinidumpsSync(minidumpUploader, false /* expectReschedule */);
+        // Ensure there are no minidumps left to upload.
+        File[] nonUploadedMinidumps =
+                fileManager.getAllMinidumpFiles(MinidumpUploaderImpl.MAX_UPLOAD_TRIES_ALLOWED);
+        assertEquals(0, nonUploadedMinidumps.length);
+
+        File[] uploadedFiles = fileManager.getAllUploadedFiles();
+        assertEquals(numMinidumps, uploadedFiles.length);
+        return uploadedFiles;
+    }
+}