AW: ensure we respect user opt-out for SafeBrowsing

This separates Safe Browsing enabling into 3 requirements:

 1. The user must opt into the feature
 2. The app can enable/disable via WebSettings#setSafeBrowsingEnabled
 3. The app can enable via manifest flag

This adds tests to ensure that (1) has the highest priority. We already
have tests to ensure that (2) has higher priority than (3).

The CLI switch is treated equivalently to the manifest flag (same behavior
as before).

This also renames AwContentsStatics#{get,set}SafeBrowsingEnabled ->
{get,set}SafeBrowsingEnabledByManifest. This is to clarify what this
function actually does. The old method can be removed once downstream
code no longer uses it.

Bug: 731927
Test: run_webview_instrumentation_test_apk -f SafeBrowsingTest#testSafeBrowsingUserOptOutOverrides*
Change-Id: Id1d1a41f503b259addda14b027df0b3c18325b9c
Reviewed-on: https://chromium-review.googlesource.com/572431
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Selim Gurun <sgurun@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487643}

android_webview/browser/aw_contents_statics.cc

@@ -80,15 +80,16 @@ ScopedJavaLocalRef<jstring> GetProductVersion(JNIEnv* env,
 }
 
 // static
-jboolean GetSafeBrowsingEnabled(JNIEnv* env, const JavaParamRef<jclass>&) {
-  return AwSafeBrowsingConfigHelper::GetSafeBrowsingEnabled();
+jboolean GetSafeBrowsingEnabledByManifest(JNIEnv* env,
+                                          const JavaParamRef<jclass>&) {
+  return AwSafeBrowsingConfigHelper::GetSafeBrowsingEnabledByManifest();
 }
 
 // static
-void SetSafeBrowsingEnabled(JNIEnv* env,
-                            const JavaParamRef<jclass>&,
-                            jboolean enable) {
-  AwSafeBrowsingConfigHelper::SetSafeBrowsingEnabled(enable);
+void SetSafeBrowsingEnabledByManifest(JNIEnv* env,
+                                      const JavaParamRef<jclass>&,
+                                      jboolean enable) {
+  AwSafeBrowsingConfigHelper::SetSafeBrowsingEnabledByManifest(enable);
 }
 
 // static

android_webview/browser/aw_safe_browsing_config_helper.cc

@@ -9,24 +9,26 @@
 #include "base/synchronization/lock.h"
 
 namespace {
-// g_safebrowsing_enabled can be set and read from different threads.
-base::LazyInstance<base::Lock>::Leaky g_safebrowsing_enabled_lock =
+// g_safebrowsing_enabled_by_manifest can be set and read from different
+// threads.
+base::LazyInstance<base::Lock>::Leaky g_safebrowsing_enabled_by_manifest_lock =
     LAZY_INSTANCE_INITIALIZER;
-bool g_safebrowsing_enabled = false;
+bool g_safebrowsing_enabled_by_manifest = false;
 }  // namespace
 
 namespace android_webview {
 
 // static
-void AwSafeBrowsingConfigHelper::SetSafeBrowsingEnabled(bool enabled) {
-  base::AutoLock lock(g_safebrowsing_enabled_lock.Get());
-  g_safebrowsing_enabled = enabled;
+void AwSafeBrowsingConfigHelper::SetSafeBrowsingEnabledByManifest(
+    bool enabled) {
+  base::AutoLock lock(g_safebrowsing_enabled_by_manifest_lock.Get());
+  g_safebrowsing_enabled_by_manifest = enabled;
 }
 
 // static
-bool AwSafeBrowsingConfigHelper::GetSafeBrowsingEnabled() {
-  base::AutoLock lock(g_safebrowsing_enabled_lock.Get());
-  return g_safebrowsing_enabled;
+bool AwSafeBrowsingConfigHelper::GetSafeBrowsingEnabledByManifest() {
+  base::AutoLock lock(g_safebrowsing_enabled_by_manifest_lock.Get());
+  return g_safebrowsing_enabled_by_manifest;
 }
 
 }  // namespace android_webview

android_webview/browser/aw_safe_browsing_config_helper.h

@@ -11,8 +11,8 @@ namespace android_webview {
 
 class AwSafeBrowsingConfigHelper {
  public:
-  static bool GetSafeBrowsingEnabled();
-  static void SetSafeBrowsingEnabled(bool enabled);
+  static bool GetSafeBrowsingEnabledByManifest();
+  static void SetSafeBrowsingEnabledByManifest(bool enabled);
 
  private:
   AwSafeBrowsingConfigHelper();

android_webview/java/src/org/chromium/android_webview/AwContentsStatics.java

@@ -89,12 +89,22 @@ public class AwContentsStatics {
     }
 
     // Can be called from any thread.
+    public static boolean getSafeBrowsingEnabledByManifest() {
+        return nativeGetSafeBrowsingEnabledByManifest();
+    }
+
+    public static void setSafeBrowsingEnabledByManifest(boolean enable) {
+        nativeSetSafeBrowsingEnabledByManifest(enable);
+    }
+
+    // TODO(ntfschr): remove this when downstream no longer depends on it
     public static boolean getSafeBrowsingEnabled() {
-        return nativeGetSafeBrowsingEnabled();
+        return getSafeBrowsingEnabledByManifest();
     }
 
+    // TODO(ntfschr): remove this when downstream no longer depends on it
     public static void setSafeBrowsingEnabled(boolean enable) {
-        nativeSetSafeBrowsingEnabled(enable);
+        setSafeBrowsingEnabledByManifest(enable);
     }
 
     public static void setSafeBrowsingWhiteList(String[] urls) {
@@ -161,8 +171,8 @@ public class AwContentsStatics {
     private static native String nativeGetProductVersion();
     private static native void nativeSetServiceWorkerIoThreadClient(
             AwContentsIoThreadClient ioThreadClient, AwBrowserContext browserContext);
-    private static native boolean nativeGetSafeBrowsingEnabled();
-    private static native void nativeSetSafeBrowsingEnabled(boolean enable);
+    private static native boolean nativeGetSafeBrowsingEnabledByManifest();
+    private static native void nativeSetSafeBrowsingEnabledByManifest(boolean enable);
     private static native void nativeSetSafeBrowsingWhiteList(String[] urls);
     private static native void nativeSetCheckClearTextPermitted(boolean permitted);
     private static native String nativeFindAddress(String addr);

android_webview/java/src/org/chromium/android_webview/AwSafeBrowsingConfigHelper.java

@@ -21,14 +21,16 @@ public class AwSafeBrowsingConfigHelper {
 
     private static final String OPT_IN_META_DATA_STR = "android.webkit.WebView.EnableSafeBrowsing";
 
+    private static boolean sSafeBrowsingUserOptIn;
+
     public static void maybeInitSafeBrowsingFromSettings(final Context appContext) {
-        if (CommandLine.getInstance().hasSwitch(AwSwitches.WEBVIEW_ENABLE_SAFEBROWSING_SUPPORT)
-                || appHasOptedIn(appContext)) {
-            // Assume safebrowsing on by default initially. If GMS is available, we later use
-            // isVerifyAppsEnabled() to check if "Scan device for security threats" has been checked
-            // by the user.
-            AwContentsStatics.setSafeBrowsingEnabled(true);
-        }
+        AwContentsStatics.setSafeBrowsingEnabledByManifest(
+                CommandLine.getInstance().hasSwitch(AwSwitches.WEBVIEW_ENABLE_SAFEBROWSING_SUPPORT)
+                || appHasOptedIn(appContext));
+        // Assume safebrowsing on by default initially. If GMS is available, we later use
+        // isVerifyAppsEnabled() to check if "Scan device for security threats" has been checked by
+        // the user.
+        setSafeBrowsingUserOptIn(true);
     }
 
     private static boolean appHasOptedIn(Context appContext) {
@@ -49,6 +51,15 @@ public class AwSafeBrowsingConfigHelper {
         }
     }
 
+    // Can be called from any thread.
+    public static boolean getSafeBrowsingUserOptIn() {
+        return sSafeBrowsingUserOptIn;
+    }
+
+    public static void setSafeBrowsingUserOptIn(boolean optin) {
+        sSafeBrowsingUserOptIn = optin;
+    }
+
     // Not meant to be instantiated.
     private AwSafeBrowsingConfigHelper() {}
 }

android_webview/java/src/org/chromium/android_webview/AwSettings.java

@@ -92,7 +92,7 @@ public class AwSettings {
     // Although this bit is stored on AwSettings it is actually controlled via the CookieManager.
     private boolean mAcceptThirdPartyCookies;
 
-    // if null, default to AwContentsStatics.getSafeBrowsingEnabled()
+    // if null, default to AwContentsStatics.getSafeBrowsingEnabledByManifest()
     private Boolean mSafeBrowsingEnabled;
 
     private final boolean mSupportLegacyQuirks;
@@ -357,8 +357,9 @@ public class AwSettings {
      */
     public boolean getSafeBrowsingEnabled() {
         synchronized (mAwSettingsLock) {
+            if (!AwSafeBrowsingConfigHelper.getSafeBrowsingUserOptIn()) return false;
             if (mSafeBrowsingEnabled == null) {
-                return AwContentsStatics.getSafeBrowsingEnabled();
+                return AwContentsStatics.getSafeBrowsingEnabledByManifest();
             }
             return mSafeBrowsingEnabled;
         }

android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java

@@ -21,6 +21,7 @@ import org.chromium.android_webview.AwContents.NativeDrawGLFunctorFactory;
 import org.chromium.android_webview.AwContentsClient;
 import org.chromium.android_webview.AwContentsClient.AwWebResourceRequest;
 import org.chromium.android_webview.AwContentsStatics;
+import org.chromium.android_webview.AwSafeBrowsingConfigHelper;
 import org.chromium.android_webview.AwSafeBrowsingConversionHelper;
 import org.chromium.android_webview.AwSafeBrowsingResponse;
 import org.chromium.android_webview.AwSettings;
@@ -761,4 +762,26 @@ public class SafeBrowsingTest extends AwTestBase {
         assertEquals(AwSafeBrowsingConversionHelper.SAFE_BROWSING_THREAT_MALWARE,
                 mContentsClient.getLastThreatType());
     }
+
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    @CommandLineFlags.Add(AwSwitches.WEBVIEW_ENABLE_SAFEBROWSING_SUPPORT)
+    public void testSafeBrowsingUserOptOutOverridesManifest() throws Throwable {
+        AwSafeBrowsingConfigHelper.setSafeBrowsingUserOptIn(false);
+        loadGreenPage();
+        final String responseUrl = mTestServer.getURL(MALWARE_HTML_PATH);
+        loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), responseUrl);
+        assertTargetPageHasLoaded(MALWARE_PAGE_BACKGROUND_COLOR);
+    }
+
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testSafeBrowsingUserOptOutOverridesPerWebView() throws Throwable {
+        AwSafeBrowsingConfigHelper.setSafeBrowsingUserOptIn(false);
+        getAwSettingsOnUiThread(mAwContents).setSafeBrowsingEnabled(true);
+        loadGreenPage();
+        final String responseUrl = mTestServer.getURL(MALWARE_HTML_PATH);
+        loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), responseUrl);
+        assertTargetPageHasLoaded(MALWARE_PAGE_BACKGROUND_COLOR);
+    }
 }