Commit 64880895 authored by Roger Yang's avatar Roger Yang
Browse files

For #4573: Use Browser Fallback URL when Failed to Redirect App Link

parent e5c2f4ce
Loading
Loading
Loading
Loading
+2 −3
Original line number Diff line number Diff line
@@ -12,8 +12,7 @@ import android.content.pm.ResolveInfo
 */
data class AppLinkRedirect(
    val appIntent: Intent?,
    val webUrl: String?,
    val isFallback: Boolean,
    val fallbackUrl: String?,
    val info: ResolveInfo? = null
) {
    /**
@@ -24,7 +23,7 @@ data class AppLinkRedirect(
    /**
     * If there is a fallback URL (should the intent fails).
     */
    fun hasFallback() = webUrl != null && isFallback
    fun hasFallback() = fallbackUrl != null

    /**
     * If the app link is a redirect (to an app or URL).
+9 −10
Original line number Diff line number Diff line
@@ -14,7 +14,6 @@ import androidx.fragment.app.FragmentManager
import mozilla.components.browser.session.SelectionAwareSessionObserver
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.concept.engine.request.RequestInterceptor
import mozilla.components.feature.app.links.RedirectDialogFragment.Companion.FRAGMENT_TAG
import mozilla.components.support.base.feature.LifecycleAwareFeature
import mozilla.components.support.ktx.android.net.hostWithoutCommonPrefixes
@@ -29,8 +28,6 @@ import mozilla.components.support.ktx.android.net.hostWithoutCommonPrefixes
 *
 * It provides use cases to detect and open links openable in third party non-browser apps.
 *
 * It provides a [RequestInterceptor] to do the detection and asking of consent.
 *
 * It requires: a [Context], and a [FragmentManager].
 *
 * A [Boolean] flag is provided at construction to allow the feature and use cases to be landed without
@@ -54,7 +51,7 @@ class AppLinksFeature(
    private val useCases: AppLinksUseCases = AppLinksUseCases(context)
) : LifecycleAwareFeature {

    @VisibleForTesting
    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
    internal val observer: SelectionAwareSessionObserver = object : SelectionAwareSessionObserver(sessionManager) {
        override fun onLoadRequest(
            session: Session,
@@ -84,7 +81,7 @@ class AppLinksFeature(
        }
    }

    @VisibleForTesting
    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
    internal fun handleLoadRequest(session: Session, url: String, triggeredByWebContent: Boolean) {
        if (!triggeredByWebContent) {
            return
@@ -104,7 +101,7 @@ class AppLinksFeature(
    }

    @SuppressLint("MissingPermission")
    @VisibleForTesting
    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
    internal fun handleRedirect(redirect: AppLinkRedirect, session: Session) {
        if (!redirect.hasExternalApp()) {
            handleFallback(redirect, session)
@@ -135,9 +132,11 @@ class AppLinksFeature(
        }
    }

    private fun handleFallback(redirect: AppLinkRedirect, session: Session) {
        redirect.webUrl?.let {
            sessionManager.getOrCreateEngineSession(session).loadUrl(it)
    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
    internal fun handleFallback(redirect: AppLinkRedirect, session: Session) {
        redirect.fallbackUrl?.let {
            val c = sessionManager.getOrCreateEngineSession(session)
            c.loadUrl(it)
        }
    }

@@ -145,7 +144,7 @@ class AppLinksFeature(
        return findPreviousDialogFragment() != null
    }

    @VisibleForTesting
    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
    internal fun reAttachOnConfirmRedirectListener(previousDialog: RedirectDialogFragment?) {
        previousDialog?.apply {
            this@AppLinksFeature.dialog = this
+48 −48
Original line number Diff line number Diff line
@@ -76,39 +76,29 @@ class AppLinksUseCases(
        private val includeInstallAppFallback: Boolean = false
    ) {
        operator fun invoke(url: String): AppLinkRedirect {
            val intents = createBrowsableIntents(url)
            val (appIntent, resolveInfos) = if (includeHttpAppLinks) {
                intents.asSequence()
                    .map { it to getNonBrowserActivities(it) }
                    .firstOrNull { it.second.isNotEmpty() }
                    ?.let {
                        // The user may have decided to keep opening this type of link in this browser.
                        if (ignoreDefaultBrowser &&
                            findDefaultActivity(it.first)?.activityInfo?.packageName == context.packageName) {
                            // in which case, this isn't an app intent anymore.
                            null
                        } else {
                            it
                        }
                    }
            } else {
                intents.asSequence()
                    .filter { it.data?.isHttpOrHttps != true }
                    .map { it to getNonBrowserActivities(it) }
                    .firstOrNull { it.second.isNotEmpty() }
            } ?: null to null
            val redirectData = createBrowsableIntents(url)
            val isAppIntentHttpOrHttps = redirectData.appIntent?.data?.isHttpOrHttps ?: false

            val webUrls = intents.mapNotNull {
                if (it.data?.isHttpOrHttps == true) it.dataString else null
            val appIntent = when {
                redirectData.resolveInfo == null -> null
                includeHttpAppLinks && (ignoreDefaultBrowser ||
                    (redirectData.appIntent != null && isDefaultBrowser(redirectData.appIntent))) -> null
                !includeHttpAppLinks && isAppIntentHttpOrHttps -> null
                else -> redirectData.appIntent
            }

            val webUrl = webUrls.firstOrNull { it != url } ?: webUrls.firstOrNull()

            val appInfo = resolveInfos?.firstOrNull()
            val fallbackUrl = if (redirectData.fallbackIntent?.data?.isHttpOrHttps == true) {
                redirectData.fallbackIntent.dataString
            } else {
                null
            }

            return AppLinkRedirect(appIntent, webUrl, webUrl != url, appInfo)
            return AppLinkRedirect(appIntent, fallbackUrl, redirectData.resolveInfo)
        }

        private fun isDefaultBrowser(intent: Intent) =
            findDefaultActivity(intent)?.activityInfo?.packageName == context.packageName

        private fun getNonBrowserActivities(intent: Intent): List<ResolveInfo> {
            return findActivities(intent)
                .map { it.activityInfo.packageName to it }
@@ -116,20 +106,16 @@ class AppLinksUseCases(
                .map { it.second }
        }

        private fun createBrowsableIntents(url: String): List<Intent> {
        private fun createBrowsableIntents(url: String): RedirectData {
            val intent = Intent.parseUri(url, 0)

            if (intent.action == Intent.ACTION_VIEW) {
                intent.addCategory(Intent.CATEGORY_BROWSABLE)
                intent.component = null
                intent.selector = null
            }

            return when (intent.data?.isHttpOrHttps) {
                null -> emptyList()
                true -> listOf(intent)
                false -> {
                    // Non http[s] schemes:

                    val fallback = intent.getStringExtra(EXTRA_BROWSER_FALLBACK_URL)?.let {
            val fallbackIntent = intent.getStringExtra(EXTRA_BROWSER_FALLBACK_URL)?.let {
                Intent.parseUri(it, 0)
            }

@@ -141,9 +127,16 @@ class AppLinksUseCases(
                }
            }

                    return listOfNotNull(intent, fallback, marketplaceIntent)
            val appIntent = when (intent.data) {
                null -> null
                else -> intent
            }

            val resolveInfo = appIntent?.let {
                getNonBrowserActivities(it).firstOrNull()
            }

            return RedirectData(appIntent, fallbackIntent, marketplaceIntent, resolveInfo)
        }
    }

@@ -175,4 +168,11 @@ class AppLinksUseCases(
            includeInstallAppFallback = false
        )
    }

    private data class RedirectData(
        val appIntent: Intent? = null,
        val fallbackIntent: Intent? = null,
        val marketplaceIntent: Intent? = null,
        val resolveInfo: ResolveInfo? = null
    )
}
+10 −13
Original line number Diff line number Diff line
@@ -18,37 +18,34 @@ class AppLinkRedirectTest {

    @Test
    fun hasExternalApp() {
        var appLink = AppLinkRedirect(appIntent = mock(), webUrl = null, isFallback = true)
        var appLink = AppLinkRedirect(appIntent = mock(), fallbackUrl = null)
        assertTrue(appLink.hasExternalApp())

        appLink = AppLinkRedirect(appIntent = null, webUrl = null, isFallback = true)
        appLink = AppLinkRedirect(appIntent = null, fallbackUrl = null)
        assertFalse(appLink.hasExternalApp())
    }

    @Test
    fun hasFallback() {
        var appLink = AppLinkRedirect(appIntent = mock(), webUrl = null, isFallback = true)
        var appLink = AppLinkRedirect(appIntent = mock(), fallbackUrl = null)
        assertFalse(appLink.hasFallback())

        appLink = AppLinkRedirect(appIntent = mock(), webUrl = "https://example.com", isFallback = false)
        assertFalse(appLink.hasFallback())

        appLink = AppLinkRedirect(appIntent = mock(), webUrl = "https://example.com", isFallback = true)
        appLink = AppLinkRedirect(appIntent = mock(), fallbackUrl = "https://example.com")
        assertTrue(appLink.hasFallback())
    }

    @Test
    fun isRedirect() {
        var appLink = AppLinkRedirect(appIntent = null, webUrl = null, isFallback = true)
        var appLink = AppLinkRedirect(appIntent = null, fallbackUrl = null)
        assertFalse(appLink.isRedirect())

        appLink = AppLinkRedirect(appIntent = mock(), webUrl = null, isFallback = true)
        appLink = AppLinkRedirect(appIntent = mock(), fallbackUrl = null)
        assertTrue(appLink.isRedirect())

        appLink = AppLinkRedirect(appIntent = null, webUrl = "https://example.com", isFallback = true)
        appLink = AppLinkRedirect(appIntent = null, fallbackUrl = "https://example.com")
        assertTrue(appLink.isRedirect())

        appLink = AppLinkRedirect(appIntent = mock(), webUrl = "https://example.com", isFallback = true)
        appLink = AppLinkRedirect(appIntent = mock(), fallbackUrl = "https://example.com")
        assertTrue(appLink.isRedirect())
    }

@@ -59,10 +56,10 @@ class AppLinkRedirectTest {
        `when`(intent.data).thenReturn(uri)
        `when`(uri.scheme).thenReturn("market")

        var appLink = AppLinkRedirect(appIntent = null, webUrl = "https://example.com", isFallback = true)
        var appLink = AppLinkRedirect(appIntent = null, fallbackUrl = "https://example.com")
        assertFalse(appLink.isInstallable())

        appLink = AppLinkRedirect(appIntent = intent, webUrl = "https://example.com", isFallback = true)
        appLink = AppLinkRedirect(appIntent = intent, fallbackUrl = "https://example.com")
        assertTrue(appLink.isInstallable())
    }
}
 No newline at end of file
+36 −12
Original line number Diff line number Diff line
@@ -9,9 +9,11 @@ import android.content.Intent
import android.net.Uri
import androidx.fragment.app.FragmentManager
import androidx.test.ext.junit.runners.AndroidJUnit4
import mozilla.components.browser.session.LegacySessionManager
import mozilla.components.browser.session.Session
import mozilla.components.browser.session.SessionManager
import mozilla.components.concept.engine.Engine
import mozilla.components.concept.engine.EngineSession
import mozilla.components.support.test.any
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
@@ -36,6 +38,8 @@ class AppLinksFeatureTest {
    private lateinit var mockUseCases: AppLinksUseCases
    private lateinit var mockGetRedirect: AppLinksUseCases.GetAppLinkRedirect
    private lateinit var mockOpenRedirect: AppLinksUseCases.OpenAppLinkRedirect
    private lateinit var mockEngineSession: EngineSession
    private lateinit var mockLegacySessionManager: LegacySessionManager

    private lateinit var feature: AppLinksFeature

@@ -48,18 +52,20 @@ class AppLinksFeatureTest {
        mockContext = mock()

        val engine = mock<Engine>()
        mockSessionManager = spy(SessionManager(engine))
        mockLegacySessionManager = mock()
        mockSessionManager = spy(SessionManager(engine, delegate = mockLegacySessionManager))
        mockFragmentManager = mock()
        mockUseCases = mock()
        mockEngineSession = mock()

        mockGetRedirect = mock()
        mockOpenRedirect = mock()
        `when`(mockUseCases.interceptedAppLinkRedirect).thenReturn(mockGetRedirect)
        `when`(mockUseCases.openAppLink).thenReturn(mockOpenRedirect)

        val webRedirect = AppLinkRedirect(null, webUrl, false)
        val appRedirect = AppLinkRedirect(Intent.parseUri(intentUrl, 0), null, false)
        val appRedirectFromWebUrl = AppLinkRedirect(Intent.parseUri(webUrlWithAppLink, 0), null, false)
        val webRedirect = AppLinkRedirect(null, webUrl)
        val appRedirect = AppLinkRedirect(Intent.parseUri(intentUrl, 0), null)
        val appRedirectFromWebUrl = AppLinkRedirect(Intent.parseUri(webUrlWithAppLink, 0), null)

        `when`(mockGetRedirect.invoke(webUrl)).thenReturn(webRedirect)
        `when`(mockGetRedirect.invoke(intentUrl)).thenReturn(appRedirect)
@@ -116,9 +122,9 @@ class AppLinksFeatureTest {
            useCases = mockUseCases
        )

        subject.handleLoadRequest(session, webUrl, true)
        subject.handleLoadRequest(session, webUrlWithAppLink, true)

        verify(mockGetRedirect).invoke(webUrl)
        verify(mockGetRedirect).invoke(webUrlWithAppLink)
        verifyNoMoreInteractions(mockOpenRedirect)
    }

@@ -135,7 +141,7 @@ class AppLinksFeatureTest {
        )

        val url = "$whitelistedScheme://example.com"
        val whitelistedRedirect = AppLinkRedirect(Intent.parseUri(url, 0), url, false)
        val whitelistedRedirect = AppLinkRedirect(Intent.parseUri(url, 0), url)
        `when`(mockGetRedirect.invoke(url)).thenReturn(whitelistedRedirect)

        subject.handleLoadRequest(session, url, true)
@@ -153,7 +159,7 @@ class AppLinksFeatureTest {
            useCases = mockUseCases
        )
        val mockSession = createSession(false)
        `when`(mockSessionManager.findSessionById(ArgumentMatchers.anyString())).thenReturn(mockSession)
        `when`(mockLegacySessionManager.findSessionById(ArgumentMatchers.anyString())).thenReturn(mockSession)

        feature.start()

@@ -205,14 +211,15 @@ class AppLinksFeatureTest {
        val mockDialog = spy(RedirectDialogFragment::class.java)

        val featureWithDialog =
            AppLinksFeature(
            spy(AppLinksFeature(
                context = mockContext,
                sessionManager = mockSessionManager,
                useCases = mockUseCases,
                fragmentManager = mockFragmentManager,
                dialog = mockDialog
            )
            ))

        `when`(mockLegacySessionManager.getOrCreateEngineSession(any())).thenReturn(mockEngineSession)
        featureWithDialog.start()

        userTapsOnSession(webUrl, true)
@@ -322,11 +329,28 @@ class AppLinksFeatureTest {

        val redirect = AppLinkRedirect(
            intent,
            javascriptUri,
            false)
            javascriptUri)

        feature.handleRedirect(redirect, Session("https://www.amazon.ca"))

        verify(openAppUseCase, never()).invoke(redirect)
    }

    @Test
    fun `Use the fallback URL when app is not installed`() {
        val feature = spy(AppLinksFeature(
                testContext,
                sessionManager = mockSessionManager,
                interceptLinkClicks = true,
                useCases = mockUseCases
        ))

        val redirect = AppLinkRedirect(null, webUrl)
        val session = Session(webUrl)

        `when`(mockLegacySessionManager.getOrCreateEngineSession(any())).thenReturn(mockEngineSession)
        feature.handleRedirect(redirect, session)

        verify(feature).handleFallback(redirect, session)
    }
}
Loading