Remove extra call to onSafeBrowsingHit after dismissing interstitial

In Android webview, with committed interstitials enabled, the
onSafeBrowsingHit callback was called again after proceeding through an
interstitial. This adds a check that omits the call if the user is
coming from an interstitial and the site is already allowlisted. This
also adds a check for the extra call in an existing test.

Bug: 1047477
Change-Id: Ifd6e9c59218b3c53f24c31f027681685ffd15333
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2031671
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737113}

android_webview/browser/safe_browsing/aw_url_checker_delegate_impl.cc

@@ -25,6 +25,7 @@
 #include "components/safe_browsing/core/db/v4_protocol_manager_util.h"
 #include "components/safe_browsing/core/features.h"
 #include "components/safe_browsing/core/web_ui/constants.h"
+#include "components/security_interstitials/content/security_interstitial_tab_helper.h"
 #include "components/security_interstitials/content/unsafe_resource_util.h"
 #include "components/security_interstitials/core/unsafe_resource.h"
 #include "components/security_interstitials/core/urls.h"
@@ -160,6 +161,23 @@ void AwUrlCheckerDelegateImpl::StartApplicationResponse(
     const security_interstitials::UnsafeResource& resource,
     const AwWebResourceRequest& request) {
   content::WebContents* web_contents = resource.web_contents_getter.Run();
+
+  if (base::FeatureList::IsEnabled(safe_browsing::kCommittedSBInterstitials)) {
+    security_interstitials::SecurityInterstitialTabHelper*
+        security_interstitial_tab_helper = security_interstitials::
+            SecurityInterstitialTabHelper::FromWebContents(web_contents);
+    if (ui_manager->IsWhitelisted(resource) &&
+        security_interstitial_tab_helper &&
+        security_interstitial_tab_helper->IsDisplayingInterstitial()) {
+      // In this case we are about to leave an interstitial due to the user
+      // clicking proceed on it, we shouldn't call OnSafeBrowsingHit again.
+      resource.callback_thread->PostTask(
+          FROM_HERE, base::BindOnce(resource.callback, true /* proceed */,
+                                    false /* showed_interstitial */));
+      return;
+    }
+  }
+
   AwContentsClientBridge* client =
       AwContentsClientBridge::FromWebContents(web_contents);
 

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

@@ -286,6 +286,7 @@ public class SafeBrowsingTest {
         private AwWebResourceRequest mLastRequest;
         private int mLastThreatType;
         private int mAction = SafeBrowsingAction.SHOW_INTERSTITIAL;
+        private int mOnSafeBrowsingHitCount;
         private boolean mReporting = true;
 
         @Override
@@ -293,6 +294,7 @@ public class SafeBrowsingTest {
                 Callback<AwSafeBrowsingResponse> callback) {
             mLastRequest = request;
             mLastThreatType = threatType;
+            mOnSafeBrowsingHitCount++;
             callback.onResult(new AwSafeBrowsingResponse(mAction, mReporting));
         }
 
@@ -304,6 +306,10 @@ public class SafeBrowsingTest {
             return mLastThreatType;
         }
 
+        public int getOnSafeBrowsingHitCount() {
+            return mOnSafeBrowsingHitCount;
+        }
+
         public void setSafeBrowsingAction(int action) {
             mAction = action;
         }
@@ -697,9 +703,13 @@ public class SafeBrowsingTest {
         int pageFinishedCount = mContentsClient.getOnPageFinishedHelper().getCallCount();
         loadPathAndWaitForInterstitial(MALWARE_HTML_PATH);
         waitForInterstitialDomToLoad();
+        int onSafeBrowsingCountBeforeClick = mContentsClient.getOnSafeBrowsingHitCount();
         clickVisitUnsafePage();
         mContentsClient.getOnPageFinishedHelper().waitForCallback(pageFinishedCount);
         assertTargetPageHasLoaded(MALWARE_PAGE_BACKGROUND_COLOR);
+        // Check there is not an extra onSafeBrowsingHit call after proceeding.
+        Assert.assertEquals(
+                onSafeBrowsingCountBeforeClick, mContentsClient.getOnSafeBrowsingHitCount());
     }
 
     @Test