[Android WebView] Ensure we have user consent before uploading minidumps

android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java

Index: android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
index b7fb06b95604bf6e1b142f2741731eb1ae1a8c55..ae2556fb628477d08a0474f99d25ca9174d2ee65 100644
--- a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
+++ b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
@@ -16,7 +16,6 @@ import android.net.Uri;
 import android.os.Build;
 import android.os.Looper;
 import android.os.Process;
-import android.os.StrictMode;
 import android.os.UserManager;
 import android.provider.Settings;
 import android.util.Log;
@@ -49,6 +48,7 @@ import org.chromium.android_webview.AwSettings;
 import org.chromium.android_webview.HttpAuthDatabase;
 import org.chromium.android_webview.PlatformServiceBridge;
 import org.chromium.android_webview.ResourcesContextWrapperFactory;
+import org.chromium.android_webview.command_line.CommandLineUtil;
 import org.chromium.base.BuildConfig;
 import org.chromium.base.CommandLine;
 import org.chromium.base.ContextUtils;
@@ -58,7 +58,6 @@ import org.chromium.base.PathService;
 import org.chromium.base.PathUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.TraceEvent;
-import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.base.library_loader.NativeLibraries;
@@ -83,7 +82,6 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
 
     private static final String CHROMIUM_PREFS_NAME = "WebViewChromiumPrefs";
     private static final String VERSION_CODE_PREF = "lastVersionCodeUsed";
-    private static final String COMMAND_LINE_FILE = "/data/local/tmp/webview-command-line";
     private static final String HTTP_AUTH_DATABASE_FILE = "http_auth.db";
 
     private class WebViewChromiumRunQueue {
@@ -197,7 +195,6 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
         initialize(WebViewDelegateFactory.createProxyDelegate(delegate));
     }
 
-    @SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
     private void initialize(WebViewDelegate webViewDelegate) {
         mWebViewDelegate = webViewDelegate;
         Context ctx = mWebViewDelegate.getApplication().getApplicationContext();
@@ -215,14 +212,7 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
         // WebView needs to make sure to always use the wrapped application context.
         ContextUtils.initApplicationContext(ResourcesContextWrapperFactory.get(ctx));
 
-        if (isBuildDebuggable()) {
-            // Suppress the StrictMode violation as this codepath is only hit on debuggable builds.
-            StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
-            CommandLine.initFromFile(COMMAND_LINE_FILE);
-            StrictMode.setThreadPolicy(oldPolicy);
-        } else {
-            CommandLine.init(null);
-        }
+        CommandLineUtil.initCommandLine();
 
         if (Settings.Global.getInt(ContextUtils.getApplicationContext().getContentResolver(),
                     Settings.Global.WEBVIEW_MULTIPROCESS, 0)
@@ -270,10 +260,6 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
         }
     }
 
-    private static boolean isBuildDebuggable() {
-        return !Build.TYPE.equals("user");
-    }
-
     /**
      * Both versionCodes should be from a WebView provider package implemented by Chromium.
      * VersionCodes from other kinds of packages won't make any sense in this method.
@@ -406,17 +392,27 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
         final boolean isExternalService = true;
         AwBrowserProcess.configureChildProcessLauncher(webViewPackageName, isExternalService);
         AwBrowserProcess.start();
-        AwBrowserProcess.handleMinidumps(webViewPackageName);
 
+        final boolean enableMinidumpUploadingForTesting = CommandLine.getInstance().hasSwitch(

[sgurun-gerrit only, 2017/01/25 19:42:06]

you can inline this to if

+                CommandLineUtil.CRASH_UPLOADS_ENABLED_FOR_TESTING_SWITCH);
+        if (enableMinidumpUploadingForTesting) {
+            AwBrowserProcess.handleMinidumps(webViewPackageName, true /* enabled */);
+        }
+
+        // Actions conditioned on whether the Android Checkbox is toggled on
         PlatformServiceBridge.getInstance(context)

[sgurun-gerrit only, 2017/01/25 19:42:06]

move to else as no need to do this if command line flag already exists

[paulmiller, 2017/01/25 21:46:31]

This is still needed for AwMetricsServiceClient.setConsentSetting().

[gsennton, 2017/01/26 10:24:50]

I'm doing all of this because we wanted to merge the
AwMetricsServiceClient.setConsentSetting() call with the handleMinidumps call
into one queryMetricsSetting call (to not call GMS twice). We can't move all of
this into an else since AwMetricsServiceClient.setConsentSetting() shouldn't be
conditional on enableMinidumpUploadingForTesting. Or am I missing something?

This comment holds for all three of your (Selim's) comments in this file :)

                 .queryMetricsSetting(new ValueCallback<Boolean>() {
                     public void onReceiveValue(Boolean enabled) {
                         ThreadUtils.assertOnUiThread();
                         AwMetricsServiceClient.setConsentSetting(context, enabled);
+
+                        if (!enableMinidumpUploadingForTesting) {

[sgurun-gerrit only, 2017/01/25 19:42:06]

after moving to else, remove that.

+                            AwBrowserProcess.handleMinidumps(webViewPackageName, enabled);
+                        }
                     }
                 });
 
-        if (isBuildDebuggable()) {
+        if (CommandLineUtil.isBuildDebuggable()) {
             setWebContentsDebuggingEnabled(true);
         }
 
@@ -512,7 +508,7 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
                     @Override
                     public void setWebContentsDebuggingEnabled(boolean enable) {
                         // Web Contents debugging is always enabled on debug builds.
-                        if (!isBuildDebuggable()) {
+                        if (!CommandLineUtil.isBuildDebuggable()) {
                             WebViewChromiumFactoryProvider.this.setWebContentsDebuggingEnabled(
                                     enable);
                         }