From 35ef18f1aad291729f59c9c7acb7af92996c2513 Mon Sep 17 00:00:00 2001
From: Chidera Olibie <[email protected]>
Date: Tue, 09 Jun 2026 08:55:02 -0700
Subject: [PATCH] Only update cached SB/RCB preference if non-null result received

This CL updates Safe Browsing and Restricted Content Blocking config
helpers to only update their cached preferences (and log associated
histograms) when GMS queries return a non-null result (i.e. not on
timeout or error).

It also updates corresponding histograms.xml to document this behavior,
and adds test cases to verify it.

TAG=agy
CONV=151e4036-1bf7-4756-a458-17bb9a74d65f

Bug: 508162988, 517335150
Change-Id: I03aa75a1043647a696b0fdc479af55184f92fef1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7899996
Auto-Submit: Chidera Olibie <[email protected]>
Reviewed-by: Nate Fischer <[email protected]>
Reviewed-by: Daniel Rubery <[email protected]>
Reviewed-by: Michael van Ouwerkerk <[email protected]>
Commit-Queue: Daniel Rubery <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1644001}
---

diff --git a/android_webview/java/src/org/chromium/android_webview/common/AwSupervisedUserUrlClassifierDelegate.java b/android_webview/java/src/org/chromium/android_webview/common/AwSupervisedUserUrlClassifierDelegate.java
index 2e86f943..ca7066bb2 100644
--- a/android_webview/java/src/org/chromium/android_webview/common/AwSupervisedUserUrlClassifierDelegate.java
+++ b/android_webview/java/src/org/chromium/android_webview/common/AwSupervisedUserUrlClassifierDelegate.java
@@ -6,6 +6,7 @@
 
 import org.chromium.base.Callback;
 import org.chromium.build.annotations.NullMarked;
+import org.chromium.build.annotations.Nullable;
 import org.chromium.url.GURL;
 
 /**
@@ -35,5 +36,5 @@
      * <p>callback.onResult(false) - indicates the user does not require restricted content
      * blocking. callback.onResult(true) - indicates the user requires restricted content blocking.
      */
-    void needsRestrictedContentBlocking(final Callback<Boolean> callback);
+    void needsRestrictedContentBlocking(final Callback<@Nullable Boolean> callback);
 }
diff --git a/android_webview/java/src/org/chromium/android_webview/common/PlatformServiceBridge.java b/android_webview/java/src/org/chromium/android_webview/common/PlatformServiceBridge.java
index df35c5a..0c829bb1 100644
--- a/android_webview/java/src/org/chromium/android_webview/common/PlatformServiceBridge.java
+++ b/android_webview/java/src/org/chromium/android_webview/common/PlatformServiceBridge.java
@@ -63,7 +63,7 @@
     }
 
     // Overriding implementations may call "callback" asynchronously, on any thread.
