From db4d9bc0119507c4dcd2ef7708f7c1338155f226 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Thu, 19 Dec 2024 10:55:18 +0100 Subject: [PATCH] Extract part of shouldOverrideUrlLoading to RequestInterceptor --- .../app/browser/BrowserWebViewClientTest.kt | 1 - .../UrlExtractingWebViewClientTest.kt | 1 - .../app/browser/BrowserWebViewClient.kt | 14 ++------ .../app/browser/WebViewRequestInterceptor.kt | 35 +++++++++++++++---- .../app/browser/di/BrowserModule.kt | 2 -- .../UrlExtractingWebViewClient.kt | 2 -- .../MaliciousSiteBlockerWebViewIntegration.kt | 2 -- .../AndroidBrowserConfigFeature.kt | 2 -- 8 files changed, 31 insertions(+), 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 index 76d90012132e..3061968b86f1 100644 --- 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/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt index c8092cab9873..df4281b4a21b 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/urlextraction/UrlExtractingWebViewClientTest.kt +++ b/app/src/androidTest/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 index 03196139f267..a0cb6de7cda9 100644 --- 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 index 1c0aabd3615c..5c3ec224118e 100644 --- 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 index 5f31afb4e012..46c600887664 100644 --- 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 index f92cfa71e904..f8c4e2a509cc 100644 --- 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/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt b/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt index 329dbf449f2f..ab00b5d78f2a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/webview/MaliciousSiteBlockerWebViewIntegration.kt +++ b/app/src/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 index 964d32aa388f..9866ea2b6c92 100644 --- 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 }