Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@CrisBarreiro
CrisBarreiro committed Dec 16, 2024
1 parent bba00aa commit 601db6b
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 83 deletions.
4 changes: 2 additions & 2 deletions app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class BrowserWebViewClientTest {
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(0)
whenever(webViewVersionProvider.getMajorVersion()).thenReturn("1")
whenever(deviceInfo.appVersion).thenReturn("1")
whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any(), any())).thenReturn(false)
whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any())).thenReturn(false)
}

@UiThreadTest
Expand Down Expand Up @@ -338,7 +338,7 @@ class BrowserWebViewClientTest {
TestScope().launch {
val webResourceRequest = mock<WebResourceRequest>()
testee.shouldInterceptRequest(webView, webResourceRequest)
verify(requestInterceptor).shouldIntercept(any(), any(), any<Uri>(), any(), any())
verify(requestInterceptor).shouldIntercept(any(), any(), any<Uri>(), any())
}
}

Expand Down
36 changes: 1 addition & 35 deletions app/src/androidTest/java/com/duckduckgo/app/browser/WebViewRequestInterceptorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class WebViewRequestInterceptorTest {
cloakedCnameDetector = mockCloakedCnameDetector,
requestFilterer = mockRequestFilterer,
duckPlayer = mockDuckPlayer,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection,
)
}

Expand All @@ -130,7 +131,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)
assertCancelledResponse(response)
Expand All @@ -143,7 +143,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -157,7 +156,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -172,7 +170,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -187,7 +184,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)
verify(mockHttpsUpgrader).upgrade(any())
Expand All @@ -203,7 +199,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -217,7 +212,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)
verify(mockDuckPlayer).intercept(any(), any(), any())
Expand All @@ -231,7 +225,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -245,7 +238,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -259,7 +251,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)
assertRequestCanContinueToLoad(response)
Expand All @@ -272,7 +263,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "duckduckgo.com/a/b/c?q=123".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -286,7 +276,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "donttrack.us/a/b/c?q=123".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -300,7 +289,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "spreadprivacy.com/a/b/c?q=123".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -314,7 +302,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "duckduckhack.com/a/b/c?q=123".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -328,7 +315,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "privatebrowsingmyths.com/a/b/c?q=123".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -342,7 +328,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "duck.co/a/b/c?q=123".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -359,7 +344,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockListener,
)

Expand All @@ -377,7 +361,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockListener,
)

Expand All @@ -395,7 +378,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -418,7 +400,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -442,7 +423,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -457,7 +437,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -474,7 +453,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -492,7 +470,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -510,7 +487,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -528,7 +504,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -546,7 +521,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -563,7 +537,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -580,7 +553,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -597,7 +569,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand All @@ -614,7 +585,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = null,
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = mockWebViewClientListener,
)

Expand Down Expand Up @@ -674,7 +644,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand Down Expand Up @@ -713,7 +682,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -735,7 +703,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand All @@ -755,7 +722,6 @@ class WebViewRequestInterceptorTest {
request = mockRequest,
documentUri = "foo.com".toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand Down
2 changes: 1 addition & 1 deletion app/src/androidTest/java/com/duckduckgo/app/referencetests/DomainsReferenceTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class DomainsReferenceTest(private val testCase: TestCase) {
adClickManager = mockAdClickManager,
cloakedCnameDetector = CloakedCnameDetectorImpl(tdsCnameEntityDao, mockTrackerAllowlist, mockUserAllowListRepository),
requestFilterer = mockRequestFilterer,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection,
duckPlayer = mockDuckPlayer,
)
}
Expand All @@ -200,7 +201,6 @@ class DomainsReferenceTest(private val testCase: TestCase) {
request = mockRequest,
documentUri = testCase.siteURL.toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand Down
2 changes: 1 addition & 1 deletion app/src/androidTest/java/com/duckduckgo/app/referencetests/SurrogatesReferenceTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) {
adClickManager = mockAdClickManager,
cloakedCnameDetector = mockCloakedCnameDetector,
requestFilterer = mockRequestFilterer,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection,
duckPlayer = mockDuckPlayer,
)
}
Expand All @@ -189,7 +190,6 @@ class SurrogatesReferenceTest(private val testCase: TestCase) {
request = mockRequest,
documentUri = testCase.siteURL.toUri(),
webView = webView,
maliciousSiteProtectionWebViewIntegration = mockMaliciousSiteProtection,
webViewClientListener = null,
)

Expand Down
14 changes: 5 additions & 9 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,11 @@ class BrowserWebViewClient @Inject constructor(
Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url")
webViewClientListener?.onShouldOverride()

if (runBlocking {
maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading(
url,
webView.url?.toUri(),
isForMainFrame,
confirmationCallback,
)
}
if (maliciousSiteProtectionWebViewIntegration.shouldOverrideUrlLoading(
url,
isForMainFrame,
confirmationCallback,
)
) {
// TODO (cbarreiro): Handle site blocked synchronously
return true
Expand Down Expand Up @@ -534,7 +531,6 @@ class BrowserWebViewClient @Inject constructor(
request,
webView,
documentUrl?.toUri(),
maliciousSiteProtectionWebViewIntegration,
webViewClientListener,
)
}
Expand Down
5 changes: 2 additions & 3 deletions app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ interface RequestInterceptor {
request: WebResourceRequest,
webView: WebView,
documentUri: Uri?,
maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
webViewClientListener: WebViewClientListener?,
): WebResourceResponse?

Expand All @@ -73,6 +72,7 @@ class WebViewRequestInterceptor(
private val cloakedCnameDetector: CloakedCnameDetector,
private val requestFilterer: RequestFilterer,
private val duckPlayer: DuckPlayer,
private val maliciousSiteBlockerWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
private val dispatchers: DispatcherProvider = DefaultDispatcherProvider(),
) : RequestInterceptor {

Expand All @@ -94,7 +94,6 @@ class WebViewRequestInterceptor(
request: WebResourceRequest,
webView: WebView,
documentUri: Uri?,
maliciousSiteProtectionWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
webViewClientListener: WebViewClientListener?,
): WebResourceResponse? {
val url: Uri? = request.url
Expand All @@ -103,7 +102,7 @@ class WebViewRequestInterceptor(
// TODO (cbarreiro): Handle site blocked asynchronously
}

maliciousSiteProtectionWebViewIntegration.shouldIntercept(request, documentUri, confirmationCallback)?.let {
maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri, confirmationCallback)?.let {
// TODO (cbarreiro): Handle site blocked synchronously
return it
}
Expand Down
Loading

0 comments on commit 601db6b

Please sign in to comment.