[Android Crash Reporting] Componentize MinidumpUploadImpl.java

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

Index: android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java
index a12000f9debc175f1f2b9603e33640edd9c70922..cc503f15449ecf926292142b803957687b9c7bb7 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java
@@ -12,8 +12,8 @@ import android.support.test.filters.MediumTest;
 import android.webkit.ValueCallback;
 
 import org.chromium.android_webview.PlatformServiceBridge;
+import org.chromium.android_webview.crash.AwMinidumpUploaderDelegate;
 import org.chromium.android_webview.crash.CrashReceiverService;
-import org.chromium.android_webview.crash.MinidumpUploaderImpl;
 import org.chromium.base.FileUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.components.minidump_uploader.CrashFileManager;
@@ -21,6 +21,8 @@ import org.chromium.components.minidump_uploader.CrashTestCase;
 import org.chromium.components.minidump_uploader.MinidumpUploadCallable;
 import org.chromium.components.minidump_uploader.MinidumpUploadCallableTest;
 import org.chromium.components.minidump_uploader.MinidumpUploader;
+import org.chromium.components.minidump_uploader.MinidumpUploaderDelegate;
+import org.chromium.components.minidump_uploader.MinidumpUploaderImpl;
 import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager;
 import org.chromium.components.minidump_uploader.util.HttpURLConnectionFactory;
 
@@ -73,28 +75,46 @@ public class MinidumpUploaderTest extends CrashTestCase {
         }
     }
 
+    private static class TestMinidumpUploaderDelegate extends AwMinidumpUploaderDelegate {
+        private final CrashReportingPermissionManager mPermissionManager;
+
+        TestMinidumpUploaderDelegate(
+                Context context, CrashReportingPermissionManager permissionManager) {
+            super(context);
+            mPermissionManager = permissionManager;
+        }
+
+        @Override
+        public PlatformServiceBridge createPlatformServiceBridge() {
+            return new TestPlatformServiceBridge(true /* canUseGms */,
+                    mPermissionManager.isUsageAndCrashReportingPermittedByUser());
+        }
+    }
+
     private static class TestMinidumpUploaderImpl extends MinidumpUploaderImpl {
-        private CrashReportingPermissionManager mPermissionManager;
+        private final CrashReportingPermissionManager mPermissionManager;
 
-        TestMinidumpUploaderImpl(Context context, CrashReportingPermissionManager permissionManager,
-                boolean cleanOutMinidumps) {
-            super(context, cleanOutMinidumps);
+        TestMinidumpUploaderImpl(MinidumpUploaderDelegate delegate,
+                CrashReportingPermissionManager permissionManager) {
+            super(delegate);
             mPermissionManager = permissionManager;
         }
 
         @Override
+        public CrashFileManager createCrashFileManager() {
+            return new CrashFileManager(getDelegateForTesting().createCrashDir()) {
+                @Override
+                public void cleanOutAllNonFreshMinidumpFiles() {}

[gsennton, 2017/02/27 18:54:55]

So by overriding this method here we avoid having to pass a boolean to
MinidumpUploaderImpl? Neat :)

[Ilya Sherman, 2017/02/28 03:54:13]

Yep! =)

+            };
+        }
+
+        @Override
         public MinidumpUploadCallable createMinidumpUploadCallable(
                 File minidumpFile, File logfile) {
             return new MinidumpUploadCallable(minidumpFile, logfile,
                     new MinidumpUploadCallableTest.TestHttpURLConnectionFactory(),
                     mPermissionManager);
         }
-
-        @Override
-        public PlatformServiceBridge createPlatformServiceBridge() {
-            return new TestPlatformServiceBridge(true /* canUseGms */,
-                    mPermissionManager.isUsageAndCrashReportingPermittedByUser());
-        }
     }
 
     /**
@@ -114,9 +134,9 @@ public class MinidumpUploaderTest extends CrashTestCase {
                         mIsEnabledForTests = false;
                     }
                 };
-        MinidumpUploader minidumpUploader =
-                new TestMinidumpUploaderImpl(getInstrumentation().getTargetContext(), permManager,
-                        false /* cleanOutMinidumps */);
+        MinidumpUploaderDelegate delegate = new TestMinidumpUploaderDelegate(
+                getInstrumentation().getTargetContext(), permManager);
+        MinidumpUploader minidumpUploader = new TestMinidumpUploaderImpl(delegate, permManager);
 
         File firstFile = createMinidumpFileInCrashDir("1_abc.dmp0");
         File secondFile = createMinidumpFileInCrashDir("12_abc.dmp0");
