Extract part of shouldOverrideUrlLoading to RequestInterceptor

duckduckgo · Dec 19, 2024 · db4d9bc · db4d9bc
1 parent cfc8cea
commit db4d9bc
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 28 deletions.
diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt
@@ -191,7 +191,6 @@ class BrowserWebViewClientTest {
             mockDuckDuckGoUrlDetector,
             mockUriLoadedManager,
             mockAndroidFeaturesHeaderPlugin,
-            mockMaliciousSiteProtection,
         )
         testee.webViewClientListener = listener
         whenever(webResourceRequest.url).thenReturn(Uri.EMPTY)

diff --git a/...droidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt b/...droidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt
@@ -65,7 +65,6 @@ class UrlExtractingWebViewClientTest {
             TestScope(),
             coroutinesTestRule.testDispatcherProvider,
             urlExtractor,
-            mockMaliciousSiteProtection,
         )
         whenever(cookieManagerProvider.get()).thenReturn(cookieManager)
     }

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt
@@ -60,7 +60,6 @@ import com.duckduckgo.app.browser.pageloadpixel.firstpaint.PagePaintedHandler
 import com.duckduckgo.app.browser.print.PrintInjector
 import com.duckduckgo.app.browser.trafficquality.AndroidFeaturesHeaderPlugin
 import com.duckduckgo.app.browser.uriloaded.UriLoadedManager
-import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
 import com.duckduckgo.app.di.AppCoroutineScope
 import com.duckduckgo.app.statistics.pixels.Pixel
 import com.duckduckgo.autoconsent.api.Autoconsent
