Merge branch 'release/5.36.2'

Author: CDRussell

app/src/androidTest/java/com/duckduckgo/app/browser/navigation/WebViewBackForwardListSafeExtractorTest.kt

@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2019 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.navigation
+
+import android.content.Context
+import android.webkit.WebBackForwardList
+import android.webkit.WebView
+import androidx.test.annotation.UiThreadTest
+import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
+import org.junit.Test
+
+class WebViewBackForwardListSafeExtractorTest {
+
+    private val context = getInstrumentation().targetContext
+
+    @UiThreadTest
+    @Test
+    fun whenCopyBackForwardListCalledAndExceptionThrownThenNavigationListIsNull() {
+        val testWebView = TestWebViewThrowsNullPointerException(context)
+        assertNull(testWebView.safeCopyBackForwardList())
+    }
+
+    @UiThreadTest
+    @Test
+    fun whenCopyBackForwardListCalledAndNoExceptionThrownThenNavigationListIsNotNull() {
+        val testWebView = WebView(context)
+        assertNotNull(testWebView.safeCopyBackForwardList())
+    }
+}
+
+class TestWebViewThrowsNullPointerException(context: Context) : WebView(context) {
+
+    override fun copyBackForwardList(): WebBackForwardList {
+        throw NullPointerException("Deliberate")
+    }
+}

app/src/main/java/com/duckduckgo/app/browser/BrowserChromeClient.kt

@@ -21,6 +21,7 @@ import android.view.View
 import android.webkit.ValueCallback
 import android.webkit.WebChromeClient
 import android.webkit.WebView
+import com.duckduckgo.app.browser.navigation.safeCopyBackForwardList
 import com.duckduckgo.app.global.exception.UncaughtExceptionRepository
 import com.duckduckgo.app.global.exception.UncaughtExceptionSource.*
 import kotlinx.coroutines.*
@@ -75,7 +76,7 @@ class BrowserChromeClient @Inject constructor(private val uncaughtExceptionRepos
     override fun onProgressChanged(webView: WebView, newProgress: Int) {
         try {
             Timber.d("onProgressChanged ${webView.url}, $newProgress")
-            val navigationList = webView.copyBackForwardList()
+            val navigationList = webView.safeCopyBackForwardList() ?: return
             webViewClientListener?.navigationStateChanged(WebViewNavigationState(navigationList))
             webViewClientListener?.progressChanged(newProgress)
         } catch (e: Throwable) {

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

@@ -627,7 +627,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope {
         }
 
         viewModel.privacyGrade.observe(this, Observer<PrivacyGrade> {
-            Timber.i("Observed grade: $it")
+            Timber.d("Observed grade: $it")
             it?.let { privacyGrade ->
                 val drawable = context?.getDrawable(privacyGrade.icon()) ?: return@let
                 privacyGradeButton?.setImageDrawable(drawable)

app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt

@@ -26,6 +26,7 @@ import androidx.annotation.WorkerThread
 import com.duckduckgo.app.browser.BrowserWebViewClient.RequestOrigin.MainFrame
 import com.duckduckgo.app.browser.BrowserWebViewClient.RequestOrigin.SubFrame
 import com.duckduckgo.app.browser.model.BasicAuthenticationRequest
+import com.duckduckgo.app.browser.navigation.safeCopyBackForwardList
 import com.duckduckgo.app.global.exception.UncaughtExceptionRepository
 import com.duckduckgo.app.global.exception.UncaughtExceptionSource.*
 import com.duckduckgo.app.statistics.store.OfflinePixelCountDataStore
@@ -88,7 +89,8 @@ class BrowserWebViewClient(
     @UiThread
     override fun onPageStarted(webView: WebView, url: String?, favicon: Bitmap?) {
         try {
-            webViewClientListener?.navigationStateChanged(WebViewNavigationState(webView.copyBackForwardList()))
+            val navigationList = webView.safeCopyBackForwardList() ?: return
+            webViewClientListener?.navigationStateChanged(WebViewNavigationState(navigationList))
             if (url != null && url == lastPageStarted) {
                 webViewClientListener?.pageRefreshed(url)
             }
@@ -104,7 +106,8 @@ class BrowserWebViewClient(
     @UiThread
     override fun onPageFinished(webView: WebView, url: String?) {
         try {
-            webViewClientListener?.navigationStateChanged(WebViewNavigationState(webView.copyBackForwardList()))
+            val navigationList = webView.safeCopyBackForwardList() ?: return
+            webViewClientListener?.navigationStateChanged(WebViewNavigationState(navigationList))
         } catch (e: Throwable) {
             GlobalScope.launch {
                 uncaughtExceptionRepository.recordUncaughtException(e, ON_PAGE_FINISHED)

app/src/main/java/com/duckduckgo/app/browser/WebNavigationState.kt

@@ -132,6 +132,6 @@ private val WebBackForwardList.isHttpsUpgrade: Boolean
     get() {
         if (currentIndex < 1) return false
         val current = originalUrl?.toUri() ?: return false
-        val previous = getItemAtIndex(currentIndex - 1).originalUrl?.toUri() ?: return false
+        val previous = getItemAtIndex(currentIndex - 1)?.originalUrl?.toUri() ?: return false
         return current.isHttpsVersionOfUri(previous)
     }

app/src/main/java/com/duckduckgo/app/browser/navigation/WebViewBackForwardListSafeExtractor.kt

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2019 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.navigation
+
+import android.webkit.WebBackForwardList
+import android.webkit.WebView
+import timber.log.Timber
+
+/**
+ * There is a bug in WebView whereby `webView.copyBackForwardList()` can internally throw a NPE
+ *
+ * This extension function can be used as a direct replacement of `copyBackForwardList()`
+ * It will catch the NullPointerException and return `null` when it happens.
+ *
+ * https://bugs.chromium.org/p/chromium/issues/detail?id=498796
+ */
+fun WebView.safeCopyBackForwardList(): WebBackForwardList? {
+    return try {
+        copyBackForwardList()
+    } catch (e: NullPointerException) {
+        Timber.e(e, "Failed to extract WebView back forward list")
+        null
+    }
+}

app/src/main/java/com/duckduckgo/app/global/AlertingUncaughtExceptionHandler.kt

@@ -20,8 +20,8 @@ import com.duckduckgo.app.global.exception.UncaughtExceptionRepository
 import com.duckduckgo.app.global.exception.UncaughtExceptionSource
 import com.duckduckgo.app.statistics.store.OfflinePixelCountDataStore
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.GlobalScope
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withContext
 
 class AlertingUncaughtExceptionHandler(
     private val originalHandler: Thread.UncaughtExceptionHandler,
@@ -30,11 +30,15 @@ class AlertingUncaughtExceptionHandler(
 ) : Thread.UncaughtExceptionHandler {
 
     override fun uncaughtException(t: Thread?, originalException: Throwable?) {
-        GlobalScope.launch(Dispatchers.IO) {
-            uncaughtExceptionRepository.recordUncaughtException(originalException, UncaughtExceptionSource.GLOBAL)
 
-            offlinePixelCountDataStore.applicationCrashCount += 1
-            originalHandler.uncaughtException(t, originalException)
+        // block until the exception has been fully processed
+        runBlocking {
+            withContext(Dispatchers.IO) {
+                uncaughtExceptionRepository.recordUncaughtException(originalException, UncaughtExceptionSource.GLOBAL)
+                offlinePixelCountDataStore.applicationCrashCount += 1
+            }
         }
+        originalHandler.uncaughtException(t, originalException)
     }
+
 }

app/version/version.properties

@@ -1 +1 @@
-VERSION=5.36.1
+VERSION=5.36.2