From a51d19e117c281b2ef9baa23c52e068f5f24c28c Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Fri, 2 Feb 2024 16:58:21 -0500 Subject: [PATCH] Bug 1832357 - New window requests in custom tab should load within the same custom tab --- .../customtabs/CustomTabWindowFeature.kt | 59 +------------- .../customtabs/CustomTabWindowFeatureTest.kt | 81 +++---------------- .../feature/pwa/WebAppShortcutManager.kt | 2 +- .../customtabs/ExternalAppBrowserFragment.kt | 16 +--- 4 files changed, 16 insertions(+), 142 deletions(-) diff --git a/android-components/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabWindowFeature.kt b/android-components/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabWindowFeature.kt index 6b6e84137b43..376109ac5538 100644 --- a/android-components/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabWindowFeature.kt +++ b/android-components/components/feature/customtabs/src/main/java/mozilla/components/feature/customtabs/CustomTabWindowFeature.kt @@ -4,73 +4,27 @@ package mozilla.components.feature.customtabs -import android.app.Activity -import android.content.ActivityNotFoundException -import android.net.Uri -import androidx.annotation.VisibleForTesting -import androidx.annotation.VisibleForTesting.Companion.PRIVATE -import androidx.browser.customtabs.CustomTabColorSchemeParams -import androidx.browser.customtabs.CustomTabsIntent -import androidx.core.net.toUri import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.distinctUntilChangedBy import kotlinx.coroutines.flow.mapNotNull import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.selector.findCustomTab -import mozilla.components.browser.state.state.CustomTabConfig import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.window.WindowRequest import mozilla.components.lib.state.ext.flowScoped import mozilla.components.support.base.feature.LifecycleAwareFeature -const val SHORTCUT_CATEGORY = "mozilla.components.pwa.category.SHORTCUT" - /** * Feature implementation for handling window requests by opening custom tabs. */ class CustomTabWindowFeature( - private val activity: Activity, private val store: BrowserStore, private val sessionId: String, - internal val onLaunchUrlFallback: (Uri) -> Unit, ) : LifecycleAwareFeature { private var scope: CoroutineScope? = null - /** - * Transform a [CustomTabConfig] into a [CustomTabsIntent] that creates a - * new custom tab with the same styling and layout - */ - @Suppress("ComplexMethod") - @VisibleForTesting(otherwise = PRIVATE) - internal fun configToIntent(config: CustomTabConfig?): CustomTabsIntent { - val intent = CustomTabsIntent.Builder().apply { - setInstantAppsEnabled(false) - - val customTabColorSchemeBuilder = CustomTabColorSchemeParams.Builder() - config?.toolbarColor?.let { - customTabColorSchemeBuilder.setToolbarColor(it) - } - config?.navigationBarColor?.let { - customTabColorSchemeBuilder.setNavigationBarColor(it) - } - setDefaultColorSchemeParams(customTabColorSchemeBuilder.build()) - - if (config?.enableUrlbarHiding == true) setUrlBarHidingEnabled(true) - config?.closeButtonIcon?.let { setCloseButtonIcon(it) } - if (config?.showShareMenuItem == true) setShareState(CustomTabsIntent.SHARE_STATE_ON) - config?.titleVisible?.let { setShowTitle(it) } - config?.actionButtonConfig?.apply { setActionButton(icon, description, pendingIntent, tint) } - config?.menuItems?.forEach { addMenuItem(it.name, it.pendingIntent) } - }.build() - - intent.intent.`package` = activity.packageName - intent.intent.addCategory(SHORTCUT_CATEGORY) - - return intent - } - /** * Starts observing the configured session to listen for window requests. */ @@ -83,18 +37,7 @@ class CustomTabWindowFeature( .collect { state -> val windowRequest = state.content.windowRequest if (windowRequest?.type == WindowRequest.Type.OPEN) { - val intent = configToIntent(state.config) - val uri = windowRequest.url.toUri() - // This could only fail if the above intent is for our application - // and we are not registered to handle its schemes. - // Let's log this to better asses how often this happens in real world and - // if we need to add new schemes to properly support this workflow. - // See Fenix #8412 - try { - intent.launchUrl(activity, uri) - } catch (e: ActivityNotFoundException) { - onLaunchUrlFallback(uri) - } + state.engineState.engineSession?.loadUrl(windowRequest.url) store.dispatch(ContentAction.ConsumeWindowRequestAction(sessionId)) } } diff --git a/android-components/components/feature/customtabs/src/test/java/mozilla/components/feature/customtabs/CustomTabWindowFeatureTest.kt b/android-components/components/feature/customtabs/src/test/java/mozilla/components/feature/customtabs/CustomTabWindowFeatureTest.kt index f46c84612431..a57dbe701afd 100644 --- a/android-components/components/feature/customtabs/src/test/java/mozilla/components/feature/customtabs/CustomTabWindowFeatureTest.kt +++ b/android-components/components/feature/customtabs/src/test/java/mozilla/components/feature/customtabs/CustomTabWindowFeatureTest.kt @@ -6,24 +6,18 @@ package mozilla.components.feature.customtabs import android.app.Activity import android.content.ActivityNotFoundException -import android.graphics.Color -import android.net.Uri -import androidx.core.net.toUri import androidx.test.ext.junit.runners.AndroidJUnit4 import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.state.BrowserState -import mozilla.components.browser.state.state.CustomTabActionButtonConfig -import mozilla.components.browser.state.state.CustomTabConfig -import mozilla.components.browser.state.state.CustomTabMenuItem import mozilla.components.browser.state.state.createCustomTab import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.window.WindowRequest import mozilla.components.support.test.any import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.mock import mozilla.components.support.test.rule.MainCoroutineRule import mozilla.components.support.test.whenever -import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Rule import org.junit.Test @@ -31,7 +25,6 @@ import org.junit.runner.RunWith import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.verify -import org.mockito.Mockito.verifyNoInteractions @RunWith(AndroidJUnit4::class) class CustomTabWindowFeatureTest { @@ -42,17 +35,22 @@ class CustomTabWindowFeatureTest { private lateinit var store: BrowserStore private val sessionId = "session-uuid" private lateinit var activity: Activity - private val launchUrlFallback: (Uri) -> Unit = spy { _ -> } + private lateinit var engineSession: EngineSession @Before fun setup() { activity = mock() + engineSession = mock() store = spy( BrowserStore( BrowserState( customTabs = listOf( - createCustomTab(id = sessionId, url = "https://www.mozilla.org"), + createCustomTab( + id = sessionId, + url = "https://www.mozilla.org", + engineSession = engineSession, + ), ), ), ), @@ -62,8 +60,8 @@ class CustomTabWindowFeatureTest { } @Test - fun `given a request to open window, when the url can be handled, then the activity should start`() { - val feature = spy(CustomTabWindowFeature(activity, store, sessionId, launchUrlFallback)) + fun `given a request to open window, then url is loaded`() { + val feature = spy(CustomTabWindowFeature(store, sessionId)) val windowRequest: WindowRequest = mock() feature.start() @@ -71,14 +69,14 @@ class CustomTabWindowFeatureTest { whenever(windowRequest.url).thenReturn("https://www.firefox.com") store.dispatch(ContentAction.UpdateWindowRequestAction(sessionId, windowRequest)).joinBlocking() - verify(activity).startActivity(any(), any()) + verify(engineSession).loadUrl("https://www.firefox.com") verify(store).dispatch(ContentAction.ConsumeWindowRequestAction(sessionId)) } @Test fun `given a request to open window, when the url can't be handled, then handleError should be called`() { val exception = ActivityNotFoundException() - val feature = spy(CustomTabWindowFeature(activity, store, sessionId, launchUrlFallback)) + val feature = spy(CustomTabWindowFeature(store, sessionId)) val windowRequest: WindowRequest = mock() feature.start() @@ -86,63 +84,11 @@ class CustomTabWindowFeatureTest { whenever(windowRequest.url).thenReturn("blob:https://www.firefox.com") whenever(activity.startActivity(any(), any())).thenThrow(exception) store.dispatch(ContentAction.UpdateWindowRequestAction(sessionId, windowRequest)).joinBlocking() - - verify(launchUrlFallback).invoke("blob:https://www.firefox.com".toUri()) - } - - @Test - fun `creates intent based on default custom tab config`() { - val feature = CustomTabWindowFeature(activity, store, sessionId, launchUrlFallback) - val config = CustomTabConfig() - val intent = feature.configToIntent(config) - - val newConfig = createCustomTabConfigFromIntent(intent.intent, null) - assertEquals("org.mozilla.firefox", intent.intent.`package`) - assertEquals(config, newConfig) - } - - @Test - fun `creates intent based on custom tab config`() { - val feature = CustomTabWindowFeature(activity, store, sessionId, launchUrlFallback) - val config = CustomTabConfig( - toolbarColor = Color.RED, - navigationBarColor = Color.BLUE, - enableUrlbarHiding = true, - showShareMenuItem = true, - titleVisible = true, - ) - val intent = feature.configToIntent(config) - - val newConfig = createCustomTabConfigFromIntent(intent.intent, null) - assertEquals("org.mozilla.firefox", intent.intent.`package`) - assertEquals(config, newConfig) - } - - @Test - fun `creates intent with same menu items`() { - val feature = CustomTabWindowFeature(activity, store, sessionId, launchUrlFallback) - val config = CustomTabConfig( - actionButtonConfig = CustomTabActionButtonConfig( - description = "button", - icon = mock(), - pendingIntent = mock(), - ), - menuItems = listOf( - CustomTabMenuItem("Item A", mock()), - CustomTabMenuItem("Item B", mock()), - CustomTabMenuItem("Item C", mock()), - ), - ) - val intent = feature.configToIntent(config) - - val newConfig = createCustomTabConfigFromIntent(intent.intent, null) - assertEquals("org.mozilla.firefox", intent.intent.`package`) - assertEquals(config, newConfig) } @Test fun `handles no requests when stopped`() { - val feature = CustomTabWindowFeature(activity, store, sessionId, launchUrlFallback) + val feature = spy(CustomTabWindowFeature(store, sessionId)) feature.start() feature.stop() @@ -151,7 +97,6 @@ class CustomTabWindowFeatureTest { whenever(windowRequest.url).thenReturn("https://www.firefox.com") store.dispatch(ContentAction.UpdateWindowRequestAction(sessionId, windowRequest)).joinBlocking() verify(activity, never()).startActivity(any(), any()) - verifyNoInteractions(launchUrlFallback) verify(store, never()).dispatch(ContentAction.ConsumeWindowRequestAction(sessionId)) } } diff --git a/android-components/components/feature/pwa/src/main/java/mozilla/components/feature/pwa/WebAppShortcutManager.kt b/android-components/components/feature/pwa/src/main/java/mozilla/components/feature/pwa/WebAppShortcutManager.kt index a97b3b8415e7..56ce0d976700 100644 --- a/android-components/components/feature/pwa/src/main/java/mozilla/components/feature/pwa/WebAppShortcutManager.kt +++ b/android-components/components/feature/pwa/src/main/java/mozilla/components/feature/pwa/WebAppShortcutManager.kt @@ -46,7 +46,7 @@ import java.util.UUID private val pwaIconMemoryCache = IconMemoryCache() -const val SHORTCUT_CATEGORY = mozilla.components.feature.customtabs.SHORTCUT_CATEGORY +const val SHORTCUT_CATEGORY = "mozilla.components.pwa.category.SHORTCUT" /** * Helper to manage pinned shortcuts for websites. diff --git a/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt index be147c0393b1..e626a5229e81 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserFragment.kt @@ -80,21 +80,7 @@ class ExternalAppBrowserFragment : BaseBrowserFragment() { ) windowFeature.set( - feature = CustomTabWindowFeature( - activity, - components.core.store, - customTabSessionId, - ) { uri -> - val intent = - Intent.parseUri("${BuildConfig.DEEP_LINK_SCHEME}://open?url=$uri", 0) - if (intent.action == Intent.ACTION_VIEW) { - intent.addCategory(Intent.CATEGORY_BROWSABLE) - intent.component = null - intent.selector = null - intent.flags = Intent.FLAG_ACTIVITY_NEW_TASK - } - activity.startActivity(intent) - }, + feature = CustomTabWindowFeature(components.core.store, customTabSessionId), owner = this, view = view, )