Skip to content

Commit

Permalink
Add isDestroyed guard to WebView methods (#4787)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1200204095367872/1207860204325574/f

### Description
This task adds `isDestroyed` checks to all the `WebView` methods used in
`DuckDuckGoWebView`.

### Steps to test this PR
- [ ] Smoke test
  • Loading branch information
joshliebe authored Jul 30, 2024
1 parent df4b5ec commit d071a44
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 12 deletions.
24 changes: 14 additions & 10 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ import com.duckduckgo.app.browser.menu.BrowserPopupMenu
import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials
import com.duckduckgo.app.browser.model.BasicAuthenticationRequest
import com.duckduckgo.app.browser.model.LongPressTarget
import com.duckduckgo.app.browser.navigation.safeCopyBackForwardList
import com.duckduckgo.app.browser.newtab.FocusedViewProvider
import com.duckduckgo.app.browser.newtab.NewTabPageProvider
import com.duckduckgo.app.browser.omnibar.OmnibarScrolling
Expand Down Expand Up @@ -156,6 +155,7 @@ import com.duckduckgo.app.browser.viewstate.SavedSiteChangedViewState
import com.duckduckgo.app.browser.webshare.WebShareChooser
import com.duckduckgo.app.browser.webview.WebContentDebugging
import com.duckduckgo.app.browser.webview.WebViewBlobDownloadFeature
import com.duckduckgo.app.browser.webview.safewebview.SafeWebViewFeature
import com.duckduckgo.app.cta.ui.*
import com.duckduckgo.app.di.AppCoroutineScope
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity
Expand Down Expand Up @@ -491,6 +491,9 @@ class BrowserTabFragment :
@Inject
lateinit var singlePrintSafeguardFeature: SinglePrintSafeguardFeature

@Inject
lateinit var safeWebViewFeature: SafeWebViewFeature

/**
* We use this to monitor whether the user was seeing the in-context Email Protection signup prompt
* This is needed because the activity stack will be cleared if an external link is opened in our browser
Expand Down Expand Up @@ -1237,7 +1240,7 @@ class BrowserTabFragment :
url: String,
headers: Map<String, String>,
) {
clientBrandHintProvider.setOn(webView?.settings, url)
clientBrandHintProvider.setOn(webView?.safeSettings, url)
hideKeyboard()
renderer.hideFindInPage()
viewModel.registerDaxBubbleCtaDismissed()
Expand Down Expand Up @@ -1362,7 +1365,7 @@ class BrowserTabFragment :
dismissAppLinkSnackBar()
val navList = webView?.safeCopyBackForwardList()
val currentIndex = navList?.currentIndex ?: 0
clientBrandHintProvider.setOn(webView?.settings, navList?.getItemAtIndex(currentIndex - 1)?.url.toString())
clientBrandHintProvider.setOn(webView?.safeSettings, navList?.getItemAtIndex(currentIndex - 1)?.url.toString())
viewModel.refreshBrowserError()
webView?.goBackOrForward(-it.steps)
}
Expand All @@ -1371,7 +1374,7 @@ class BrowserTabFragment :
dismissAppLinkSnackBar()
val navList = webView?.safeCopyBackForwardList()
val currentIndex = navList?.currentIndex ?: 0
clientBrandHintProvider.setOn(webView?.settings, navList?.getItemAtIndex(currentIndex + 1)?.url.toString())
clientBrandHintProvider.setOn(webView?.safeSettings, navList?.getItemAtIndex(currentIndex + 1)?.url.toString())
viewModel.refreshBrowserError()
webView?.goForward()
}
Expand Down Expand Up @@ -2219,6 +2222,7 @@ class BrowserTabFragment :
).findViewById(R.id.browserWebView) as DuckDuckGoWebView

webView?.let {
it.isSafeWebViewEnabled = safeWebViewFeature.self().isEnabled()
it.webViewClient = webViewClient
it.webChromeClient = webChromeClient
it.clearSslPreferences()
Expand Down Expand Up @@ -2619,7 +2623,7 @@ class BrowserTabFragment :
view: View,
menuInfo: ContextMenu.ContextMenuInfo?,
) {
webView?.hitTestResult?.let {
webView?.safeHitTestResult?.let {
val target = getLongPressTarget(it) ?: return
viewModel.userLongPressedInWebView(target, menu)
}
Expand Down Expand Up @@ -2662,7 +2666,7 @@ class BrowserTabFragment :

override fun onContextItemSelected(item: MenuItem): Boolean {
runCatching {
webView?.hitTestResult?.let {
webView?.safeHitTestResult?.let {
val target = getLongPressTarget(it)
if (target != null && viewModel.userSelectedItemFromLongPressMenu(target, item)) {
return true
Expand Down Expand Up @@ -2885,10 +2889,10 @@ class BrowserTabFragment :
url: String?,
isDesktop: Boolean,
) {
val currentAgent = webView?.settings?.userAgentString
val currentAgent = webView?.safeSettings?.userAgentString
val newAgent = userAgentProvider.userAgent(url, isDesktop)
if (newAgent != currentAgent) {
webView?.settings?.userAgentString = newAgent
webView?.safeSettings?.userAgentString = newAgent
}
Timber.d("User Agent is $newAgent")
}
Expand Down Expand Up @@ -3180,7 +3184,7 @@ class BrowserTabFragment :
val navList = webView?.safeCopyBackForwardList()
val currentIndex = navList?.currentIndex ?: 0

clientBrandHintProvider.setOn(webView?.settings, navList?.getItemAtIndex(currentIndex + stepsToMove)?.url.toString())
clientBrandHintProvider.setOn(webView?.safeSettings, navList?.getItemAtIndex(currentIndex + stepsToMove)?.url.toString())
webView?.goBackOrForward(stepsToMove)
}

Expand Down Expand Up @@ -4033,7 +4037,7 @@ class BrowserTabFragment :
if (viewModel.isPrinting()) return

(activity?.getSystemService(Context.PRINT_SERVICE) as? PrintManager)?.let { printManager ->
webView?.createPrintDocumentAdapter(url)?.let { webViewPrintDocumentAdapter ->
webView?.createSafePrintDocumentAdapter(url)?.let { webViewPrintDocumentAdapter ->

val printAdapter = if (singlePrintSafeguardFeature.self().isEnabled()) {
PrintDocumentAdapterFactory.createPrintDocumentAdapter(
Expand Down
139 changes: 137 additions & 2 deletions app/src/main/java/com/duckduckgo/app/browser/DuckDuckGoWebView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,25 @@ package com.duckduckgo.app.browser

import android.annotation.SuppressLint
import android.content.Context
import android.os.Message
import android.print.PrintDocumentAdapter
import android.util.AttributeSet
import android.view.MotionEvent
import android.view.inputmethod.EditorInfo
import android.view.inputmethod.InputConnection
import android.webkit.DownloadListener
import android.webkit.WebBackForwardList
import android.webkit.WebChromeClient
import android.webkit.WebSettings
import android.webkit.WebView
import android.webkit.WebViewClient
import androidx.core.view.NestedScrollingChild3
import androidx.core.view.NestedScrollingChildHelper
import androidx.core.view.ViewCompat
import androidx.webkit.WebViewCompat
import androidx.webkit.WebViewCompat.WebMessageListener
import androidx.webkit.WebViewFeature
import com.duckduckgo.app.browser.navigation.safeCopyBackForwardList
import com.duckduckgo.browser.api.WebViewVersionProvider
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.extensions.compareSemanticVersion
Expand Down Expand Up @@ -57,7 +65,8 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
private var nestedScrollHelper: NestedScrollingChildHelper = NestedScrollingChildHelper(this)
private val helper = CoordinatorLayoutHelper()

var isDestroyed: Boolean = false
private var isDestroyed: Boolean = false
var isSafeWebViewEnabled: Boolean = false

constructor(context: Context) : this(context, null)
constructor(
Expand All @@ -73,10 +82,136 @@ class DuckDuckGoWebView : WebView, NestedScrollingChild3 {
}

override fun destroy() {
isDestroyed = true
isDestroyed = true && isSafeWebViewEnabled
super.destroy()
}

override fun stopLoading() {
if (!isDestroyed) {
super.stopLoading()
}
}

override fun onPause() {
if (!isDestroyed) {
super.onPause()
}
}

override fun onResume() {
if (!isDestroyed) {
super.onResume()
}
}

override fun loadUrl(url: String) {
if (!isDestroyed) {
super.loadUrl(url)
}
}

override fun loadUrl(
url: String,
additionalHttpHeaders: Map<String, String>,
) {
if (!isDestroyed) {
super.loadUrl(url, additionalHttpHeaders)
}
}

override fun reload() {
if (!isDestroyed) {
super.reload()
}
}

override fun goForward() {
if (!isDestroyed) {
super.goForward()
}
}

override fun goBackOrForward(steps: Int) {
if (!isDestroyed) {
super.goBackOrForward(steps)
}
}

override fun findAllAsync(find: String) {
if (!isDestroyed) {
super.findAllAsync(find)
}
}

override fun findNext(forward: Boolean) {
if (!isDestroyed) {
super.findNext(forward)
}
}

override fun clearSslPreferences() {
if (!isDestroyed) {
super.clearSslPreferences()
}
}

override fun setDownloadListener(listener: DownloadListener?) {
if (!isDestroyed) {
super.setDownloadListener(listener)
}
}

override fun requestFocusNodeHref(hrefMsg: Message?) {
if (!isDestroyed) {
super.requestFocusNodeHref(hrefMsg)
}
}

override fun setWebChromeClient(client: WebChromeClient?) {
if (!isDestroyed) {
super.setWebChromeClient(client)
}
}

override fun setWebViewClient(client: WebViewClient) {
if (!isDestroyed) {
super.setWebViewClient(client)
}
}

override fun setFindListener(listener: FindListener?) {
if (!isDestroyed) {
super.setFindListener(listener)
}
}

override fun getUrl(): String? {
if (isDestroyed) return null
return super.getUrl()
}

fun safeCopyBackForwardList(): WebBackForwardList? {
if (isDestroyed) return null
return (this as WebView).safeCopyBackForwardList()
}

fun createSafePrintDocumentAdapter(documentName: String): PrintDocumentAdapter? {
if (isDestroyed) return null
return createPrintDocumentAdapter(documentName)
}

val safeSettings: WebSettings?
get() {
if (isDestroyed) return null
return getSettings()
}

val safeHitTestResult: HitTestResult?
get() {
if (isDestroyed) return null
return getHitTestResult()
}

fun setBottomMatchingBehaviourEnabled(value: Boolean) {
helper.setBottomMatchingBehaviourEnabled(value)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright (c) 2024 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.app.browser.webview.safewebview

import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.feature.toggles.api.Toggle

@ContributesRemoteFeature(
scope = AppScope::class,
featureName = "safeWebView",
)

interface SafeWebViewFeature {

@Toggle.DefaultValue(true)
fun self(): Toggle
}

0 comments on commit d071a44

Please sign in to comment.