Merge branch 'release/5.13.0'

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

@@ -16,9 +16,13 @@
 
 package com.duckduckgo.app.browser
 
+import android.content.Context
 import android.support.test.InstrumentationRegistry
+import android.support.test.annotation.UiThreadTest
 import android.view.View
 import android.webkit.WebChromeClient
+import android.webkit.WebView
+import com.nhaarman.mockito_kotlin.any
 import com.nhaarman.mockito_kotlin.mock
 import com.nhaarman.mockito_kotlin.times
 import com.nhaarman.mockito_kotlin.verify
@@ -28,14 +32,17 @@ import org.junit.Test
 class BrowserChromeClientTest {
 
     private lateinit var testee: BrowserChromeClient
+    private lateinit var webView: TestWebView
     private lateinit var mockWebViewClientListener: WebViewClientListener
     private val fakeView = View(InstrumentationRegistry.getTargetContext())
 
+    @UiThreadTest
     @Before
     fun setup() {
         testee = BrowserChromeClient()
         mockWebViewClientListener = mock()
         testee.webViewClientListener = mockWebViewClientListener
+        webView = TestWebView(InstrumentationRegistry.getTargetContext())
     }
 
     @Test
@@ -66,4 +73,59 @@ class BrowserChromeClientTest {
         testee.onHideCustomView()
         verify(mockWebViewClientListener).exitFullScreen()
     }
+
+    @UiThreadTest
+    @Test
+    fun whenOnProgressChangedCalledThenListenerInstructedToUpdateProgress() {
+        testee.onProgressChanged(webView, 10)
+        verify(mockWebViewClientListener).progressChanged(10)
+    }
+
+    @UiThreadTest
+    @Test
+    fun whenOnProgressChangedCalledButNoUrlChangeThenListenerInstructedToUpdateProgressASecondTime() {
+        webView.stubUrl = "foo.com"
+        testee.onProgressChanged(webView, 10)
+        testee.onProgressChanged(webView, 20)
+        verify(mockWebViewClientListener, times(2)).progressChanged(any())
+    }
+
+    @UiThreadTest
+    @Test
+    fun whenOnProgressChangedCalledAfterUrlChangeThenListenerInstructedToUpdateProgressAgain() {
+        webView.stubUrl = "foo.com"
+        testee.onProgressChanged(webView, 10)
+        testee.onProgressChanged(webView, 20)
+        webView.stubUrl = "bar.com"
+        testee.onProgressChanged(webView, 30)
+        verify(mockWebViewClientListener, times(3)).progressChanged(any())
+    }
+
+    @UiThreadTest
+    @Test
+    fun whenOnProgressChangedCalledThenListenerInstructedToUpdateUrl() {
+        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())
+    }
+
+    private class TestWebView(context: Context) : WebView(context) {
+        var stubUrl: String = ""
+
+        override fun getUrl(): String {
+            return stubUrl
+        }
+    }
 }

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

@@ -17,10 +17,8 @@
 package com.duckduckgo.app.browser
 
 import android.content.Context
-import android.os.Build
 import android.support.test.InstrumentationRegistry
 import android.support.test.annotation.UiThreadTest
-import android.support.test.filters.SdkSuppress
 import android.webkit.WebView
 import com.duckduckgo.app.httpsupgrade.HttpsUpgrader
 import com.duckduckgo.app.statistics.pixels.Pixel
