Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2002)

Unified Diff: android_webview/java/src/org/chromium/android_webview/crash/CrashReceiverService.java

Issue 2628863004: [Android WebView] Ensure we have user consent before uploading minidumps (Closed)
Patch Set: Check user consent before copying minidumps, delete minidumps in app dir if no consent, add unit te… Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: android_webview/java/src/org/chromium/android_webview/crash/CrashReceiverService.java
diff --git a/android_webview/java/src/org/chromium/android_webview/crash/CrashReceiverService.java b/android_webview/java/src/org/chromium/android_webview/crash/CrashReceiverService.java
index f3b994ef35a72f4493662f589bd7f807a26eb0f6..b7b91c619b3cc90bdcc38708ddb70a31592523b1 100644
--- a/android_webview/java/src/org/chromium/android_webview/crash/CrashReceiverService.java
+++ b/android_webview/java/src/org/chromium/android_webview/crash/CrashReceiverService.java
@@ -15,7 +15,10 @@ import android.os.Binder;
import android.os.Build;
import android.os.IBinder;
import android.os.ParcelFileDescriptor;
+import android.webkit.ValueCallback;
+import org.chromium.android_webview.PlatformServiceBridge;
+import org.chromium.android_webview.command_line.CommandLineUtil;
import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting;
import org.chromium.components.minidump_uploader.CrashFileManager;
@@ -44,26 +47,89 @@ public class CrashReceiverService extends Service {
private Object mCopyingLock = new Object();
private boolean mIsCopying = false;
- // same switch as kEnableCrashReporterForTesting in //base/base_switches.h
- static final String CRASH_UPLOADS_ENABLED_FOR_TESTING_SWITCH =
- "enable-crash-reporter-for-testing";
+ private static final int CONSENT_NOT_RETURNED = 0;
+ private static final int CONSENT_OK = 1;
+ private static final int CONSENT_NOT_OK = 2;
+
+ private Object mConsentLock = new Object();
+ private int mConsentValueState = CONSENT_NOT_RETURNED;
sgurun-gerrit only 2017/01/24 08:58:49 nit: rename to mConsentValue
gsennton 2017/01/24 10:41:48 Done.
+
+ private void resetConsentState() {
+ synchronized (mConsentLock) {
+ mConsentValueState = CONSENT_NOT_RETURNED;
+ }
+ }
+
+ private void setConsentValue(boolean ok) {
+ synchronized (mConsentLock) {
+ assert mConsentValueState == CONSENT_NOT_RETURNED;
+ mConsentValueState = ok ? CONSENT_OK : CONSENT_NOT_OK;
+ mConsentLock.notifyAll();
+ }
+ }
+
+ /**
+ * Wait until a consent value was received, and return that value.
+ */
+ private boolean getConsentValue() {
sgurun-gerrit only 2017/01/24 08:58:49 would it be better to keep the methods that provid
gsennton 2017/01/24 10:41:47 Done.
+ synchronized (mConsentLock) {
+ while (mConsentValueState == CONSENT_NOT_RETURNED) {
+ try {
+ mConsentLock.wait();
+ } catch (InterruptedException e) {
+ break;
+ }
+ }
+ return mConsentValueState == CONSENT_OK;
+ }
+ }
@Override
public void onCreate() {
super.onCreate();
- SynchronizedWebViewCommandLine.initOnSeparateThread();
+ getSynchronizedWebViewCommandLine().initOnSeparateThread();
+
+ resetConsentState();
+ PlatformServiceBridge platformServiceBridge = getPlaformServiceBridge();
+ if (platformServiceBridge.canUseGms()) {
+ platformServiceBridge.queryMetricsSetting(new ValueCallback<Boolean>() {
+ @Override
+ public void onReceiveValue(Boolean enabled) {
+ setConsentValue(enabled);
+ }
+ });
+ } else {
+ setConsentValue(false);
+ }
+ }
+
+ @VisibleForTesting
+ final boolean isCrashCopyingEnabled() {
+ if (getSynchronizedWebViewCommandLine().hasSwitch(
+ CommandLineUtil.CRASH_UPLOADS_ENABLED_FOR_TESTING_SWITCH)) {
+ return true;
+ }
+ return getConsentValue();
+ }
+
+ @VisibleForTesting
+ SynchronizedWebViewCommandLine getSynchronizedWebViewCommandLine() {
+ return SynchronizedWebViewCommandLine.getInstance();
+ }
+
+ @VisibleForTesting
+ PlatformServiceBridge getPlaformServiceBridge() {
+ return PlatformServiceBridge.getInstance(this);
}
private final ICrashReceiverService.Stub mBinder = new ICrashReceiverService.Stub() {
@Override
public void transmitCrashes(ParcelFileDescriptor[] fileDescriptors) {
- // TODO(gsennton): replace this check with a check for Android Checkbox when we have
- // access to that value through GmsCore.
- if (!SynchronizedWebViewCommandLine.hasSwitch(
- CRASH_UPLOADS_ENABLED_FOR_TESTING_SWITCH)) {
+ if (!isCrashCopyingEnabled()) {
Log.i(TAG, "Crash reporting is not enabled, bailing!");
return;
}
+
int uid = Binder.getCallingUid();
performMinidumpCopyingSerially(
CrashReceiverService.this, uid, fileDescriptors, true /* scheduleUploads */);

Powered by Google App Engine
This is Rietveld 408576698