-    public void querySafeBrowsingUserConsent(final Callback<Boolean> callback) {
+    public void querySafeBrowsingUserConsent(final Callback<@Nullable Boolean> callback) {
         // User opt-in preference depends on a SafetyNet API. In purely upstream builds (which don't
         // communicate with GMS), assume the user has not opted in.
         callback.onResult(false);
diff --git a/android_webview/java/src/org/chromium/android_webview/safe_browsing/AwSafeBrowsingConfigHelper.java b/android_webview/java/src/org/chromium/android_webview/safe_browsing/AwSafeBrowsingConfigHelper.java
index f64b87f4..d95a5eb 100644
--- a/android_webview/java/src/org/chromium/android_webview/safe_browsing/AwSafeBrowsingConfigHelper.java
+++ b/android_webview/java/src/org/chromium/android_webview/safe_browsing/AwSafeBrowsingConfigHelper.java
@@ -77,14 +77,15 @@
     }
 
     public static void maybeEnableSafeBrowsingFromGms() {
-        Callback<Boolean> cb =
+        Callback<@Nullable Boolean> cb =
                 verifyAppsValue -> {
-                    ThreadUtils.postOnUiThread(
-                            () -> {
-                                AwSafeBrowsingConfigHelperJni.get()
-                                        .setSafeBrowsingUserOptIn(
-                                                Boolean.TRUE.equals(verifyAppsValue));
-                            });
+                    if (verifyAppsValue != null) {
+                        ThreadUtils.postOnUiThread(
+                                () -> {
+                                    AwSafeBrowsingConfigHelperJni.get()
+                                            .setSafeBrowsingUserOptIn(verifyAppsValue);
+                                });
+                    }
                 };
         PlatformServiceBridge.getInstance().querySafeBrowsingUserConsent(cb);
     }
diff --git a/android_webview/java/src/org/chromium/android_webview/supervised_user/AwSupervisedUserUrlClassifier.java b/android_webview/java/src/org/chromium/android_webview/supervised_user/AwSupervisedUserUrlClassifier.java
index cf2bffa..43025df 100644
--- a/android_webview/java/src/org/chromium/android_webview/supervised_user/AwSupervisedUserUrlClassifier.java
+++ b/android_webview/java/src/org/chromium/android_webview/supervised_user/AwSupervisedUserUrlClassifier.java
@@ -74,11 +74,13 @@
     public void checkIfNeedRestrictedContentBlocking() {
         mDelegate.needsRestrictedContentBlocking(
                 result -> {
-                    ThreadUtils.postOnUiThread(
-                            () -> {
-                                AwSupervisedUserUrlClassifierJni.get()
-                                        .setUserRequiresUrlChecks(result);
-                            });
+                    if (result != null) {
+                        ThreadUtils.postOnUiThread(
+                                () -> {
+                                    AwSupervisedUserUrlClassifierJni.get()
+                                            .setUserRequiresUrlChecks(result);
+                                });
+                    }
                 });
     }
 
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java
index 9dc78d71..577147b 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java
@@ -50,6 +50,7 @@
 import org.chromium.base.test.util.Criteria;
 import org.chromium.base.test.util.CriteriaHelper;
 import org.chromium.base.test.util.Feature;
+import org.chromium.base.test.util.HistogramWatcher;
 import org.chromium.build.annotations.NullMarked;
 import org.chromium.build.annotations.Nullable;
 import org.chromium.content_public.browser.MessagePayload;
@@ -420,13 +421,15 @@
 
     private static class OnProgressChangedClient extends TestAwContentsClient {
         private final CallbackHelper mCallbackHelper = new CallbackHelper();
+        private int mLastProgress = -1;
 
         @Override
         public void onProgressChanged(int progress) {
             super.onProgressChanged(progress);
-            if (progress == 100) {
+            if (progress == 100 && mLastProgress != 100) {
                 mCallbackHelper.notifyCalled();
             }
+            mLastProgress = progress;
         }
 
         public void waitForFullLoad() throws TimeoutException {
@@ -462,7 +465,7 @@
         // works.
         private final Executor mExecutor = new BackgroundThreadExecutor("TEST_BACKGROUND_THREAD");
         private final CallbackHelper mNeedsRestrictionHelper = new CallbackHelper();
-        private boolean mNeedsRestrictionResponse;
+        private @Nullable Boolean mNeedsRestrictionResponse;
         private static final Set RESTRICTED_CONTENT_BLOCKLIST =
                 Set.of(MATURE_SITE_PATH, MATURE_SITE_IFRAME_PATH);
 
@@ -477,7 +480,7 @@
         }
 
         @Override
-        public void needsRestrictedContentBlocking(final Callback<Boolean> callback) {
+        public void needsRestrictedContentBlocking(final Callback<@Nullable Boolean> callback) {
             mExecutor.execute(
                     () -> {
                         callback.onResult(mNeedsRestrictionResponse);
@@ -485,7 +488,7 @@
                     });
         }
 
-        public void setNeedsRestrictedContentBlockingResponse(boolean value) {
+        public void setNeedsRestrictedContentBlockingResponse(@Nullable Boolean value) {
             mNeedsRestrictionResponse = value;
         }
 
@@ -507,7 +510,7 @@
         }
     }
 
-    private void resetNeedsRestriction(boolean value) throws Exception {
+    private void resetNeedsRestriction(@Nullable Boolean value) throws Exception {
         mDelegate.setNeedsRestrictedContentBlockingResponse(value);
         int count = mDelegate.getNeedsRestrictionHelper().getCallCount();
         AwSupervisedUserUrlClassifier classifier = AwSupervisedUserUrlClassifier.getInstance();
@@ -516,4 +519,49 @@
         classifier.checkIfNeedRestrictedContentBlocking();
         mDelegate.getNeedsRestrictionHelper().waitForCallback(count);
     }
+
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testRestrictedContentBlockingNullDoesNotUpdateCache() throws Throwable {
+        String embeddedUrl = setUpWebPage(MATURE_SITE_IFRAME_PATH, MATURE_SITE_IFRAME_TITLE, null);
+        String requestUrl = setUpWebPage(MATURE_SITE_PATH, MATURE_SITE_TITLE, embeddedUrl);
+
+        // Start with restriction enabled (setUp sets it to true, but let's be explicit)
+        resetNeedsRestriction(true);
+        loadUrl(requestUrl);
+        assertPageTitle(BLOCKED_SITE_TITLE);
+
+        // Now set restriction response to null (simulating timeout/error)
+        // It should NOT update the cache, so restriction should remain enabled (mature pages
+        // blocked)
+        // We also check that the histogram is NOT recorded.
+        try (HistogramWatcher watcher =
+                HistogramWatcher.newBuilder()
+                        .expectNoRecords(
+                                "Android.WebView.RestrictedContentBlocking.ApiCallMatchesDiskCache")
+                        .build()) {

diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java
index 9dc78d71..577147b 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwSupervisedUserTest.java
@@ -50,6 +50,7 @@
 import org.chromium.base.test.util.Criteria;
 import org.chromium.base.test.util.CriteriaHelper;
 import org.chromium.base.test.util.Feature;
+import org.chromium.base.test.util.HistogramWatcher;
 import org.chromium.build.annotations.NullMarked;
 import org.chromium.build.annotations.Nullable;
 import org.chromium.content_public.browser.MessagePayload;
@@ -420,13 +421,15 @@
 
     private static class OnProgressChangedClient extends TestAwContentsClient {
         private final CallbackHelper mCallbackHelper = new CallbackHelper();
+        private int mLastProgress = -1;
 
         @Override
         public void onProgressChanged(int progress) {
             super.onProgressChanged(progress);
-            if (progress == 100) {
+            if (progress == 100 && mLastProgress != 100) {
                 mCallbackHelper.notifyCalled();
             }
+            mLastProgress = progress;
         }
 
         public void waitForFullLoad() throws TimeoutException {
@@ -462,7 +465,7 @@
         // works.
         private final Executor mExecutor = new BackgroundThreadExecutor("TEST_BACKGROUND_THREAD");
         private final CallbackHelper mNeedsRestrictionHelper = new CallbackHelper();
-        private boolean mNeedsRestrictionResponse;
+        private @Nullable Boolean mNeedsRestrictionResponse;
         private static final Set RESTRICTED_CONTENT_BLOCKLIST =
                 Set.of(MATURE_SITE_PATH, MATURE_SITE_IFRAME_PATH);
 
@@ -477,7 +480,7 @@
         }
 
         @Override
-        public void needsRestrictedContentBlocking(final Callback<Boolean> callback) {
+        public void needsRestrictedContentBlocking(final Callback<@Nullable Boolean> callback) {
             mExecutor.execute(
                     () -> {
                         callback.onResult(mNeedsRestrictionResponse);
@@ -485,7 +488,7 @@
                     });
         }
 
-        public void setNeedsRestrictedContentBlockingResponse(boolean value) {
+        public void setNeedsRestrictedContentBlockingResponse(@Nullable Boolean value) {
             mNeedsRestrictionResponse = value;
         }
 
@@ -507,7 +510,7 @@
         }
     }
 
-    private void resetNeedsRestriction(boolean value) throws Exception {
+    private void resetNeedsRestriction(@Nullable Boolean value) throws Exception {
         mDelegate.setNeedsRestrictedContentBlockingResponse(value);
         int count = mDelegate.getNeedsRestrictionHelper().getCallCount();
         AwSupervisedUserUrlClassifier classifier = AwSupervisedUserUrlClassifier.getInstance();
@@ -516,4 +519,49 @@
         classifier.checkIfNeedRestrictedContentBlocking();
         mDelegate.getNeedsRestrictionHelper().waitForCallback(count);
     }
+
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testRestrictedContentBlockingNullDoesNotUpdateCache() throws Throwable {
+        String embeddedUrl = setUpWebPage(MATURE_SITE_IFRAME_PATH, MATURE_SITE_IFRAME_TITLE, null);
+        String requestUrl = setUpWebPage(MATURE_SITE_PATH, MATURE_SITE_TITLE, embeddedUrl);
+
+        // Start with restriction enabled (setUp sets it to true, but let's be explicit)
+        resetNeedsRestriction(true);
+        loadUrl(requestUrl);
+        assertPageTitle(BLOCKED_SITE_TITLE);
+
+        // Now set restriction response to null (simulating timeout/error)
+        // It should NOT update the cache, so restriction should remain enabled (mature pages
+        // blocked)
+        // We also check that the histogram is NOT recorded.
+        try (HistogramWatcher watcher =
+                HistogramWatcher.newBuilder()
+                        .expectNoRecords(
+                                "Android.WebView.RestrictedContentBlocking.ApiCallMatchesDiskCache")
+                        .build()) {
+            resetNeedsRestriction(null);
+            loadUrl(requestUrl);
+            assertPageTitle(BLOCKED_SITE_TITLE);
+        }
+
+        // Now set restriction to false
+        resetNeedsRestriction(false);
+        loadUrl(requestUrl);
+        assertPageTitle(MATURE_SITE_TITLE);
+
+        // Now set restriction response to null again
+        // It should NOT update the cache, so restriction should remain disabled (mature pages
+        // allowed)
+        try (HistogramWatcher watcher =
+                HistogramWatcher.newBuilder()
+                        .expectNoRecords(
+                                "Android.WebView.RestrictedContentBlocking.ApiCallMatchesDiskCache")
+                        .build()) {
+            resetNeedsRestriction(null);
+            loadUrl(requestUrl);
+            assertPageTitle(MATURE_SITE_TITLE);
+        }
+    }
 }
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java
index 84c84447..dab5556 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java
@@ -56,6 +56,7 @@
 import org.chromium.base.test.util.DoNotBatch;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.HistogramWatcher;
+import org.chromium.build.annotations.Nullable;
 import org.chromium.components.safe_browsing.SafeBrowsingApiBridge;
 import org.chromium.components.safe_browsing.SafeBrowsingApiHandler;
 import org.chromium.net.test.EmbeddedTestServer;
@@ -183,8 +184,8 @@
      * A fake PlatformServiceBridge that allows tests to make safe browsing requests without GMS.
      */
     private static class MockPlatformServiceBridge extends PlatformServiceBridge {
-        private Callback<Boolean> mCallback;
-        private Boolean mConsent;
+        private Callback<@Nullable Boolean> mCallback;
+        private @Nullable Boolean mConsent;
 
         @Override
         public boolean canUseGms() {
@@ -192,7 +193,7 @@
         }
 
         @Override
-        public void querySafeBrowsingUserConsent(Callback<Boolean> callback) {
+        public void querySafeBrowsingUserConsent(Callback<@Nullable Boolean> callback) {
             mCallback = callback;
             if (mConsent != null) {
                 callback.onResult(mConsent);
@@ -201,7 +202,7 @@
 
         public void setConsent(Boolean consent) {
             mConsent = consent;
-            if (mCallback != null && consent != null) {
+            if (mCallback != null) {
                 mCallback.onResult(consent);
             }
         }
@@ -1281,4 +1282,49 @@
             Assert.assertFalse(AwSafeBrowsingConfigHelper.getSafeBrowsingUserOptInForTesting());
         }
     }
+
+    @Test
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testSafeBrowsingUserOptInNullDoesNotUpdateCache() throws Throwable {
+        MockPlatformServiceBridge bridge =
+                (MockPlatformServiceBridge) PlatformServiceBridge.getInstance();
+
+        // Set initial consent to true in pref
+        AwSafeBrowsingConfigHelper.setSafeBrowsingUserOptInForTesting(true);
+
+        bridge.setConsent(null); // Don't return immediately
+
+        AwSafeBrowsingConfigHelper.maybeEnableSafeBrowsingFromGms();
+
+        // Now trigger callback with null (simulating timeout/error)
+        // We expect that the cache is NOT updated (stays true)
+        // And we expect NO histogram record for ApiCallMatchesDiskCache
+        try (HistogramWatcher watcher =
+                HistogramWatcher.newBuilder()
+                        .expectNoRecords("SafeBrowsing.WebView.GmsOptIn.ApiCallMatchesDiskCache")
+                        .build()) {
+            bridge.setConsent(null);
+            // Waits for posted task from callback to complete (if any)
+            // and verifies pref is still true
+            Assert.assertTrue(AwSafeBrowsingConfigHelper.getSafeBrowsingUserOptInForTesting());
+        }
+
+        // Set initial consent to false in pref
+        AwSafeBrowsingConfigHelper.setSafeBrowsingUserOptInForTesting(false);
+
+        bridge.setConsent(null); // Reset
+
+        AwSafeBrowsingConfigHelper.maybeEnableSafeBrowsingFromGms();
+
+        // Trigger callback with null again
+        // We expect that the cache is NOT updated (stays false)
+        try (HistogramWatcher watcher =
+                HistogramWatcher.newBuilder()
+                        .expectNoRecords("SafeBrowsing.WebView.GmsOptIn.ApiCallMatchesDiskCache")
+                        .build()) {
+            bridge.setConsent(null);
+            Assert.assertFalse(AwSafeBrowsingConfigHelper.getSafeBrowsingUserOptInForTesting());
+        }
+    }
 }