Skip to content

Commit

Permalink
[Android Crash Reporting] Componentize MinidumpUploadImpl.java
Browse files Browse the repository at this point in the history
BUG=694884
TEST=compile, android_webview_test_apk
R=gsennton@chromium.org

Review-Url: https://codereview.chromium.org/2709163008
Cr-Commit-Position: refs/heads/master@{#454390}
  • Loading branch information
ishermandom authored and Commit bot committed Mar 2, 2017
1 parent 5c97d3e commit 375b5de
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 133 deletions.
2 changes: 1 addition & 1 deletion android_webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,8 @@ android_library("android_webview_commandline_java") {
android_library("android_webview_crash_services_java") {
java_files = [
"java/src/org/chromium/android_webview/crash/AwMinidumpUploadJobService.java",
"java/src/org/chromium/android_webview/crash/AwMinidumpUploaderDelegate.java",
"java/src/org/chromium/android_webview/crash/CrashReceiverService.java",
"java/src/org/chromium/android_webview/crash/MinidumpUploaderImpl.java",
]
deps = [
":android_webview_commandline_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.chromium.base.ContextUtils;
import org.chromium.components.minidump_uploader.MinidumpUploadJobService;
import org.chromium.components.minidump_uploader.MinidumpUploader;
import org.chromium.components.minidump_uploader.MinidumpUploaderImpl;

/**
* Class that interacts with the Android JobScheduler to upload Minidumps at appropriate times.
Expand All @@ -31,6 +32,6 @@ protected MinidumpUploader createMinidumpUploader() {
// Ensure we can use ContextUtils later on (from the minidump_uploader component).
ContextUtils.initApplicationContext(this.getApplicationContext());

return new MinidumpUploaderImpl(this);
return new MinidumpUploaderImpl(new AwMinidumpUploaderDelegate(this));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// 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.crash;

import android.content.Context;
import android.net.ConnectivityManager;
import android.net.NetworkInfo;
import android.webkit.ValueCallback;

import org.chromium.android_webview.PlatformServiceBridge;
import org.chromium.android_webview.command_line.CommandLineUtil;
import org.chromium.base.CommandLine;
import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.components.minidump_uploader.MinidumpUploaderDelegate;
import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager;

import java.io.File;

/**
* Android Webview-specific implementations for minidump uploading logic.
*/
public class AwMinidumpUploaderDelegate implements MinidumpUploaderDelegate {
private final Context mContext;
private final ConnectivityManager mConnectivityManager;

private boolean mPermittedByUser = false;

@VisibleForTesting
public AwMinidumpUploaderDelegate(Context context) {
mContext = context;
mConnectivityManager =
(ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
}

@Override
public File createCrashDir() {
return CrashReceiverService.createWebViewCrashDir(mContext);
}

@Override
public CrashReportingPermissionManager createCrashReportingPermissionManager() {
return new CrashReportingPermissionManager() {
@Override
public boolean isClientInMetricsSample() {
// We will check whether the client is in the metrics sample before
// generating a minidump - so if no minidump is generated this code will
// never run and we don't need to check whether we are in the sample.
// TODO(gsennton): when we switch to using Finch for this value we should use the
// Finch value here as well.
return true;
}
@Override
public boolean isNetworkAvailableForCrashUploads() {
// JobScheduler will call onStopJob causing our upload to be interrupted when our
// network requirements no longer hold.
// TODO(isherman): This code should really be shared with Chrome. Chrome currently
// checks only whether the network is WiFi (or ethernet) vs. cellular. Most likely,
// Chrome should instead check whether the network is metered, as is done here.
NetworkInfo networkInfo = mConnectivityManager.getActiveNetworkInfo();
if (networkInfo == null || !networkInfo.isConnected()) return false;
return !mConnectivityManager.isActiveNetworkMetered();
}
@Override
public boolean isCrashUploadDisabledByCommandLine() {
return false;
}
/**
* This method is already represented by isClientInMetricsSample() and
* isNetworkAvailableForCrashUploads().
*/
@Override
public boolean isMetricsUploadPermitted() {
return true;
}
@Override
public boolean isUsageAndCrashReportingPermittedByUser() {
return mPermittedByUser;
}
@Override
public boolean isUploadEnabledForTests() {
// Note that CommandLine/CommandLineUtil are not thread safe. They are initialized
// on the main thread, but before the current worker thread started - so this thread
// will have seen the initialization of the CommandLine.
return CommandLine.getInstance().hasSwitch(
CommandLineUtil.CRASH_UPLOADS_ENABLED_FOR_TESTING_SWITCH);
}
};
}

@Override
public void prepareToUploadMinidumps(final Runnable startUploads) {
createPlatformServiceBridge().queryMetricsSetting(new ValueCallback<Boolean>() {
public void onReceiveValue(Boolean enabled) {
ThreadUtils.assertOnUiThread();
mPermittedByUser = enabled;
startUploads.run();
}
});
}

/**
* Utility method to allow us to test the logic of this class by injecting
* a test-specific PlatformServiceBridge.
*/
@VisibleForTesting
public PlatformServiceBridge createPlatformServiceBridge() {
return PlatformServiceBridge.getInstance(mContext);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@
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;
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;

Expand Down Expand Up @@ -73,15 +75,31 @@ public void queryMetricsSetting(ValueCallback<Boolean> callback) {
}
}

private static class TestMinidumpUploaderImpl extends MinidumpUploaderImpl {
private static class TestMinidumpUploaderDelegate extends AwMinidumpUploaderDelegate {
private final CrashReportingPermissionManager mPermissionManager;

TestMinidumpUploaderImpl(
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 final CrashReportingPermissionManager mPermissionManager;

TestMinidumpUploaderImpl(MinidumpUploaderDelegate delegate,
CrashReportingPermissionManager permissionManager) {
super(delegate);
mPermissionManager = permissionManager;
}

@Override
public CrashFileManager createCrashFileManager(File crashDir) {
return new CrashFileManager(crashDir) {
Expand All @@ -97,12 +115,6 @@ public MinidumpUploadCallable createMinidumpUploadCallable(
new MinidumpUploadCallableTest.TestHttpURLConnectionFactory(),
mPermissionManager);
}

@Override
public PlatformServiceBridge createPlatformServiceBridge() {
return new TestPlatformServiceBridge(true /* canUseGms */,
mPermissionManager.isUsageAndCrashReportingPermittedByUser());
}
}

/**
Expand All @@ -122,8 +134,9 @@ public void testFailUploadingMinidumps() throws IOException {
mIsEnabledForTests = false;
}
};
MinidumpUploader minidumpUploader =
new TestMinidumpUploaderImpl(getInstrumentation().getTargetContext(), permManager);
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");
Expand Down Expand Up @@ -213,8 +226,9 @@ public void testUploadingWithoutCrashDir() throws IOException {
new MockCrashReportingPermissionManager() {
{ mIsEnabledForTests = true; }
};
MinidumpUploader minidumpUploader =
new TestMinidumpUploaderImpl(getInstrumentation().getTargetContext(), permManager);
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");
Expand All @@ -237,7 +251,13 @@ private interface MinidumpUploadCallableCreator {

private static MinidumpUploaderImpl createCallableListMinidumpUploader(Context context,
final List<MinidumpUploadCallableCreator> callables, final boolean userPermitted) {
return new TestMinidumpUploaderImpl(context, null) {
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
Expand All @@ -248,10 +268,6 @@ public MinidumpUploadCallable createMinidumpUploadCallable(
}
return callables.get(mIndex++).createCallable(minidumpFile, logfile);
}
@Override
public PlatformServiceBridge createPlatformServiceBridge() {
return new TestPlatformServiceBridge(true /* canUseGms*/, userPermitted);
}
};
}

Expand Down Expand Up @@ -365,20 +381,23 @@ private void testCancellation(final boolean successfulUpload) throws IOException
{ mIsEnabledForTests = true; }
};
final CountDownLatch stopStallingLatch = new CountDownLatch(1);
MinidumpUploaderImpl minidumpUploader = new TestMinidumpUploaderImpl(
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) {
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");
Expand Down Expand Up @@ -471,37 +490,24 @@ public void testPlatformServicesBridgeIsUsedNoUserConsent() throws IOException {
}

/**
* 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 {
private static class WebViewUserConsentMinidumpUploaderDelegate
extends AwMinidumpUploaderDelegate {
private final boolean mUserConsent;
WebViewUserConsentMinidumpUploaderImpl(Context context, boolean userConsent) {
WebViewUserConsentMinidumpUploaderDelegate(Context context, boolean userConsent) {
super(context);
mUserConsent = userConsent;
}
@Override
public CrashFileManager createCrashFileManager(File crashDir) {
return new CrashFileManager(crashDir) {
@Override
public void cleanOutAllNonFreshMinidumpFiles() {}
};
}
@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() {
public CrashReportingPermissionManager createCrashReportingPermissionManager() {
final CrashReportingPermissionManager realPermissionManager =
super.createWebViewCrashReportingManager();
super.createCrashReportingPermissionManager();
return new MockCrashReportingPermissionManager() {
{
// This setup ensures we depend on
Expand All @@ -526,8 +532,10 @@ public boolean isUsageAndCrashReportingPermittedByUser() {
}

private void testPlatformServicesBridgeIsUsed(final boolean userConsent) throws IOException {
MinidumpUploader minidumpUploader = new WebViewUserConsentMinidumpUploaderImpl(
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");
Expand Down Expand Up @@ -668,8 +676,9 @@ private File[] copyAndUploadMinidumpsSync(CrashFileManager fileManager, File[][]
new MockCrashReportingPermissionManager() {
{ mIsEnabledForTests = true; }
};
MinidumpUploader minidumpUploader =
new TestMinidumpUploaderImpl(getInstrumentation().getTargetContext(), permManager);
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.
Expand Down
2 changes: 2 additions & 0 deletions components/minidump_uploader/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ android_library("minidump_uploader_java") {
"android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadCallable.java",
"android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadJobService.java",
"android/java/src/org/chromium/components/minidump_uploader/MinidumpUploader.java",
"android/java/src/org/chromium/components/minidump_uploader/MinidumpUploaderDelegate.java",
"android/java/src/org/chromium/components/minidump_uploader/MinidumpUploaderImpl.java",
"android/java/src/org/chromium/components/minidump_uploader/util/CrashReportingPermissionManager.java",
"android/java/src/org/chromium/components/minidump_uploader/util/HttpURLConnectionFactory.java",
"android/java/src/org/chromium/components/minidump_uploader/util/HttpURLConnectionFactoryImpl.java",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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.components.minidump_uploader;

import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager;

import java.io.File;

/**
* Interface for embedder-specific implementations for uploading minidumps.
*/
public interface MinidumpUploaderDelegate {
/**
* Creates the directory in which the embedder will store its minidumps.
* @return A reference to the created directory, or null if the creation failed.
*/
File createCrashDir();

/**
* Creates the permission manager used to evaluate whether uploading should be allowed.
* @return The permission manager.
*/
CrashReportingPermissionManager createCrashReportingPermissionManager();

/**
* Performs any pre-work necessary for uploading minidumps, then calls the {@param startUploads}
* continuation to initiate uploading the minidumps.
* @param startUploads The continuation to call once any necessary pre-work is completed.
*/
void prepareToUploadMinidumps(Runnable startUploads);
}
Loading

0 comments on commit 375b5de

Please sign in to comment.