Show Duck Player icon and not privacy shield in custom tabs (#5066)

Task/Issue URL: https://app.asana.com/0/1207252092703676/1208391013610683/f ### Description * Show Duck Player icon and not privacy shield in custom tabs * Fix bug where tracking links redirecting to YouTube wouldn't trigger Duck Player * Display duck://player instead of youtube-nocookie in custom tabs ### Steps to test this PR _Feature 1_ - [x] Set Duck Player settings to Always - [x] Open a YouTube video in a custom tab - [x] Check Duck Player icon is displayed instead of the privacy shield Task/Issue URL: https://app.asana.com/0/1207252092703676/1208391013610683/f _Feature 2_ - [x] Set Duck Player settings to Always enabled - [x] Open https://clicks.aweber.com/y/ct/?l=pbFxrn&m=gzpbMhiix7McQMQ&b=cIoUrwlB2qpdga9S7Y8fWQ - [x] Check Duck Player is launched - [x] Navigate back - [x] Check you're taken to the previous page (or the customtab is closed if the navigation was initiated from an external link) _Feature 3_ - [x] Open Duck Player in a custom tab - [x] Check the domain displayed is duck://player and not youtube-nocookie.com ### UI changes ![merge](https://github.com/user-attachments/assets/0c9c7313-d0f0-40f0-92fb-2d33f18bcdbe) ![merge](https://github.com/user-attachments/assets/1d164069-86f6-4b56-be4b-436af1e945b9)
duckduckgo · Sep 26, 2024 · f89fc55 · f89fc55
1 parent f5fab29
commit f89fc55
Show file tree

Hide file tree

Showing 19 changed files with 187 additions and 44 deletions.
diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserChromeClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserChromeClientTest.kt
@@ -163,7 +163,7 @@ class BrowserChromeClientTest {
     fun whenOnReceivedTitleThenTitleReceived() {
         val title = "title"
         testee.onReceivedTitle(webView, title)
-        verify(mockWebViewClientListener).titleReceived(title, webView.url)
+        verify(mockWebViewClientListener).titleReceived(title)
     }
 
     @Test

diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
@@ -550,6 +550,7 @@ class BrowserTabViewModelTest {
             coroutineRule.testScope,
             coroutineRule.testDispatcherProvider,
             DuckDuckGoUrlDetectorImpl(),
+            mockDuckPlayer,
         )
 
         accessibilitySettingsDataStore = AccessibilitySettingsSharedPreferences(
@@ -1911,13 +1912,42 @@ class BrowserTabViewModelTest {
         )
         whenever(mockSavedSitesRepository.insertBookmark(title = anyString(), url = anyString())).thenReturn(bookmark)
         loadUrl(url = url)
-        testee.titleReceived(newTitle = title, url = url)
+        testee.titleReceived(newTitle = title)
         testee.onBookmarkMenuClicked()
         val command = captureCommands().lastValue as Command.ShowSavedSiteAddedConfirmation
         assertEquals(url, command.savedSiteChangedViewState.savedSite.url)
         assertEquals(title, command.savedSiteChangedViewState.savedSite.title)
     }
 
+    @Test
+    fun whenSiteLoadedWithSimulatedYouTubeNoCookieAndDuckPlayerEnabledThenShowWebPageTitleWithDuckPlayerIcon() = runTest {
+        val url = "http://youtube-nocookie.com/videoID=1234"
+        val title = "Duck Player"
+        whenever(mockDuckPlayer.isDuckPlayerUri(anyString())).thenReturn(true)
+        whenever(mockDuckPlayer.isSimulatedYoutubeNoCookie(anyUri())).thenReturn(true)
+        whenever(mockDuckPlayer.createDuckPlayerUriFromYoutubeNoCookie(any())).thenReturn("duck://player/1234")
+        whenever(mockDuckPlayer.getDuckPlayerState()).thenReturn(ENABLED)
+
+        loadUrl(url = url)
+        testee.titleReceived(newTitle = title)
+        val command = captureCommands().lastValue as Command.ShowWebPageTitle
+        assertTrue(command.showDuckPlayerIcon)
+        assertEquals("duck://player/1234", command.url)
+    }
+
+    @Test
+    fun whenSiteLoadedWithDuckPlayerDisabledThenShowWebPageTitleWithoutDuckPlayerIcon() = runTest {
+        val url = "http://youtube-nocookie.com/videoID=1234"
+        val title = "Duck Player"
+        whenever(mockDuckPlayer.getDuckPlayerState()).thenReturn(DISABLED)
+
+        loadUrl(url = url)
+        testee.titleReceived(newTitle = title)
+        val command = captureCommands().lastValue as Command.ShowWebPageTitle
+        assertFalse(command.showDuckPlayerIcon)
+        assertEquals("http://youtube-nocookie.com/videoID=1234", command.url)
+    }
+
     @Test
     fun whenNoSiteAndUserSelectsToAddBookmarkThenBookmarkIsNotAdded() = runTest {
         val bookmark = Bookmark(
@@ -6054,8 +6084,6 @@ class BrowserTabViewModelTest {
         title: String? = null,
         isBrowserShowing: Boolean = true,
     ) = runTest {
-        whenever(mockDuckPlayer.isSimulatedYoutubeNoCookie(anyUri())).thenReturn(false)
-        whenever(mockDuckPlayer.getDuckPlayerState()).thenReturn(DISABLED)
         whenever(mockDuckPlayer.observeUserPreferences()).thenReturn(flowOf(UserPreferences(false, Disabled)))
 
         setBrowserShowing(isBrowserShowing)

diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt
@@ -35,6 +35,7 @@ import com.duckduckgo.app.tabs.store.TabSwitcherDataStore
 import com.duckduckgo.app.trackerdetection.EntityLookup
 import com.duckduckgo.common.test.CoroutineTestRule
 import com.duckduckgo.common.test.InstantSchedulersRule
+import com.duckduckgo.duckplayer.api.DuckPlayer
 import com.duckduckgo.privacy.config.api.ContentBlocking
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.flow.consumeAsFlow
@@ -61,6 +62,8 @@ class TabDataRepositoryTest {
 
     private val mockDao: TabsDao = mock()
 
+    private val mockDuckPlayer: DuckPlayer = mock()
+
     private val daoDeletableTabs = Channel<List<TabEntity>>()
 
     @After
@@ -438,6 +441,7 @@ class TabDataRepositoryTest {
                 coroutinesTestRule.testScope,
                 coroutinesTestRule.testDispatcherProvider,
                 duckDuckGoUrlDetector,
+                mockDuckPlayer,
             ),
             webViewPreviewPersister,
             faviconManager,

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserChromeClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserChromeClient.kt
@@ -111,7 +111,7 @@ class BrowserChromeClient @Inject constructor(
         view: WebView,
         title: String,
     ) {
-        webViewClientListener?.titleReceived(title, view.url)
+        webViewClientListener?.titleReceived(title)
     }
 
     override fun onShowFileChooser(

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
@@ -1626,7 +1626,7 @@ class BrowserTabFragment :
             is Command.ScreenLock -> screenLock(it.data)
             is Command.ScreenUnlock -> screenUnlock()
             is Command.ShowFaviconsPrompt -> showFaviconsPrompt()
-            is Command.ShowWebPageTitle -> showWebPageTitleInCustomTab(it.title, it.url)
+            is Command.ShowWebPageTitle -> showWebPageTitleInCustomTab(it.title, it.url, it.showDuckPlayerIcon)
             is Command.ShowSSLError -> showSSLWarning(it.handler, it.error)
             is Command.HideSSLError -> hideSSLWarning()
             is Command.LaunchScreen -> launchScreen(it.screen, it.payload)
@@ -1725,6 +1725,7 @@ class BrowserTabFragment :
     private fun showWebPageTitleInCustomTab(
         title: String,
         url: String?,
+        showDuckPlayerIcon: Boolean,
     ) {
         if (isActiveCustomTab()) {
             omnibar.customTabToolbarContainer.customTabTitle.text = title
@@ -1737,6 +1738,8 @@ class BrowserTabFragment :
             omnibar.customTabToolbarContainer.customTabTitle.show()
             omnibar.customTabToolbarContainer.customTabDomainOnly.hide()
             omnibar.customTabToolbarContainer.customTabDomain.show()
+            omnibar.customTabToolbarContainer.customTabShieldIcon.isInvisible = showDuckPlayerIcon
+            omnibar.customTabToolbarContainer.customTabDuckPlayerIcon.isVisible = showDuckPlayerIcon
         }
     }
 

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt
@@ -1895,10 +1895,16 @@ class BrowserTabViewModel @Inject constructor(
         showErrorWithAction(R.string.dosErrorMessage)
     }
 
-    override fun titleReceived(newTitle: String, url: String?) {
+    override fun titleReceived(newTitle: String) {
         site?.title = newTitle
-        command.postValue(ShowWebPageTitle(newTitle, url))
-        onSiteChanged()
+        val url = site?.url
+        viewModelScope.launch(dispatchers.main()) {
+            val isDuckPlayerUrl = withContext(dispatchers.io()) {
+                url != null && duckPlayer.getDuckPlayerState() == ENABLED && duckPlayer.isDuckPlayerUri(url)
+            }
+            command.postValue(ShowWebPageTitle(newTitle, url, isDuckPlayerUrl))
+            onSiteChanged()
+        }
     }
 
     @AnyThread

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt
@@ -80,10 +80,7 @@ import com.duckduckgo.subscriptions.api.Subscriptions
 import com.duckduckgo.user.agent.api.ClientBrandHintProvider
 import java.net.URI
 import javax.inject.Inject
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
-import kotlinx.coroutines.runBlocking
-import kotlinx.coroutines.withContext
+import kotlinx.coroutines.*
 import timber.log.Timber
 
 private const val ABOUT_BLANK = "about:blank"
@@ -133,7 +130,7 @@ class BrowserWebViewClient @Inject constructor(
         request: WebResourceRequest,
     ): Boolean {
         val url = request.url
-        return shouldOverride(view, url, request.isForMainFrame)
+        return shouldOverride(view, url, request.isForMainFrame, request.isRedirect)
     }
 
     /**
@@ -143,6 +140,7 @@ class BrowserWebViewClient @Inject constructor(
         webView: WebView,
         url: Uri,
         isForMainFrame: Boolean,
+        isRedirect: Boolean,
     ): Boolean {
         try {
             Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url")
@@ -180,7 +178,18 @@ class BrowserWebViewClient @Inject constructor(
                     }
                     false
                 }
-
+                is SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink -> {
+                    if (isRedirect) {
+                        /*
+                        This forces shouldInterceptRequest to be called with the YouTube URL, otherwise that method is never executed and
+                        therefore the Duck Player page is never launched if YouTube comes from a redirect.
+                         */
+                        webView.loadUrl(url.toString())
+                        return true
+                    } else {
+                        shouldOverrideWebRequest(url, webView, isForMainFrame)
+                    }
+                }
                 is SpecialUrlDetector.UrlType.NonHttpAppLink -> {
                     Timber.i("Found non-http app link for ${urlType.uriString}")
                     if (isForMainFrame) {
@@ -201,29 +210,7 @@ class BrowserWebViewClient @Inject constructor(
 
                 is SpecialUrlDetector.UrlType.SearchQuery -> false
                 is SpecialUrlDetector.UrlType.Web -> {
-                    if (requestRewriter.shouldRewriteRequest(url)) {
-                        webViewClientListener?.let { listener ->
-                            val newUri = requestRewriter.rewriteRequestWithCustomQueryParams(url)
-                            loadUrl(listener, webView, newUri.toString())
-                            return true
-                        }
-                    }
-                    if (isForMainFrame) {
-                        webViewClientListener?.let { listener ->
-                            listener.willOverrideUrl(url.toString())
-                            clientProvider?.let { provider ->
-                                if (provider.shouldChangeBranding(url.toString())) {
-                                    provider.setOn(webView.settings, url.toString())
-                                    loadUrl(listener, webView, url.toString())
-                                    return true
-                                } else {
-                                    return false
-                                }
-                            }
-                            return false
-                        }
-                    }
-                    false
+                    shouldOverrideWebRequest(url, webView, isForMainFrame)
                 }
 
                 is SpecialUrlDetector.UrlType.ExtractedAmpLink -> {
@@ -295,6 +282,36 @@ class BrowserWebViewClient @Inject constructor(
         }
     }
 
+    private fun shouldOverrideWebRequest(
+        url: Uri,
+        webView: WebView,
+        isForMainFrame: Boolean,
+    ): Boolean {
+        if (requestRewriter.shouldRewriteRequest(url)) {
+            webViewClientListener?.let { listener ->
+                val newUri = requestRewriter.rewriteRequestWithCustomQueryParams(url)
+                loadUrl(listener, webView, newUri.toString())
+                return true
+            }
+        }
+        if (isForMainFrame) {
+            webViewClientListener?.let { listener ->
+                listener.willOverrideUrl(url.toString())
+                clientProvider?.let { provider ->
+                    if (provider.shouldChangeBranding(url.toString())) {
+                        provider.setOn(webView.settings, url.toString())
+                        loadUrl(listener, webView, url.toString())
+                        return true
+                    } else {
+                        return false
+                    }
+                }
+                return false
+            }
+        }
+        return false
+    }
+
     @UiThread
     override fun onPageCommitVisible(webView: WebView, url: String) {
         Timber.v("onPageCommitVisible webViewUrl: ${webView.url} URL: $url progress: ${webView.progress}")

diff --git a/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt b/app/src/main/java/com/duckduckgo/app/browser/SpecialUrlDetector.kt
@@ -94,7 +94,9 @@ class SpecialUrlDetectorImpl(
 
         val willNavigateToDuckPlayer = runBlocking { willNavigateToDuckPlayerDeferred.await() }
 
-        if (!willNavigateToDuckPlayer) {
+        if (willNavigateToDuckPlayer) {
+            return UrlType.ShouldLaunchDuckPlayerLink(url = uri)
+        } else {
             try {
                 val browsableIntent = Intent.parseUri(uriString, URI_ANDROID_APP_SCHEME).apply {
                     addCategory(Intent.CATEGORY_BROWSABLE)

diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt
@@ -51,7 +51,7 @@ interface WebViewClientListener {
         callback: GeolocationPermissions.Callback,
     )
 
-    fun titleReceived(newTitle: String, url: String?)
+    fun titleReceived(newTitle: String)
     fun trackerDetected(event: TrackingEvent)
     fun pageHasHttpResources(page: String)
     fun pageHasHttpResources(page: Uri)

diff --git a/app/src/main/java/com/duckduckgo/app/browser/commands/Command.kt b/app/src/main/java/com/duckduckgo/app/browser/commands/Command.kt
@@ -157,6 +157,7 @@ sealed class Command {
     class ShowWebPageTitle(
         val title: String,
         val url: String?,
+        val showDuckPlayerIcon: Boolean = false,
     ) : Command()
     class CheckSystemLocationPermission(
         val domain: String,

diff --git a/app/src/main/java/com/duckduckgo/app/global/model/SiteFactoryImpl.kt b/app/src/main/java/com/duckduckgo/app/global/model/SiteFactoryImpl.kt
@@ -27,6 +27,7 @@ import com.duckduckgo.app.privacy.db.UserAllowListRepository
 import com.duckduckgo.app.trackerdetection.EntityLookup
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.di.scopes.AppScope
+import com.duckduckgo.duckplayer.api.DuckPlayer
 import com.duckduckgo.privacy.config.api.ContentBlocking
 import com.squareup.anvil.annotations.ContributesBinding
 import dagger.SingleInstanceIn
@@ -43,6 +44,7 @@ class SiteFactoryImpl @Inject constructor(
     @AppCoroutineScope private val appCoroutineScope: CoroutineScope,
     private val dispatcherProvider: DispatcherProvider,
     private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector,
+    private val duckPlayer: DuckPlayer,
 ) : SiteFactory {
 
     private val siteCache = LruCache<String, Site>(1)
@@ -74,6 +76,7 @@ class SiteFactoryImpl @Inject constructor(
                 appCoroutineScope,
                 dispatcherProvider,
                 RealBrokenSiteContext(duckDuckGoUrlDetector),
+                duckPlayer,
             ).also {
                 siteCache.put(cacheKey, it)
             }

diff --git a/app/src/main/java/com/duckduckgo/app/global/model/SiteMonitor.kt b/app/src/main/java/com/duckduckgo/app/global/model/SiteMonitor.kt
@@ -34,6 +34,7 @@ import com.duckduckgo.app.trackerdetection.model.TrackingEvent
 import com.duckduckgo.browser.api.brokensite.BrokenSiteContext
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.common.utils.isHttps
+import com.duckduckgo.duckplayer.api.DuckPlayer
 import com.duckduckgo.privacy.config.api.ContentBlocking
 import java.util.concurrent.CopyOnWriteArrayList
 import kotlinx.coroutines.CoroutineScope
@@ -51,6 +52,7 @@ class SiteMonitor(
     appCoroutineScope: CoroutineScope,
     dispatcherProvider: DispatcherProvider,
     brokenSiteContext: BrokenSiteContext,
+    private val duckPlayer: DuckPlayer,
 ) : Site {
 
     override var url: String = url
@@ -163,6 +165,7 @@ class SiteMonitor(
 
     override fun privacyProtection(): PrivacyShield {
         userAllowList = domain?.let { isAllowListed(it) } ?: false
+        if (duckPlayer.isDuckPlayerUri(url)) return UNKNOWN
         if (userAllowList || !isHttps) return UNPROTECTED
 
         if (!fullSiteDetailsAvailable) {

diff --git a/app/src/main/res/layout/include_custom_tab_toolbar.xml b/app/src/main/res/layout/include_custom_tab_toolbar.xml
@@ -45,7 +45,19 @@
         android:importantForAccessibility="no"
         app:layout_constraintBottom_toBottomOf="parent"
         app:layout_constraintStart_toEndOf="@id/customTabCloseIcon"
-        app:layout_constraintTop_toTopOf="parent" />
+        app:layout_constraintTop_toTopOf="parent"/>
+
+    <ImageView
+        android:id="@+id/customTabDuckPlayerIcon"
+        android:layout_width="wrap_content"
+        android:layout_height="match_parent"
+        android:importantForAccessibility="no"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintStart_toEndOf="@id/customTabCloseIcon"
+        app:layout_constraintEnd_toStartOf="@+id/customTabTitle"
+        app:layout_constraintTop_toTopOf="parent"
+        android:src="@drawable/ic_duckplayer"
+        android:visibility="gone"/>
 
     <com.duckduckgo.common.ui.view.text.DaxTextView
         android:id="@+id/customTabTitle"