Fix wrong favicon issue (#1200)

* we only save the favicon if it comes from the site we are visiting

* fixing tests

* spotless

* fixed placeholder

* fixing test dependencies

Author: malmstein

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserChromeClientTest.kt

@@ -117,13 +117,17 @@ class BrowserChromeClientTest {
     fun whenOnReceivedIconThenIconReceived() {
         val bitmap: Bitmap = Bitmap.createBitmap(1, 1, Bitmap.Config.RGB_565)
         testee.onReceivedIcon(webView, bitmap)
-        verify(mockWebViewClientListener).iconReceived(bitmap)
+        verify(mockWebViewClientListener).iconReceived(webView.url, bitmap)
     }
 
     private val mockMsg = Message().apply {
         target = mock()
         obj = mock<WebView.WebViewTransport>()
     }
 
-    private class TestWebView(context: Context) : WebView(context)
+    private class TestWebView(context: Context) : WebView(context) {
+        override fun getUrl(): String {
+            return "https://example.com"
+        }
+    }
 }

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt

@@ -59,7 +59,12 @@ import com.duckduckgo.app.browser.session.WebViewSessionStorage
 import com.duckduckgo.app.cta.db.DismissedCtaDao
 import com.duckduckgo.app.cta.model.CtaId
 import com.duckduckgo.app.cta.model.DismissedCta
-import com.duckduckgo.app.cta.ui.*
+import com.duckduckgo.app.cta.ui.Cta
+import com.duckduckgo.app.cta.ui.CtaViewModel
+import com.duckduckgo.app.cta.ui.DaxBubbleCta
+import com.duckduckgo.app.cta.ui.DaxDialogCta
+import com.duckduckgo.app.cta.ui.HomePanelCta
+import com.duckduckgo.app.cta.ui.UseOurAppCta
 import com.duckduckgo.app.email.EmailManager
 import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteDao
 import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity
@@ -109,7 +114,16 @@ import com.duckduckgo.app.trackerdetection.EntityLookup
 import com.duckduckgo.app.trackerdetection.model.TrackingEvent
 import com.duckduckgo.app.usage.search.SearchCountDao
 import com.duckduckgo.app.widget.ui.WidgetCapabilities
-import com.nhaarman.mockitokotlin2.*
+import com.nhaarman.mockitokotlin2.any
+import com.nhaarman.mockitokotlin2.anyOrNull
+import com.nhaarman.mockitokotlin2.atLeastOnce
+import com.nhaarman.mockitokotlin2.doAnswer
+import com.nhaarman.mockitokotlin2.doReturn
+import com.nhaarman.mockitokotlin2.eq
+import com.nhaarman.mockitokotlin2.firstValue
+import com.nhaarman.mockitokotlin2.lastValue
+import com.nhaarman.mockitokotlin2.mock
+import com.nhaarman.mockitokotlin2.whenever
 import dagger.Lazy
 import io.reactivex.Observable
 import io.reactivex.Single
@@ -129,13 +143,17 @@ import org.junit.Assert.*
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
-import org.mockito.*
+import org.mockito.ArgumentCaptor
 import org.mockito.ArgumentMatchers.anyString
+import org.mockito.Captor
+import org.mockito.Mock
+import org.mockito.Mockito
 import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
+import org.mockito.MockitoAnnotations
 import org.mockito.internal.util.DefaultMockingDetails
 import java.io.File
-import java.util.*
+import java.util.Locale
 import java.util.concurrent.TimeUnit
 
 @FlowPreview
@@ -2895,7 +2913,7 @@ class BrowserTabViewModelTest {
         givenOneActiveTabSelected()
         val bitmap: Bitmap = Bitmap.createBitmap(1, 1, Bitmap.Config.RGB_565)
 
-        testee.iconReceived(bitmap)
+        testee.iconReceived("https://example.com", bitmap)
 
         verify(mockFaviconManager).saveToTemp("TAB_ID", bitmap, "https://example.com")
     }
@@ -2907,7 +2925,7 @@ class BrowserTabViewModelTest {
         val file = File("test")
         whenever(mockFaviconManager.saveToTemp(any(), any(), any())).thenReturn(file)
 
-        testee.iconReceived(bitmap)
+        testee.iconReceived("https://example.com", bitmap)
 
         verify(mockTabRepository).updateTabFavicon("TAB_ID", file.name)
     }
@@ -2918,11 +2936,24 @@ class BrowserTabViewModelTest {
         val bitmap: Bitmap = Bitmap.createBitmap(1, 1, Bitmap.Config.RGB_565)
         whenever(mockFaviconManager.saveToTemp(any(), any(), any())).thenReturn(null)
 
-        testee.iconReceived(bitmap)
+        testee.iconReceived("https://example.com", bitmap)
 
         verify(mockTabRepository, never()).updateTabFavicon(any(), any())
     }
 
+    @Test
+    fun whenIconReceivedFromPreviousUrkThenDontUpdateTabFavicon() = coroutineRule.runBlocking {
+        givenOneActiveTabSelected()
+        val bitmap: Bitmap = Bitmap.createBitmap(1, 1, Bitmap.Config.RGB_565)
+        val file = File("test")
+        whenever(mockFaviconManager.saveToTemp(any(), any(), any())).thenReturn(file)
+
+        testee.iconReceived("https://notexample.com", bitmap)
+
+        verify(mockPixel).enqueueFire(AppPixelName.FAVICON_WRONG_URL_ERROR)
+        verify(mockTabRepository, never()).updateTabFavicon("TAB_ID", file.name)
+    }
+
     @Test
     fun whenOnSiteLocationPermissionSelectedAndPermissionIsAllowAlwaysThenPersistFavicon() = coroutineRule.runBlocking {
         val url = "http://example.com"

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

@@ -85,7 +85,7 @@ class BrowserChromeClient @Inject constructor(private val uncaughtExceptionRepos
     }
 
     override fun onReceivedIcon(webView: WebView, icon: Bitmap) {
-        webViewClientListener?.iconReceived(icon)
+        webViewClientListener?.iconReceived(webView.url, icon)
     }
 
     override fun onReceivedTitle(view: WebView, title: String) {

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

@@ -627,9 +627,15 @@ class BrowserTabViewModel(
         }
     }
 
-    override fun iconReceived(icon: Bitmap) {
+    override fun iconReceived(url: String, icon: Bitmap) {
         val currentTab = tabRepository.liveSelectedTab.value ?: return
-        val url = currentTab.url ?: return
+        val currentUrl = currentTab.url ?: return
+        if (currentUrl != url) {
+            pixel.enqueueFire(AppPixelName.FAVICON_WRONG_URL_ERROR)
+            Timber.d("Favicon received for a url $url, different than the current one $currentUrl")
+            return
+        }
+
         viewModelScope.launch {
             val faviconFile = faviconManager.saveToTemp(currentTab.tabId, icon, url)
             faviconFile?.let {

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

@@ -59,6 +59,6 @@ interface WebViewClientListener {
 
     fun loginDetected()
     fun dosAttackDetected()
-    fun iconReceived(icon: Bitmap)
+    fun iconReceived(url: String, icon: Bitmap)
     fun prefetchFavicon(url: String)
 }

app/src/main/java/com/duckduckgo/app/browser/favicon/FaviconDownloader.kt

@@ -84,4 +84,5 @@ class GlideFaviconDownloader @Inject constructor(
             view.setImageDrawable(ContextCompat.getDrawable(view.context, R.drawable.ic_globe_gray_16dp))
         }
     }
+
 }

app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt

@@ -212,5 +212,8 @@ enum class AppPixelName(override val pixelName: String) : Pixel.PixelName {
 
     EMAIL_TOOLTIP_DISMISSED("m_e_t_d"),
     EMAIL_USE_ALIAS("m_e_ua"),
-    EMAIL_USE_ADDRESS("m_e_uad")
+    EMAIL_USE_ADDRESS("m_e_uad"),
+
+    FAVICON_WRONG_URL_ERROR("m_fwu_r")
+
 }