[Android WebView] Always schedule new upload job after copying minidump.

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

 // Copyright 2016 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 static org.chromium.base.test.util.ScalableTimeout.scaleTimeout;
 
 import android.content.Context;
 import android.os.ParcelFileDescriptor;
@@ skipping 13 matching lines @@
 import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager;
 import org.chromium.components.minidump_uploader.util.HttpURLConnectionFactory;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 /**
  * Instrumentation tests for MinidumpUploader.
  */
 public class MinidumpUploaderTest extends CrashTestCase {
     private static final String TAG = "MinidumpUploaderTest";
@@ skipping 95 matching lines @@
         // This file should have been left untouched.
         assertTrue(maxTriesFile.exists());
     }
 
     /**
      * Utility method for running {@param minidumpUploader}.uploadAllMinidumps on the UI thread to
      * avoid breaking any assertions about running on the UI thread.
      */
     private static void uploadAllMinidumpsOnUiThread(final MinidumpUploader minidumpUploader,
             final MinidumpUploader.UploadsFinishedCallback uploadsFinishedCallback) {
+        uploadAllMinidumpsOnUiThread(
+                minidumpUploader, uploadsFinishedCallback, false /* blockUntilJobPosted */);
+    }
+
+    private static void uploadAllMinidumpsOnUiThread(final MinidumpUploader minidumpUploader,
+            final MinidumpUploader.UploadsFinishedCallback uploadsFinishedCallback,
+            boolean blockUntilJobPosted) {
+        final CountDownLatch jobPostedLatch = new CountDownLatch(1);
         ThreadUtils.runOnUiThread(new Runnable() {
             @Override
             public void run() {
                 minidumpUploader.uploadAllMinidumps(uploadsFinishedCallback);
+                jobPostedLatch.countDown();
             }
         });
+        if (blockUntilJobPosted) {
+            try {
+                jobPostedLatch.await();
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);

[Ilya Sherman, 2017/02/11 01:00:47]

What's the reason to catch the exception just to rethrow it again as a different
type of exception?

[gsennton, 2017/02/13 10:46:55]

Just to avoid having to declare the Exception type being thrown for each method
using this method :).

+            }
+        }
     }
 
     private static void uploadMinidumpsSync(
             MinidumpUploader minidumpUploader, final boolean expectReschedule) {
         final CountDownLatch uploadsFinishedLatch = new CountDownLatch(1);
         uploadAllMinidumpsOnUiThread(
                 minidumpUploader, new MinidumpUploader.UploadsFinishedCallback() {
                     @Override
                     public void uploadsFinished(boolean reschedule) {
                         assertEquals(expectReschedule, reschedule);
@@ skipping 104 matching lines @@
             expectedSecondFile = new File(mCrashDir, secondFile.getName().replace(".dmp", ".up"));
         } else {
             File uploadedFirstFile =
                     new File(mCrashDir, firstFile.getName().replace(".dmp", ".up"));
             assertTrue(uploadedFirstFile.exists());
             expectedSecondFile = new File(mCrashDir, secondFile.getName() + ".try1");
         }
         assertTrue(expectedSecondFile.exists());
     }
 
-    private static class StallingOutputStream extends OutputStream {
-        @Override
-        public void write(int b) throws IOException {
+    private static class StallingHttpUrlConnectionFactory implements HttpURLConnectionFactory {
+        private final CountDownLatch mStopStallingLatch;
+        private final boolean mSucceed;
+
+        private class StallingOutputStream extends OutputStream {
+            @Override
+            public void write(int b) throws IOException {
+                try {
+                    mStopStallingLatch.await();
+                } catch (InterruptedException e) {
+                    throw new InterruptedIOException(e.toString());
+                }
+                if (!mSucceed) {
+                    throw new IOException();
+                }
+            }
+        }
+
+        public StallingHttpUrlConnectionFactory(CountDownLatch stopStallingLatch, boolean succeed) {
+            mStopStallingLatch = stopStallingLatch;
+            mSucceed = succeed;
+        }
+
+        public HttpURLConnection createHttpURLConnection(String url) {
             try {
-                TimeUnit.MINUTES.sleep(100);
-            } catch (InterruptedException e) {
-                throw new InterruptedIOException(e.toString());
+                return new MinidumpUploadCallableTest.TestHttpURLConnection(new URL(url)) {
+                    @Override
+                    public OutputStream getOutputStream() {
+                        return new StallingOutputStream();
+                    }
+                };
+            } catch (MalformedURLException e) {
+                return null;
             }
         }
     }
 
-    private static class StallingHttpUrlConnectionFactory implements HttpURLConnectionFactory {
-        public HttpURLConnection createHttpURLConnection(String url) {
-            return new HttpURLConnection(null) {
-                @Override
-                public OutputStream getOutputStream() throws IOException {
-                    return new StallingOutputStream();
-                }
-
-                @Override
-                public void connect() {}
-
-                @Override
-                public void disconnect() {}
-
-                @Override
-                public boolean usingProxy() {
-                    return false;
-                }
-            };
-        }
-    }
-
     private static class FailingHttpUrlConnectionFactory implements HttpURLConnectionFactory {
         public HttpURLConnection createHttpURLConnection(String url) {
             return null;
         }
     }
 
     /**
-     * Test that ensure we can interrupt the MinidumpUploader when uploading minidumps.
+     * Test that ensures we can interrupt the MinidumpUploader when uploading minidumps.
      */
     @MediumTest
-    public void testCancelMinidumpUploads() throws IOException {
+    public void testCancelMinidumpUploadsFailedUpload() throws IOException {
+        testCancellation(false /* successfulUpload */);
+    }
+
+    /**
+     * Test that ensures interrupting our upload-job will not interrupt the first upload.
+     */
+    @MediumTest
+    public void testCancelingWontCancelFirstUpload() throws IOException {
+        testCancellation(true /* successfulUpload */);
+    }
+
+    private void testCancellation(final boolean successfulUpload) throws IOException {
         final CrashReportingPermissionManager permManager =
                 new MockCrashReportingPermissionManager() {
                     { mIsEnabledForTests = true; }
                 };
-        MinidumpUploader minidumpUploader = new MinidumpUploaderImpl(
+        final CountDownLatch stopStallingLatch = new CountDownLatch(1);
+        MinidumpUploaderImpl minidumpUploader = new MinidumpUploaderImpl(
                 getInstrumentation().getTargetContext(), false /* cleanOutMinidumps */) {
             @Override
             public MinidumpUploadCallable createMinidumpUploadCallable(
                     File minidumpFile, File logfile) {
-                return new MinidumpUploadCallable(
-                        minidumpFile, logfile, new StallingHttpUrlConnectionFactory(), permManager);
+                return new MinidumpUploadCallable(minidumpFile, logfile,
+                        new StallingHttpUrlConnectionFactory(stopStallingLatch, successfulUpload),
+                        permManager);
             }
             @Override
             public PlatformServiceBridge createPlatformServiceBridge() {
                 return new TestPlatformServiceBridge(
                         true /* canUseGms*/, permManager.isUsageAndCrashReportingPermittedByUser());
             }
         };
 
         File firstFile = createMinidumpFileInCrashDir("123_abc.dmp0");
-        File nonExpectedFirstUploadFile = new File(mCrashDir, firstFile.getName() + ".up");
-        File nonExpectedFirstRetryFile = new File(mCrashDir, firstFile.getName() + ".try1");
+        File expectedFirstUploadFile =
+                new File(mCrashDir, firstFile.getName().replace(".dmp", ".up"));
+        File expectedFirstRetryFile = new File(mCrashDir, firstFile.getName() + ".try1");
 
         // This is run on the UI thread to avoid failing any assertOnUiThread assertions.
-        uploadAllMinidumpsOnUiThread(
-                minidumpUploader, new MinidumpUploader.UploadsFinishedCallback() {
+        uploadAllMinidumpsOnUiThread(minidumpUploader,
+                new MinidumpUploader.UploadsFinishedCallback() {
                     @Override
                     public void uploadsFinished(boolean reschedule) {
-                        fail("This method shouldn't be called when we interrupt uploads.");
+                        if (successfulUpload) {
+                            assertFalse(reschedule);
+                        } else {
+                            fail("This method shouldn't be called when a canceled upload fails.");
+                        }
                     }
-                });
+                },
+                // Block until job posted - otherwise the worker thread might not have been created
+                // before we try to join it.
+                true /* blockUntilJobPosted */);
         minidumpUploader.cancelUploads();
+        stopStallingLatch.countDown();
+        // Wait until our job finished.
+        try {
+            minidumpUploader.joinWorkerThreadForTesting();
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
 
-        assertTrue(firstFile.exists());
-        assertFalse(nonExpectedFirstUploadFile.exists());
-        assertFalse(nonExpectedFirstRetryFile.exists());
+        if (successfulUpload) {
+            // When the upload succeeds we expect the file to be renamed.
+            assertFalse(firstFile.exists());
+            assertTrue(expectedFirstUploadFile.exists());
+            assertFalse(expectedFirstRetryFile.exists());
+        } else {
+            // When the upload fails we won't change the minidump at all.
+            assertTrue(firstFile.exists());
+            assertFalse(expectedFirstUploadFile.exists());
+            assertFalse(expectedFirstRetryFile.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
@@ skipping 243 matching lines @@
         assertEquals(numMinidumps, uploadedFiles.length);
         return uploadedFiles;
     }
 
     private File createMinidumpFileInCrashDir(String name) throws IOException {
         File minidumpFile = new File(mCrashDir, name);
         setUpMinidumpFile(minidumpFile, BOUNDARY);
         return minidumpFile;
     }
 }