@@ -118,7 +117,6 @@ class BrowserWebViewClient @Inject constructor(
     private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector,
     private val uriLoadedManager: UriLoadedManager,
     private val androidFeaturesHeaderPlugin: AndroidFeaturesHeaderPlugin,
-    private val maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
 ) : WebViewClient() {
 
     var webViewClientListener: WebViewClientListener? = null
@@ -164,14 +162,7 @@ class BrowserWebViewClient @Inject constructor(
         try {
             Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url")
             webViewClientListener?.onShouldOverride()
-
-            if (maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading(
-                    url,
-                    isForMainFrame,
-                    confirmationCallback,
-                )
-            ) {
-                // TODO (cbarreiro): Handle site blocked synchronously
+            if (requestInterceptor.shouldOverrideUrlLoading(url, isForMainFrame)) {
                 return true
             }
 
@@ -424,12 +415,11 @@ class BrowserWebViewClient @Inject constructor(
             // See https://app.asana.com/0/0/1206159443951489/f (WebView limitations)
             if (it != ABOUT_BLANK && start == null) {
                 start = currentTimeProvider.elapsedRealtime()
-                maliciousSiteProtectionWebViewIntegration.onPageLoadStarted()
+                requestInterceptor.onPageStarted(url)
             }
             handleMediaPlayback(webView, it)
             autoconsent.injectAutoconsent(webView, url)
             adClickManager.detectAdDomain(url)
-            requestInterceptor.onPageStarted(url)
             appCoroutineScope.launch(dispatcherProvider.io()) {
                 thirdPartyCookieManager.processUriForThirdPartyCookies(webView, url.toUri())
             }

diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt
@@ -59,6 +59,12 @@ interface RequestInterceptor {
     ): WebResourceResponse?
 
     fun onPageStarted(url: String)
+
+    @WorkerThread
+    fun shouldOverrideUrlLoading(
+        url: Uri,
+        isForMainFrame: Boolean,
+    ): Boolean
 }
 
 class WebViewRequestInterceptor(
@@ -78,6 +84,7 @@ class WebViewRequestInterceptor(
 
     override fun onPageStarted(url: String) {
         requestFilterer.registerOnPageCreated(url)
+        maliciousSiteBlockerWebViewIntegration.onPageLoadStarted()
     }
 
     /**
@@ -98,12 +105,10 @@ class WebViewRequestInterceptor(
     ): WebResourceResponse? {
         val url: Uri? = request.url
 
-        val confirmationCallback: (isMalicious: Boolean) -> Unit = {
-            // TODO (cbarreiro): Handle site blocked asynchronously
-        }
-
-        maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri, confirmationCallback)?.let {
-            // TODO (cbarreiro): Handle site blocked synchronously
+        maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri) {
+            handleSiteBlocked()
+        }?.let {
+            handleSiteBlocked()
             return it
         }
 
@@ -172,6 +177,24 @@ class WebViewRequestInterceptor(
         return getWebResourceResponse(request, documentUrl, null)
     }
 
+    override fun shouldOverrideUrlLoading(url: Uri, isForMainFrame: Boolean): Boolean {
+        if (maliciousSiteBlockerWebViewIntegration.shouldOverrideUrlLoading(
+                url,
+                isForMainFrame,
+            ) {
+                handleSiteBlocked()
+            }
+        ) {
+            handleSiteBlocked()
+            return true
+        }
+        return false
+    }
+
+    private fun handleSiteBlocked() {
+        // TODO (cbarreiro): Handle site blocked
+    }
+
     private fun getWebResourceResponse(
         request: WebResourceRequest,
         documentUrl: Uri,

diff --git a/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt b/app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt
@@ -123,7 +123,6 @@ class BrowserModule {
         @AppCoroutineScope appCoroutineScope: CoroutineScope,
         dispatcherProvider: DispatcherProvider,
         urlExtractor: DOMUrlExtractor,
-        maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration,
     ): UrlExtractingWebViewClient {
         return UrlExtractingWebViewClient(
             webViewHttpAuthStore,
@@ -134,7 +133,6 @@ class BrowserModule {
             appCoroutineScope,
             dispatcherProvider,
             urlExtractor,
-            maliciousSiteProtection,
         )
     }
 

diff --git a/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClient.kt
@@ -28,7 +28,6 @@ import com.duckduckgo.app.browser.certificates.rootstore.CertificateValidationSt
 import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore
 import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager
 import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore
-import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.cookies.api.CookieManagerProvider
 import kotlinx.coroutines.*
@@ -43,7 +42,6 @@ class UrlExtractingWebViewClient(
     private val appCoroutineScope: CoroutineScope,
     private val dispatcherProvider: DispatcherProvider,
     private val urlExtractor: DOMUrlExtractor,
-    private val maliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration,
 ) : WebViewClient() {
 
     var urlExtractionListener: UrlExtractionListener? = null

diff --git a/...rc/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt b/...rc/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt
@@ -31,7 +31,6 @@ import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMali
 import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin
 import com.squareup.anvil.annotations.ContributesBinding
 import com.squareup.anvil.annotations.ContributesMultibinding
-import dagger.SingleInstanceIn
 import java.net.URLDecoder
 import javax.inject.Inject
 import kotlinx.coroutines.CoroutineScope
@@ -56,7 +55,6 @@ interface MaliciousSiteBlockerWebViewIntegration {
     fun onPageLoadStarted()
 }
 
-@SingleInstanceIn(AppScope::class)
 @ContributesMultibinding(AppScope::class, PrivacyConfigCallbackPlugin::class)
 @ContributesBinding(AppScope::class, MaliciousSiteBlockerWebViewIntegration::class)
 class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(

diff --git a/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt b/app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt
@@ -19,7 +19,6 @@ package com.duckduckgo.app.pixels.remoteconfig
 import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
 import com.duckduckgo.di.scopes.AppScope
 import com.duckduckgo.feature.toggles.api.Toggle
-import com.duckduckgo.feature.toggles.api.Toggle.InternalAlwaysEnabled
 
 /**
  * This is the class that represents the browser feature flags
@@ -90,7 +89,6 @@ interface AndroidBrowserConfigFeature {
      * sub-feature flag enabled
      * If the remote feature is not present defaults to `false`
      */
-    @InternalAlwaysEnabled
     @Toggle.DefaultValue(false)
     fun enableMaliciousSiteProtection(): Toggle
 }