From c549b71c2a0f14bdc3b8d66d0e4d2c6704676010 Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Thu, 5 Nov 2020 16:25:45 -0500 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/16032: Support installing recommended add-ons from AMO --- .../mozilla/fenix/AppRequestInterceptor.kt | 10 +- .../mozilla/fenix/AppRequestInterceptor.kt | 56 ++++++++++- .../java/org/mozilla/fenix/HomeActivity.kt | 2 + .../fenix/addons/AddonsManagementFragment.kt | 54 ++++++++++- .../org/mozilla/fenix/addons/Extensions.kt | 4 +- .../java/org/mozilla/fenix/components/Core.kt | 11 ++- app/src/main/res/navigation/nav_graph.xml | 13 ++- app/src/main/res/values/strings.xml | 6 ++ .../fenix/AppRequestInterceptorTest.kt | 95 ++++++++++++++++++- .../addons/AddonsManagementFragmentTest.kt | 72 ++++++++++++++ 10 files changed, 314 insertions(+), 9 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/addons/AddonsManagementFragmentTest.kt diff --git a/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt index c16ee4d63d..59e2de9a5c 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt @@ -5,18 +5,26 @@ package org.mozilla.fenix import android.content.Context +import androidx.navigation.NavController import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.request.RequestInterceptor import org.mozilla.fenix.ext.components import org.mozilla.fenix.ui.robots.appContext +import java.lang.ref.WeakReference /** * This class overrides the application's request interceptor to * deactivate the FxA web channel * which is not supported on the staging servers. */ - class AppRequestInterceptor(private val context: Context) : RequestInterceptor { + + private var navController: WeakReference? = null + + fun setNavigationController(navController: NavController) { + this.navController = WeakReference(navController) + } + override fun onLoadRequest( engineSession: EngineSession, uri: String, diff --git a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt index 9153208903..e8ef423c84 100644 --- a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt +++ b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix import android.content.Context import android.net.ConnectivityManager import androidx.core.content.getSystemService +import androidx.navigation.NavController import mozilla.components.browser.errorpages.ErrorPages import mozilla.components.browser.errorpages.ErrorType import mozilla.components.concept.engine.EngineSession @@ -14,8 +15,18 @@ import mozilla.components.concept.engine.request.RequestInterceptor import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.isOnline +import java.lang.ref.WeakReference + +class AppRequestInterceptor( + private val context: Context +) : RequestInterceptor { + + private var navController: WeakReference? = null + + fun setNavigationController(navController: NavController) { + this.navController = WeakReference(navController) + } -class AppRequestInterceptor(private val context: Context) : RequestInterceptor { override fun onLoadRequest( engineSession: EngineSession, uri: String, @@ -26,6 +37,11 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor { isDirectNavigation: Boolean, isSubframeRequest: Boolean ): RequestInterceptor.InterceptionResponse? { + + interceptAmoRequest(uri, isSameDomain, hasUserGesture)?.let { response -> + return response + } + return context.components.services.appLinksInterceptor .onLoadRequest( engineSession, @@ -59,6 +75,42 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor { return RequestInterceptor.ErrorResponse.Uri(errorPageUri) } + /** + * Checks if the provided [uri] is a request to install an add-on from addons.mozilla.org and + * redirects to Add-ons Manager to trigger installation if needed. + * + * @return [RequestInterceptor.InterceptionResponse.Deny] when installation was triggered and + * the original request can be skipped, otherwise null to continue loading the page. + */ + private fun interceptAmoRequest( + uri: String, + isSameDomain: Boolean, + hasUserGesture: Boolean + ): RequestInterceptor.InterceptionResponse? { + // First we execute a quick check to see if this is a request we're interested in i.e. a + // request triggered by the user and coming from AMO. + if (hasUserGesture && isSameDomain && uri.startsWith(AMO_BASE_URL)) { + + // Check if this is a request to install an add-on. + val matchResult = AMO_INSTALL_URL_REGEX.toRegex().matchEntire(uri) + if (matchResult != null) { + + // Navigate and trigger add-on installation. + matchResult.groupValues.getOrNull(1)?.let { addonId -> + navController?.get()?.navigate( + NavGraphDirections.actionGlobalAddonsManagementFragment(addonId) + ) + + // We've redirected to the add-ons management fragment, skip original request. + return RequestInterceptor.InterceptionResponse.Deny + } + } + } + + // In all other case we let the original request proceed. + return null + } + /** * Where possible, this will make the error type more accurate by including information not * available to AC. @@ -116,5 +168,7 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor { companion object { internal const val LOW_AND_MEDIUM_RISK_ERROR_PAGES = "low_and_medium_risk_error_pages.html" internal const val HIGH_RISK_ERROR_PAGES = "high_risk_error_pages.html" + internal const val AMO_BASE_URL = "https://addons.mozilla.org" + internal const val AMO_INSTALL_URL_REGEX = "$AMO_BASE_URL/android/downloads/file/([^\\s]+)/([^\\s]+\\.xpi)" } } diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index a8bfd89d95..a5c24db0df 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -238,6 +238,8 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { startupTelemetryOnCreateCalled(intent.toSafeIntent(), savedInstanceState != null) + components.core.requestInterceptor.setNavigationController(navHost.navController) + StartupTimeline.onActivityCreateEndHome(this) // DO NOT MOVE ANYTHING BELOW HERE. } diff --git a/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt index 670df4130f..d486fac13f 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt @@ -9,11 +9,13 @@ import android.os.Bundle import android.view.Gravity import android.view.View import android.view.accessibility.AccessibilityEvent +import androidx.annotation.VisibleForTesting import androidx.core.content.res.ResourcesCompat import androidx.core.view.isVisible import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController +import androidx.navigation.fragment.navArgs import androidx.recyclerview.widget.LinearLayoutManager import kotlinx.android.synthetic.main.fragment_add_ons_management.* import kotlinx.android.synthetic.main.fragment_add_ons_management.view.* @@ -28,6 +30,7 @@ import mozilla.components.feature.addons.ui.AddonsManagerAdapter import mozilla.components.feature.addons.ui.PermissionsDialogFragment import mozilla.components.feature.addons.ui.translateName import org.mozilla.fenix.R +import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView @@ -44,10 +47,21 @@ import java.util.concurrent.CancellationException @Suppress("TooManyFunctions", "LargeClass") class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) { + private val args by navArgs() + /** * Whether or not an add-on installation is in progress. */ private var isInstallationInProgress = false + + private var installExternalAddonComplete: Boolean + set(value) { + arguments?.putBoolean(BUNDLE_KEY_INSTALL_EXTERNAL_ADDON_COMPLETE, value) + } + get() { + return arguments?.getBoolean(BUNDLE_KEY_INSTALL_EXTERNAL_ADDON_COMPLETE, false) ?: false + } + private var adapter: AddonsManagerAdapter? = null override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -82,9 +96,13 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) val recyclerView = view.add_ons_list recyclerView.layoutManager = LinearLayoutManager(requireContext()) val shouldRefresh = adapter != null + + // If the fragment was launched to install an "external" add-on from AMO, we deactivate + // the cache to get the most up-to-date list of add-ons to match against. + val allowCache = args.installAddonId == null || installExternalAddonComplete lifecycleScope.launch(IO) { try { - val addons = requireContext().components.addonManager.getAddons() + val addons = requireContext().components.addonManager.getAddons(allowCache = allowCache) lifecycleScope.launch(Dispatchers.Main) { runIfFragmentIsAttached { if (!shouldRefresh) { @@ -103,6 +121,12 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) if (shouldRefresh) { adapter?.updateAddons(addons) } + + args.installAddonId?.let { addonIn -> + if (!installExternalAddonComplete) { + installExternalAddon(addons, addonIn) + } + } } } } catch (e: AddonManagerException) { @@ -121,6 +145,30 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) } } + @VisibleForTesting + internal fun installExternalAddon(supportedAddons: List, installAddonId: String) { + val addonToInstall = supportedAddons.find { it.downloadId == installAddonId } + if (addonToInstall == null) { + showErrorSnackBar(getString(R.string.addon_not_supported_error)) + } else { + if (addonToInstall.isInstalled()) { + showErrorSnackBar(getString(R.string.addon_already_installed)) + } else { + showPermissionDialog(addonToInstall) + } + } + installExternalAddonComplete = true + } + + @VisibleForTesting + internal fun showErrorSnackBar(text: String) { + runIfFragmentIsAttached { + view?.let { + showSnackBar(it, text, FenixSnackbar.LENGTH_LONG) + } + } + } + private fun createAddonStyle(context: Context): AddonsManagerAdapter.Style { return AddonsManagerAdapter.Style( sectionsTextColor = ThemeManager.resolveAttribute(R.attr.primaryText, context), @@ -144,7 +192,8 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) as? AddonInstallationDialogFragment != null } - private fun showPermissionDialog(addon: Addon) { + @VisibleForTesting + internal fun showPermissionDialog(addon: Addon) { if (!isInstallationInProgress && !hasExistingPermissionDialogFragment()) { val dialog = PermissionsDialogFragment.newInstance( addon = addon, @@ -278,5 +327,6 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) companion object { private const val PERMISSIONS_DIALOG_FRAGMENT_TAG = "ADDONS_PERMISSIONS_DIALOG_FRAGMENT" private const val INSTALLATION_DIALOG_FRAGMENT_TAG = "ADDONS_INSTALLATION_DIALOG_FRAGMENT" + private const val BUNDLE_KEY_INSTALL_EXTERNAL_ADDON_COMPLETE = "INSTALL_EXTERNAL_ADDON_COMPLETE" } } diff --git a/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt b/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt index 6c8a991f06..c0b135a60d 100644 --- a/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt +++ b/app/src/main/java/org/mozilla/fenix/addons/Extensions.kt @@ -14,10 +14,10 @@ import org.mozilla.fenix.components.FenixSnackbar * @param view A [View] used to determine a parent for the [FenixSnackbar]. * @param text The text to display in the [FenixSnackbar]. */ -internal fun showSnackBar(view: View, text: String) { +internal fun showSnackBar(view: View, text: String, duration: Int = FenixSnackbar.LENGTH_SHORT) { FenixSnackbar.make( view = view, - duration = FenixSnackbar.LENGTH_SHORT, + duration = duration, isDisplayedWithBrowserToolbar = true ) .setText(text) diff --git a/app/src/main/java/org/mozilla/fenix/components/Core.kt b/app/src/main/java/org/mozilla/fenix/components/Core.kt index 644319e3a1..a6011c217b 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -100,7 +100,7 @@ class Core( */ val engine: Engine by lazyMonitored { val defaultSettings = DefaultSettings( - requestInterceptor = AppRequestInterceptor(context), + requestInterceptor = requestInterceptor, remoteDebuggingEnabled = context.settings().isRemoteDebuggingEnabled && Build.VERSION.SDK_INT >= Build.VERSION_CODES.M, testingModeEnabled = false, @@ -141,6 +141,15 @@ class Core( } } + /** + * Passed to [engine] to intercept requests for app links, + * and various features triggered by page load requests. + * + * NB: This does not need to be lazy as it is initialized + * with the engine on startup. + */ + val requestInterceptor = AppRequestInterceptor(context) + /** * [Client] implementation to be used for code depending on `concept-fetch`` */ diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index 2fca420f10..1ca254248c 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -85,7 +85,13 @@ app:destination="@id/bookmarkEditFragment" /> + app:destination="@id/addons_management_graph"> + + @@ -866,6 +872,11 @@ + Add-on collection modified. Quitting the application to apply changes… + + + Add-on is not supported + + Add-on is already installed + Sync now diff --git a/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt b/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt index f843555faf..b24cbee298 100644 --- a/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt @@ -6,14 +6,17 @@ package org.mozilla.fenix import android.net.ConnectivityManager import androidx.core.content.getSystemService +import androidx.navigation.NavController import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic +import io.mockk.verify import mozilla.components.browser.errorpages.ErrorPages import mozilla.components.browser.errorpages.ErrorType import mozilla.components.concept.engine.request.RequestInterceptor import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -26,6 +29,7 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner class AppRequestInterceptorTest { private lateinit var interceptor: RequestInterceptor + private lateinit var navigationController: NavController @Before fun setUp() { @@ -34,7 +38,96 @@ class AppRequestInterceptorTest { every { testContext.getSystemService()!!.isOnline() } returns true - interceptor = AppRequestInterceptor(testContext) + navigationController = mockk(relaxed = true) + interceptor = AppRequestInterceptor(testContext).also { + it.setNavigationController(navigationController) + } + } + + @Test + fun `GIVEN request to install add-on WHEN on same domain and triggered by user THEN start add-on installation`() { + val addonId = "12345678" + val result = interceptor.onLoadRequest( + engineSession = mockk(), + uri = "https://addons.mozilla.org/android/downloads/file/$addonId/test.xpi", + lastUri = "https://addons.mozilla.org/en-US/firefox/", + hasUserGesture = true, + isSameDomain = true, + isDirectNavigation = false, + isRedirect = false, + isSubframeRequest = false + ) + + verify { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment(addonId)) } + assertEquals(RequestInterceptor.InterceptionResponse.Deny, result) + } + + @Test + fun `GIVEN request to install add-on WHEN on a different domain THEN no add-on installation is started`() { + val result = interceptor.onLoadRequest( + engineSession = mockk(), + uri = "https://addons.mozilla.org/android/downloads/file/12345678/test.xpi", + lastUri = "https://getpocket.com", + hasUserGesture = true, + isSameDomain = false, + isDirectNavigation = false, + isRedirect = false, + isSubframeRequest = false + ) + + verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) } + assertNull(result) + } + + @Test + fun `GIVEN invalid request to install add-on WHEN on same domain and triggered by user THEN no add-on installation is started`() { + val result = interceptor.onLoadRequest( + engineSession = mockk(), + uri = "https://addons.mozilla.org/android/downloads/file/12345678/test.invalid", + lastUri = "https://addons.mozilla.org/en-US/firefox/", + hasUserGesture = true, + isSameDomain = true, + isDirectNavigation = false, + isRedirect = false, + isSubframeRequest = false + ) + + verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) } + assertNull(result) + } + + @Test + fun `GIVEN request to install add-on WHEN not triggered by user THEN no add-on installation is started`() { + val result = interceptor.onLoadRequest( + engineSession = mockk(), + uri = "https://addons.mozilla.org/android/downloads/file/12345678/test.xpi", + lastUri = "https://addons.mozilla.org/en-US/firefox/", + hasUserGesture = false, + isSameDomain = true, + isDirectNavigation = false, + isRedirect = false, + isSubframeRequest = false + ) + + verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) } + assertNull(result) + } + + @Test + fun `GIVEN any request WHEN on same domain and triggered by user THEN no add-on installation is started`() { + val result = interceptor.onLoadRequest( + engineSession = mockk(), + uri = "https://blog.mozilla.org/blog/2020/10/20/mozilla-reaction-to-u-s-v-google/", + lastUri = "https://blog.mozilla.org", + hasUserGesture = true, + isSameDomain = true, + isDirectNavigation = false, + isRedirect = false, + isSubframeRequest = false + ) + + verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) } + assertNull(result) } @Test diff --git a/app/src/test/java/org/mozilla/fenix/addons/AddonsManagementFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/addons/AddonsManagementFragmentTest.kt new file mode 100644 index 0000000000..3b327fcb70 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/addons/AddonsManagementFragmentTest.kt @@ -0,0 +1,72 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.addons + +import android.content.Context +import androidx.coordinatorlayout.widget.CoordinatorLayout +import io.mockk.every +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import mozilla.components.feature.addons.Addon +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.R +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class AddonsManagementFragmentTest { + + private lateinit var context: Context + private lateinit var view: CoordinatorLayout + private lateinit var fragment: AddonsManagementFragment + + private val addonNotSupportedErrorMessage = "not supported" + private val addonAlreadyInstalledErrorMessage = "already installed" + + @Before + fun setup() { + context = mockk(relaxed = true) + view = mockk(relaxed = true) + fragment = spyk(AddonsManagementFragment()) + every { fragment.context } returns context + every { fragment.view } returns view + every { fragment.showErrorSnackBar(any()) } returns Unit + every { fragment.showPermissionDialog(any()) } returns Unit + every { fragment.getString(R.string.addon_not_supported_error) } returns addonNotSupportedErrorMessage + every { fragment.getString(R.string.addon_already_installed) } returns addonAlreadyInstalledErrorMessage + } + + @Test + fun `GIVEN add-on is installed from external source WHEN add-on is not supported THEN error is shown`() { + val supportedAddons = listOf( + Addon("1", downloadId = "d1"), Addon("2", downloadId = "d2") + ) + val installAddonId = "d3" + fragment.installExternalAddon(supportedAddons, installAddonId) + verify { fragment.showErrorSnackBar(addonNotSupportedErrorMessage) } + } + + @Test + fun `GIVEN add-on is installed from external source WHEN add-on is already installed THEN error is shown`() { + val addon1 = Addon("1", downloadId = "d1", installedState = mockk()) + val addon2 = Addon("2", downloadId = "d2") + val supportedAddons = listOf(addon1, addon2) + + fragment.installExternalAddon(supportedAddons, "d1") + verify { fragment.showErrorSnackBar(addonAlreadyInstalledErrorMessage) } + } + + @Test + fun `GIVEN add-on is installed from external source WHEN supported and not installed THEN start installation`() { + val addon1 = Addon("1", downloadId = "d1", installedState = mockk()) + val addon2 = Addon("2", downloadId = "d2") + val supportedAddons = listOf(addon1, addon2) + + fragment.installExternalAddon(supportedAddons, "d2") + verify { fragment.showPermissionDialog(addon2) } + } +}