@@ -206,9 +226,9 @@ public class MinidumpUploaderTest extends CrashTestCase {
                 new MockCrashReportingPermissionManager() {
                     { mIsEnabledForTests = true; }
                 };
-        MinidumpUploader minidumpUploader =
-                new TestMinidumpUploaderImpl(getInstrumentation().getTargetContext(), permManager,
-                        false /* cleanOutMinidumps */);
+        MinidumpUploaderDelegate delegate = new TestMinidumpUploaderDelegate(
+                getInstrumentation().getTargetContext(), permManager);
+        MinidumpUploader minidumpUploader = new TestMinidumpUploaderImpl(delegate, permManager);
 
         File firstFile = createMinidumpFileInCrashDir("1_abc.dmp0");
         File secondFile = createMinidumpFileInCrashDir("12_abcd.dmp0");
@@ -231,7 +251,13 @@ public class MinidumpUploaderTest extends CrashTestCase {
 
     private static MinidumpUploaderImpl createCallableListMinidumpUploader(Context context,
             final List<MinidumpUploadCallableCreator> callables, final boolean userPermitted) {
-        return new MinidumpUploaderImpl(context, false /* cleanOutMinidumps */) {
+        MinidumpUploaderDelegate delegate = new AwMinidumpUploaderDelegate(context) {
+            @Override
+            public PlatformServiceBridge createPlatformServiceBridge() {
+                return new TestPlatformServiceBridge(true /* canUseGms*/, userPermitted);
+            }
+        };
+        return new TestMinidumpUploaderImpl(delegate, null) {
             private int mIndex = 0;
 
             @Override
@@ -242,10 +268,6 @@ public class MinidumpUploaderTest extends CrashTestCase {
                 }
                 return callables.get(mIndex++).createCallable(minidumpFile, logfile);
             }
-            @Override
-            public PlatformServiceBridge createPlatformServiceBridge() {
-                return new TestPlatformServiceBridge(true /* canUseGms*/, userPermitted);
-            }
         };
     }
 
@@ -359,8 +381,16 @@ public class MinidumpUploaderTest extends CrashTestCase {
                     { mIsEnabledForTests = true; }
                 };
         final CountDownLatch stopStallingLatch = new CountDownLatch(1);
-        MinidumpUploaderImpl minidumpUploader = new MinidumpUploaderImpl(
-                getInstrumentation().getTargetContext(), false /* cleanOutMinidumps */) {
+        MinidumpUploaderDelegate delegate = new TestMinidumpUploaderDelegate(
+                getInstrumentation().getTargetContext(), permManager) {
+            @Override
+            public PlatformServiceBridge createPlatformServiceBridge() {
+                return new TestPlatformServiceBridge(
+                        true /* canUseGms*/, permManager.isUsageAndCrashReportingPermittedByUser());
+            }
+        };
+        MinidumpUploaderImpl minidumpUploader = new TestMinidumpUploaderImpl(
+                delegate, permManager) {
             @Override
             public MinidumpUploadCallable createMinidumpUploadCallable(
                     File minidumpFile, File logfile) {
@@ -368,11 +398,6 @@ public class MinidumpUploaderTest extends CrashTestCase {
                         new StallingHttpUrlConnectionFactory(stopStallingLatch, successfulUpload),
                         permManager);
             }
-            @Override
-            public PlatformServiceBridge createPlatformServiceBridge() {
-                return new TestPlatformServiceBridge(
-                        true /* canUseGms*/, permManager.isUsageAndCrashReportingPermittedByUser());
-            }
         };
 
         File firstFile = createMinidumpFileInCrashDir("123_abc.dmp0");
@@ -465,14 +490,14 @@ public class MinidumpUploaderTest extends CrashTestCase {
     }
 
     /**
-     * MinidumpUploaderImpl sub-class that uses MinidumpUploaderImpl's implementation of
+     * MinidumpUploaderDelegate sub-class that uses MinidumpUploaderDelegate's implementation of
      * CrashReportingPermissionManager.isUsageAndCrashReportingPermittedByUser().
      */
-    private static class WebViewUserConsentMinidumpUploaderImpl extends MinidumpUploaderImpl {
-        boolean mUserConsent;
-        WebViewUserConsentMinidumpUploaderImpl(
-                Context context, boolean cleanOutMinidumps, boolean userConsent) {
-            super(context, cleanOutMinidumps);
+    private static class WebViewUserConsentMinidumpUploaderDelegate
+            extends AwMinidumpUploaderDelegate {
+        private final boolean mUserConsent;
+        WebViewUserConsentMinidumpUploaderDelegate(Context context, boolean userConsent) {
+            super(context);
             mUserConsent = userConsent;
         }
         @Override
@@ -480,16 +505,9 @@ public class MinidumpUploaderTest extends CrashTestCase {
             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() {
+        public CrashReportingPermissionManager createCrashReportingPermissionManager() {
             final CrashReportingPermissionManager realPermissionManager =
-                    super.createWebViewCrashReportingManager();
+                    super.createCrashReportingPermissionManager();
             return new MockCrashReportingPermissionManager() {
                 {
                     // This setup ensures we depend on
@@ -514,9 +532,10 @@ public class MinidumpUploaderTest extends CrashTestCase {
     }
 
     private void testPlatformServicesBridgeIsUsed(final boolean userConsent) throws IOException {
-        MinidumpUploader minidumpUploader =
-                new WebViewUserConsentMinidumpUploaderImpl(getInstrumentation().getTargetContext(),
-                        false /* cleanOutMinidumps */, userConsent);
+        MinidumpUploaderDelegate delegate = new WebViewUserConsentMinidumpUploaderDelegate(
+                getInstrumentation().getTargetContext(), userConsent);
+        MinidumpUploader minidumpUploader = new TestMinidumpUploaderImpl(
+                delegate, delegate.createCrashReportingPermissionManager());
 
         File firstFile = createMinidumpFileInCrashDir("1_abc.dmp0");
         File secondFile = createMinidumpFileInCrashDir("12_abcd.dmp0");
@@ -657,9 +676,9 @@ public class MinidumpUploaderTest extends CrashTestCase {
                 new MockCrashReportingPermissionManager() {
                     { mIsEnabledForTests = true; }
                 };
-        MinidumpUploader minidumpUploader =
-                new TestMinidumpUploaderImpl(getInstrumentation().getTargetContext(), permManager,
-                        false /* cleanOutMinidumps */);
+        MinidumpUploaderDelegate delegate = new TestMinidumpUploaderDelegate(
+                getInstrumentation().getTargetContext(), permManager);
+        MinidumpUploader minidumpUploader = new TestMinidumpUploaderImpl(delegate, permManager);
 
         uploadMinidumpsSync(minidumpUploader, false /* expectReschedule */);
         // Ensure there are no minidumps left to upload.