Merge branch 'release/5.19.0'

Author: subsymbolic

.github/CODEOWNERS

@@ -1,4 +1,4 @@
 #These people are listed as being the code owners for this project. As such, they will be automatically added as a reviewer when a PR is created.
 #For more details on how this is configured, see https://help.github.com/articles/about-code-owners/
 
-*       @CDRussell @subsymbolic @brindy
+*       @CDRussell @subsymbolic

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

@@ -78,7 +78,7 @@ class BrowserChromeClientTest {
     @Test
     fun whenOnProgressChangedCalledThenListenerInstructedToUpdateProgress() {
         testee.onProgressChanged(webView, 10)
-        verify(mockWebViewClientListener).progressChanged(10)
+        verify(mockWebViewClientListener).progressChanged(webView.stubUrl, 10)
     }
 
     @UiThreadTest
@@ -87,7 +87,7 @@ class BrowserChromeClientTest {
         webView.stubUrl = "foo.com"
         testee.onProgressChanged(webView, 10)
         testee.onProgressChanged(webView, 20)
-        verify(mockWebViewClientListener, times(2)).progressChanged(any())
+        verify(mockWebViewClientListener, times(2)).progressChanged(any(), any())
     }
 
     @UiThreadTest
@@ -98,27 +98,16 @@ class BrowserChromeClientTest {
         testee.onProgressChanged(webView, 20)
         webView.stubUrl = "bar.com"
         testee.onProgressChanged(webView, 30)
-        verify(mockWebViewClientListener, times(3)).progressChanged(any())
+        verify(mockWebViewClientListener, times(3)).progressChanged(any(), any())
     }
 
     @UiThreadTest
     @Test
-    fun whenOnProgressChangedCalledThenListenerInstructedToUpdateUrl() {
+    fun whenOnProgressChangedCalledThenPassedOnToWebClient() {
         val url = "https://example.com"
         webView.stubUrl = url
         testee.onProgressChanged(webView, 10)
-        verify(mockWebViewClientListener).urlChanged(url)
-    }
-
-    @UiThreadTest
-    @Test
-    fun whenOnProgressChangedCalledAfterUrlChangeThenListenerInstructedToUpdateUrlEveryTime() {
-        webView.stubUrl = "foo.com"
-        testee.onProgressChanged(webView, 10)
-        testee.onProgressChanged(webView, 20)
-        webView.stubUrl = "bar.com"
-        testee.onProgressChanged(webView, 30)
-        verify(mockWebViewClientListener, times(3)).urlChanged(any())
+        verify(mockWebViewClientListener).progressChanged(url, 10)
     }
 
     private class TestWebView(context: Context) : WebView(context) {

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

@@ -32,7 +32,7 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.Command.DisplayMessage
 import com.duckduckgo.app.browser.BrowserTabViewModel.Command.Navigate
 import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.DownloadFile
 import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.OpenInNewTab
-import com.duckduckgo.app.browser.addToHome.AddToHomeCapabilityDetector
+import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector
 import com.duckduckgo.app.browser.favicon.FaviconDownloader
 import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter
 import com.duckduckgo.app.browser.session.WebViewSessionStorage
@@ -92,9 +92,6 @@ class BrowserTabViewModelTest {
     @Mock
     private lateinit var mockStatisticsUpdater: StatisticsUpdater
 
-    @Mock
-    private lateinit var mockQueryObserver: Observer<String>
-
     @Mock
     private lateinit var mockCommandObserver: Observer<Command>
 
@@ -201,7 +198,6 @@ class BrowserTabViewModelTest {
         )
 
         testee.loadData("abc", null)
-        testee.url.observeForever(mockQueryObserver)
         testee.command.observeForever(mockCommandObserver)
 
         whenever(mockOmnibarConverter.convertQueryToUrl(any())).thenReturn("duckduckgo.com")
@@ -212,7 +208,6 @@ class BrowserTabViewModelTest {
     fun after() {
         testee.onCleared()
         db.close()
-        testee.url.removeObserver(mockQueryObserver)
         testee.command.removeObserver(mockCommandObserver)
     }
 
@@ -250,15 +245,15 @@ class BrowserTabViewModelTest {
 
     @Test
     fun whenViewBecomesVisibleWithActiveSiteThenKeyboardHidden() {
-        testee.url.value = "http://exmaple.com"
+        changeUrl("http://exmaple.com")
         testee.onViewVisible()
         verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture())
         assertTrue(commandCaptor.allValues.contains(Command.HideKeyboard))
     }
 
     @Test
     fun whenViewBecomesVisibleWithoutActiveSiteThenKeyboardShown() {
-        testee.url.value = null
+        changeUrl(null)
         testee.onViewVisible()
         verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture())
         assertTrue(commandCaptor.allValues.contains(Command.ShowKeyboard))
@@ -272,13 +267,13 @@ class BrowserTabViewModelTest {
 
     @Test
     fun whenUrlPresentThenAddBookmarkButtonEnabled() {
-        testee.urlChanged("www.example.com")
+        changeUrl("www.example.com")
         assertTrue(browserViewState().canAddBookmarks)
     }
 
     @Test
     fun whenNoUrlThenAddBookmarkButtonDisabled() {
-        testee.urlChanged(null)
+        changeUrl(null)
         assertFalse(browserViewState().canAddBookmarks)
     }
 
@@ -298,26 +293,27 @@ class BrowserTabViewModelTest {
     }
 
     @Test
-    fun whenEmptyInputQueryThenNoQueryMadeAvailableToActivity() {
+    fun whenEmptyInputQueryThenQueryNavigateCommandNotSubmittedToActivityActivity() {
         testee.onUserSubmittedQuery("")
-        verify(mockQueryObserver, never()).onChanged(ArgumentMatchers.anyString())
+        verify(mockCommandObserver, never()).onChanged(commandCaptor.capture())
     }
 
     @Test
-    fun whenBlankInputQueryThenNoQueryMadeAvailableToActivity() {
+    fun whenBlankInputQueryThenQueryNavigateCommandNotSubmittedToActivity() {
         testee.onUserSubmittedQuery("     ")
-        verify(mockQueryObserver, never()).onChanged(ArgumentMatchers.anyString())
+        verify(mockCommandObserver, never()).onChanged(commandCaptor.capture())
     }
 
     @Test
-    fun whenNonEmptyInputThenQueryMadeAvailableToActivity() {
+    fun whenNonEmptyInputThenNavigateCommandSubmittedToActivity() {
         testee.onUserSubmittedQuery("foo")
-        verify(mockQueryObserver).onChanged(ArgumentMatchers.anyString())
+        verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture())
+        assertTrue(commandCaptor.lastValue is Navigate)
     }
 
     @Test
     fun whenViewModelNotifiedThatWebViewIsLoadingThenViewStateIsUpdated() {
-        testee.loadingStarted()
+        testee.loadingStarted("http://example.com")
         assertTrue(loadingViewState().isLoading)
     }
 
@@ -327,10 +323,29 @@ class BrowserTabViewModelTest {
         assertFalse(loadingViewState().isLoading)
     }
 
+    @Test
+    fun whenLoadingFinishedAndInitialUrlNeverProgressedThenUrlUpdated() {
+        val initialUrl = "http://foo.com/abc"
+        val finalUrl = "http://bar.com/abc"
+        testee.loadingStarted(initialUrl)
+        testee.loadingFinished(finalUrl)
+        assertEquals(finalUrl, testee.url)
+    }
+
+    @Test
+    fun whenLoadingFinishedAndInitialUrlProgressedThenUrlNotUpdated() {
+        val initialUrl = "http://foo.com/abc"
+        val finalUrl = "http://foo.com/xyz"
+        testee.loadingStarted(initialUrl)
+        testee.progressChanged(initialUrl, 10)
+        testee.loadingFinished(finalUrl)
+        assertEquals(initialUrl, testee.url)
+    }
+
     @Test
     fun whenLoadingFinishedWithUrlThenSiteVisitedEntryAddedToLeaderboardDao() {
-        testee.url.value = "http://example.com/abc"
-        testee.loadingFinished(null)
+        testee.loadingStarted("http://example.com/abc")
+        testee.loadingFinished("http://example.com/abc")
         verify(mockNetworkLeaderboardDao).insert(SiteVisitedEntity("example.com"))
     }
 
@@ -351,7 +366,7 @@ class BrowserTabViewModelTest {
     @Test
     fun whenLoadingFinishedWithNoUrlThenOmnibarTextUpdatedToMatch() {
         val exampleUrl = "http://example.com/abc"
-        testee.urlChanged(exampleUrl)
+        changeUrl(exampleUrl)
         testee.loadingFinished(null)
         assertEquals(exampleUrl, omnibarViewState().omnibarText)
     }
@@ -385,65 +400,85 @@ class BrowserTabViewModelTest {
         assertEquals("", omnibarViewState().omnibarText)
     }
 
+    @Test
+    fun whenUrlStartsLoadingWithProgressChangeThenUrlUpdated() {
+        val url = "foo.com"
+        testee.loadingStarted(url)
+        testee.progressChanged(url, 10)
+        assertEquals(url, testee.url)
+    }
+
+    @Test
+    fun whenUrlStartsLoadingButProgressHasNotChangedThenUrlNotUpdated() {
+        testee.loadingStarted("foo.com")
+        assertNull(testee.url)
+    }
+
+    @Test
+    fun whenUrlHasNotStartedLoadingAndProgressChangeThenUrlNotUpdated() {
+        testee.progressChanged("foo.com", 10)
+        assertNull(testee.url)
+    }
+
     @Test
     fun whenUrlChangedThenViewStateIsUpdated() {
-        testee.urlChanged("duckduckgo.com")
+        changeUrl("duckduckgo.com")
         assertEquals("duckduckgo.com", omnibarViewState().omnibarText)
     }
 
     @Test
     fun whenUrlChangedWithDuckDuckGoUrlContainingQueryThenUrlRewrittenToContainQuery() {
-        testee.urlChanged("http://duckduckgo.com?q=test")
+        changeUrl("http://duckduckgo.com?q=test")
         assertEquals("test", omnibarViewState().omnibarText)
     }
 
     @Test
     fun whenUrlChangedWithDuckDuckGoUrlContainingQueryThenAtbRefreshed() {
-        testee.urlChanged("http://duckduckgo.com?q=test")
+        changeUrl("http://duckduckgo.com?q=test")
         verify(mockStatisticsUpdater).refreshRetentionAtb()
     }
 
     @Test
     fun whenUrlChangedWithDuckDuckGoUrlNotContainingQueryThenFullUrlShown() {
-        testee.urlChanged("http://duckduckgo.com")
+        changeUrl("http://duckduckgo.com")
         assertEquals("http://duckduckgo.com", omnibarViewState().omnibarText)
     }
 
     @Test
     fun whenUrlChangedWithNonDuckDuckGoUrlThenFullUrlShown() {
-        testee.urlChanged("http://example.com")
+        changeUrl("http://example.com")
         assertEquals("http://example.com", omnibarViewState().omnibarText)
     }
 
     @Test
     fun whenViewModelGetsProgressUpdateThenViewStateIsUpdated() {
-        testee.progressChanged(0)
+        testee.progressChanged("", 0)
         assertEquals(0, loadingViewState().progress)
 
-        testee.progressChanged(50)
+        testee.progressChanged("", 50)
         assertEquals(50, loadingViewState().progress)
 
-        testee.progressChanged(100)
+        testee.progressChanged("", 100)
         assertEquals(100, loadingViewState().progress)
     }
 
     @Test
     fun whenLoadingStartedThenPrivacyGradeIsCleared() {
-        testee.loadingStarted()
+        testee.loadingStarted("http://example.com")
         assertNull(testee.privacyGrade.value)
     }
 
     @Test
     fun whenUrlChangedThenPrivacyGradeIsReset() {
         val grade = testee.privacyGrade.value
-        testee.urlChanged("https://example.com")
+        changeUrl("https://example.com")
         assertNotEquals(grade, testee.privacyGrade.value)
     }
 
     @Test
     fun whenEnoughTrackersDetectedThenPrivacyGradeIsUpdated() {
         val grade = testee.privacyGrade.value
-        testee.urlChanged("https://example.com")
+        changeUrl("https://example.com")
         for (i in 1..10) {
             testee.trackerDetected(TrackingEvent("https://example.com", "", null, false))
         }
@@ -458,14 +493,14 @@ class BrowserTabViewModelTest {
     @Test
     fun whenUrlUpdatedAfterConfigDownloadThenPrivacyGradeIsShown() {
         testee.appConfigurationObserver.onChanged(AppConfigurationEntity(appConfigurationDownloaded = true))
-        testee.urlChanged((""))
+        changeUrl("")
         assertTrue(browserViewState().showPrivacyGrade)
     }
 
     @Test
     fun whenUrlUpdatedBeforeConfigDownloadThenPrivacyGradeIsShown() {
         testee.appConfigurationObserver.onChanged(AppConfigurationEntity(appConfigurationDownloaded = false))
-        testee.urlChanged((""))
+        changeUrl("")
         assertFalse(browserViewState().showPrivacyGrade)
     }
 
@@ -629,7 +664,7 @@ class BrowserTabViewModelTest {
     fun whenEnteringNonEmptyQueryThenHideKeyboardCommandIssued() {
         testee.onUserSubmittedQuery("foo")
         verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture())
-        assertTrue(commandCaptor.value == Command.HideKeyboard)
+        assertTrue(commandCaptor.allValues.any { it == Command.HideKeyboard })
     }
 
     @Test
@@ -760,7 +795,7 @@ class BrowserTabViewModelTest {
 
     @Test
     fun whenOnSiteAndBrokenSiteSelectedThenBrokenSiteFeedbackCommandSentWithUrl() {
-        testee.urlChanged("foo.com")
+        changeUrl("foo.com")
         testee.onBrokenSiteSelected()
         val command = captureCommands().value as Command.BrokenSiteFeedback
         assertEquals("foo.com", command.url)
@@ -796,14 +831,17 @@ class BrowserTabViewModelTest {
         whenever(mockOmnibarConverter.convertQueryToUrl("foo.com")).thenReturn("foo.com")
         whenever(webViewSessionStorage.restoreSession(anyOrNull(), anyString())).thenReturn(false)
         testee.restoreWebViewState(null, "foo.com")
-        assertEquals("foo.com", testee.url.value)
+
+        verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture())
+        val command = commandCaptor.lastValue as Navigate
+        assertEquals("foo.com", command.url)
     }
 
     @Test
     fun whenRestoringWebViewSessionNotRestorableAndNoPreviousUrlThenNoUrlLoaded() {
         whenever(webViewSessionStorage.restoreSession(anyOrNull(), anyString())).thenReturn(false)
         testee.restoreWebViewState(null, "")
-        assertNull(testee.url.value)
+        assertFalse(commandCaptor.allValues.any { it is Navigate })
     }
 
     @Test
@@ -813,6 +851,11 @@ class BrowserTabViewModelTest {
         assertFalse(globalLayoutViewState().isNewTabState)
     }
 
+    private fun changeUrl(url: String?) {
+        testee.loadingStarted(url)
+        testee.progressChanged(url, 100)
+    }
+
     private fun captureCommands(): ArgumentCaptor<Command> {
         verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture())
         return commandCaptor