@@ -63,35 +61,11 @@ class BrowserWebViewClientTest {
 
     @UiThreadTest
     @Test
-    @SdkSuppress(minSdkVersion = Build.VERSION_CODES.M)
-    fun whenOnPageStartedCalledOnNewerDevicesThenListenerNotInstructedToUpdateUrl() {
+    fun whenOnPageStartedCalledThenListenerNeverInstructedToUpdateUrl() {
         testee.onPageStarted(webView, EXAMPLE_URL, null)
         verify(listener, never()).urlChanged(any())
     }
 
-    @UiThreadTest
-    @Test
-    @SdkSuppress(maxSdkVersion = Build.VERSION_CODES.LOLLIPOP_MR1)
-    fun whenOnPageStartedCalledOnOlderDevicesThenListenerInstructedToUpdateUrl() {
-        testee.onPageStarted(webView, EXAMPLE_URL, null)
-        verify(listener).urlChanged(any())
-    }
-
-    @SdkSuppress(minSdkVersion = Build.VERSION_CODES.M)
-    @UiThreadTest
-    @Test
-    fun whenOnPageCommitVisibleCalledThenListenerInstructedToUpdateUrl() {
-        testee.onPageCommitVisible(webView, EXAMPLE_URL)
-        verify(listener).urlChanged(EXAMPLE_URL)
-    }
-
-    @UiThreadTest
-    @Test
-    fun whenOnPageCommitVisibleCalledThenListenerInstructedToUpdateNavigationOptions() {
-        testee.onPageCommitVisible(webView, EXAMPLE_URL)
-        verify(listener).navigationOptionsChanged(any())
-    }
-
     @UiThreadTest
     @Test
     fun whenOnPageFinishedCalledThenListenerNotified() {
@@ -106,6 +80,13 @@ class BrowserWebViewClientTest {
         verify(listener).navigationOptionsChanged(any())
     }
 
+    @UiThreadTest
+    @Test
+    fun whenOnPageFinishedCalledThenListenerInstructedToUpdateUrl() {
+        testee.onPageFinished(webView, EXAMPLE_URL)
+        verify(listener).urlChanged(EXAMPLE_URL)
+    }
+
     private class TestWebView(context: Context) : WebView(context)
 
     companion object {

app/src/androidTest/java/com/duckduckgo/app/settings/SettingsViewModelTest.kt

@@ -28,7 +28,6 @@ import com.duckduckgo.app.settings.SettingsViewModel.Command
 import com.duckduckgo.app.settings.db.SettingsDataStore
 import com.duckduckgo.app.statistics.Variant
 import com.duckduckgo.app.statistics.VariantManager
-import com.duckduckgo.app.statistics.VariantManager.VariantFeature.ThemeFeature.ThemeToggle
 import com.duckduckgo.app.statistics.pixels.Pixel
 import com.nhaarman.mockito_kotlin.*
 import org.junit.Assert.*
@@ -208,19 +207,5 @@ class SettingsViewModelTest {
         assertEquals(expectedStartString, latestViewState().version)
     }
 
-    @Test
-    fun whenThemeToggleFeatureExistsThenThemeToggleIsShown() {
-        whenever(mockVariantManager.getVariant()).thenReturn(Variant("aa", 1.0, listOf(ThemeToggle)))
-        testee.start()
-        assertTrue(latestViewState().showThemeToggle)
-    }
-
-    @Test
-    fun whenThemeToggleFeatureDoesNotExistThenThemeToggleIsNotShown() {
-        whenever(mockVariantManager.getVariant()).thenReturn(Variant("aa", 1.0, emptyList()))
-        testee.start()
-        assertFalse(latestViewState().showThemeToggle)
-    }
-
     private fun latestViewState() = testee.viewState.value!!
 }

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

@@ -32,7 +32,7 @@ class BrowserChromeClient @Inject constructor() : WebChromeClient() {
     private var customView: View? = null
 
     override fun onShowCustomView(view: View, callback: CustomViewCallback?) {
-        Timber.i("on show custom view")
+        Timber.d("on show custom view")
 
         if (customView != null) {
             callback?.onCustomViewHidden()
@@ -44,14 +44,22 @@ class BrowserChromeClient @Inject constructor() : WebChromeClient() {
     }
 
     override fun onHideCustomView() {
-        Timber.i("hide custom view")
+        Timber.d("on hide custom view")
 
         webViewClientListener?.exitFullScreen()
         customView = null
     }
 
     override fun onProgressChanged(webView: WebView, newProgress: Int) {
+        Timber.d("onProgressChanged - $newProgress - ${webView.url}")
+
         webViewClientListener?.progressChanged(newProgress)
+
+        val currentUrl = webViewClientListener?.currentUrl()
+        if (currentUrl != webView.url) {
+            Timber.i("Url has changed from $currentUrl to ${webView.url}")
+            webViewClientListener?.urlChanged(webView.url)
+        }
     }
 
     override fun onReceivedTitle(view: WebView, title: String) {

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

@@ -85,7 +85,6 @@ class BrowserTabViewModel(
     private val addToHomeCapabilityDetector: AddToHomeCapabilityDetector,
     appConfigurationDao: AppConfigurationDao
 ) : WebViewClientListener, SaveBookmarkListener, ViewModel() {
-
     data class GlobalLayoutViewState(
         val isNewTabState: Boolean = true
     )
@@ -343,7 +342,11 @@ class BrowserTabViewModel(
             findInPageViewState.value = FindInPageViewState(visible = false, canFindInPage = false)
 
             val currentBrowserViewState = currentBrowserViewState()
-            browserViewState.value = currentBrowserViewState.copy(canAddBookmarks = false, addToHomeEnabled = false, addToHomeVisible = addToHomeCapabilityDetector.isAddToHomeSupported())
+            browserViewState.value = currentBrowserViewState.copy(
+                canAddBookmarks = false,
+                addToHomeEnabled = false,
+                addToHomeVisible = addToHomeCapabilityDetector.isAddToHomeSupported()
+            )
 
             return
         }
@@ -367,8 +370,10 @@ class BrowserTabViewModel(
             statisticsUpdater.refreshRetentionAtb()
         }
 
-        site = siteFactory.build(url)
-        onSiteChanged()
+        if (currentUrl() != url) {
+            site = siteFactory.build(url)
+            onSiteChanged()
+        }
     }
 
     private fun omnibarTextForUrl(url: String): String {
@@ -557,6 +562,10 @@ class BrowserTabViewModel(
         }
     }
 
+    override fun currentUrl(): String? {
+        return site?.url
+    }
+
     fun resetView() {
         site = null
         url.value = null

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

@@ -52,8 +52,6 @@ class BrowserWebViewClient @Inject constructor(
 
     private var currentUrl: String? = null
 
-    private val willGetNotifiedOfPageCommits = Build.VERSION.SDK_INT >= Build.VERSION_CODES.M
-
     /**
      * This is the new method of url overriding available from API 24 onwards
      */
@@ -109,33 +107,27 @@ class BrowserWebViewClient @Inject constructor(
     }
 
     override fun onPageStarted(webView: WebView, url: String?, favicon: Bitmap?) {
-        Timber.d("onPageStarted $url")
-
-        webViewClientListener?.loadingStarted()
+        Timber.d("\nonPageStarted {\nurl: $url\nwebView.url: ${webView.url}\n}\n")
 
-        if (!willGetNotifiedOfPageCommits) onUrlChanged(url, webView)
+        currentUrl = url
+        webViewClientListener?.let {
+            it.loadingStarted()
+            it.navigationOptionsChanged(determineNavigationOptions(webView))
+        }
 
-        val uri = if (url != null) Uri.parse(url) else null
+        val uri = if (currentUrl != null) Uri.parse(currentUrl) else null
         if (uri != null) {
             reportHttpsIfInUpgradeList(uri)
         }
     }
 
-    /**
-     * Note, this method is only called on APIs >= 23
-     * While this is the ideal time to indicate the URL has changed, on lower APIs we need to handle that instead in onPageStarted()
-     */
-    override fun onPageCommitVisible(webView: WebView, url: String?) {
-        Timber.d("onPageCommitVisible $url")
-
-        onUrlChanged(url, webView)
-    }
-
     override fun onPageFinished(webView: WebView, url: String?) {
         Timber.d("onPageFinished $url")
 
+        currentUrl = url
         webViewClientListener?.let {
             it.loadingFinished(url)
+            it.urlChanged(url)
             it.navigationOptionsChanged(determineNavigationOptions(webView))
         }
     }
@@ -222,14 +214,6 @@ class BrowserWebViewClient @Inject constructor(
         return BrowserNavigationOptions(canGoBack, canGoForward)
     }
 
-    private fun onUrlChanged(url: String?, webView: WebView) {
-        currentUrl = url
-        webViewClientListener?.let {
-            it.urlChanged(url)
-            it.navigationOptionsChanged(determineNavigationOptions(webView))
-        }
-    }
-
     data class BrowserNavigationOptions(val canGoBack: Boolean, val canGoForward: Boolean)
 
 }

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

@@ -24,6 +24,8 @@ import com.duckduckgo.app.browser.BrowserWebViewClient.BrowserNavigationOptions
 import com.duckduckgo.app.trackerdetection.model.TrackingEvent
 
 interface WebViewClientListener {
+    fun currentUrl(): String?
+
     fun loadingStarted()
     fun loadingFinished(url: String? = null)
     fun progressChanged(newProgress: Int)

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

@@ -24,7 +24,6 @@ import android.support.v4.content.LocalBroadcastManager
 import android.support.v7.app.AppCompatActivity
 import android.view.MenuItem
 import com.duckduckgo.app.settings.db.SettingsDataStore
-import com.duckduckgo.app.statistics.VariantManager
 import dagger.android.AndroidInjection
 import javax.inject.Inject
 
@@ -37,14 +36,11 @@ abstract class DuckDuckGoActivity : AppCompatActivity() {
     @Inject
     lateinit var settingsDataStore: SettingsDataStore
 
-    @Inject
-    lateinit var variantManager: VariantManager
-
     private var themeChangeReceiver: BroadcastReceiver? = null
 
     override fun onCreate(savedInstanceState: Bundle?) {
         AndroidInjection.inject(this)
-        themeChangeReceiver = applyTheme(settingsDataStore, variantManager.getVariant())
+        themeChangeReceiver = applyTheme(settingsDataStore)
         super.onCreate(savedInstanceState)
     }
 

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

@@ -38,7 +38,6 @@ import com.duckduckgo.app.httpsupgrade.HttpsUpgrader
 import com.duckduckgo.app.job.AppConfigurationSyncer
 import com.duckduckgo.app.migration.LegacyMigration
 import com.duckduckgo.app.settings.db.SettingsDataStore
-import com.duckduckgo.app.statistics.VariantManager
 import com.duckduckgo.app.statistics.api.StatisticsUpdater
 import com.duckduckgo.app.statistics.pixels.Pixel
 import com.duckduckgo.app.statistics.pixels.Pixel.PixelName.APP_LAUNCH
@@ -90,10 +89,7 @@ open class DuckDuckGoApplication : HasActivityInjector, HasServiceInjector, HasS
 
     @Inject
     lateinit var notificationRegistrar: NotificationRegistrar
-
-    @Inject
-    lateinit var variantManager: VariantManager
-
+
     @Inject
     lateinit var pixel: Pixel
 
@@ -127,7 +123,7 @@ open class DuckDuckGoApplication : HasActivityInjector, HasServiceInjector, HasS
         }
 
         initializeStatistics()
-        initializeTheme(settingsDataStore, variantManager.getVariant())
+        initializeTheme(settingsDataStore)
         loadTrackerData()
         configureDataDownloader()
         recordInstallationTimestamp()