Closes mozilla-mobile#6685 enable previously unsupported add-ons when…

… available
Amejia481 · Apr 16, 2020 · d174773 · d174773
1 parent e796fa7
commit d174773
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 7 deletions.
diff --git a/components/feature/addons/src/main/java/mozilla/components/feature/addons/AddonManager.kt b/components/feature/addons/src/main/java/mozilla/components/feature/addons/AddonManager.kt
@@ -10,6 +10,7 @@ import kotlinx.coroutines.awaitAll
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.CancellableOperation
 import mozilla.components.concept.engine.Engine
+import mozilla.components.concept.engine.webextension.EnableSource
 import mozilla.components.concept.engine.webextension.WebExtension
 import mozilla.components.concept.engine.webextension.isUnsupported
 import mozilla.components.feature.addons.update.AddonUpdater
@@ -177,11 +178,13 @@ class AddonManager(
      * Enables the provided [Addon].
      *
      * @param addon the [Addon] to enable.
+     * @param source [EnableSource] to indicate why the extension is enabled.
      * @param onSuccess (optional) callback invoked with the enabled [Addon].
      * @param onError (optional) callback invoked if there was an error enabling
      */
     fun enableAddon(
         addon: Addon,
+        source: EnableSource = EnableSource.USER,
         onSuccess: ((Addon) -> Unit) = { },
         onError: ((Throwable) -> Unit) = { }
     ) {
@@ -194,6 +197,7 @@ class AddonManager(
         val pendingAction = addPendingAddonAction()
         engine.enableWebExtension(
             extension,
+            source = source,
             onSuccess = { ext ->
                 val enabledAddon = addon.copy(installedState = ext.toInstalledState())
                 completePendingAddonAction(pendingAction)
@@ -210,11 +214,13 @@ class AddonManager(
      * Disables the provided [Addon].
      *
      * @param addon the [Addon] to disable.
+     * @param source [EnableSource] to indicate why the addon is disabled.
      * @param onSuccess (optional) callback invoked with the enabled [Addon].
      * @param onError (optional) callback invoked if there was an error enabling
      */
     fun disableAddon(
         addon: Addon,
+        source: EnableSource = EnableSource.USER,
         onSuccess: ((Addon) -> Unit) = { },
         onError: ((Throwable) -> Unit) = { }
     ) {
@@ -227,6 +233,7 @@ class AddonManager(
         val pendingAction = addPendingAddonAction()
         engine.disableWebExtension(
             extension,
+            source,
             onSuccess = { ext ->
                 val disabledAddon = addon.copy(installedState = ext.toInstalledState())
                 completePendingAddonAction(pendingAction)

diff --git a/...ddons/src/main/java/mozilla/components/feature/addons/migration/SupportedAddonsChecker.kt b/...ddons/src/main/java/mozilla/components/feature/addons/migration/SupportedAddonsChecker.kt
@@ -20,6 +20,9 @@ import androidx.work.PeriodicWorkRequest
 import androidx.work.PeriodicWorkRequestBuilder
 import androidx.work.WorkManager
 import androidx.work.WorkerParameters
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
+import mozilla.components.concept.engine.webextension.EnableSource
 import mozilla.components.feature.addons.Addon
 import mozilla.components.feature.addons.R
 import mozilla.components.feature.addons.migration.SupportedAddonsChecker.Frequency
@@ -81,7 +84,7 @@ class DefaultSupportedAddonsChecker(
     override fun registerForChecks() {
         WorkManager.getInstance(applicationContext).enqueueUniquePeriodicWork(
                 CHECKER_UNIQUE_PERIODIC_WORK_NAME,
-                ExistingPeriodicWorkPolicy.REPLACE,
+                ExistingPeriodicWorkPolicy.KEEP,
                 createPeriodicWorkerRequest()
         )
         logger.info("Register check for new supported add-ons")
@@ -154,6 +157,14 @@ internal class SupportedAddonsWorker(
                 addon.isSupported() && addon.isDisabledAsUnsupported()
             }
 
+            withContext(Dispatchers.Main) {
+                newSupportedAddons.forEach {
+                    addonManager.enableAddon(it, source = EnableSource.APP_SUPPORT, onError = { error ->
+                        GlobalAddonDependencyProvider.onCrash?.invoke(error)
+                    })
+                }
+            }
+
             if (newSupportedAddons.isNotEmpty()) {
                 val extIds = newSupportedAddons.joinToString { it.id }
                 logger.info("New supported add-ons available $extIds")

diff --git a/components/feature/addons/src/test/java/AddonManagerTest.kt b/components/feature/addons/src/test/java/AddonManagerTest.kt
@@ -18,6 +18,7 @@ import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.Engine
 import mozilla.components.concept.engine.EngineSession
+import mozilla.components.concept.engine.webextension.EnableSource
 import mozilla.components.concept.engine.webextension.ActionHandler
 import mozilla.components.concept.engine.webextension.DisabledFlags
 import mozilla.components.concept.engine.webextension.Metadata
@@ -618,8 +619,8 @@ class AddonManagerTest {
 
         // Make sure engine error is forwarded to caller
         val installedAddon = addon.copy(installedState = Addon.InstalledState(addon.id, "1.0", "", true))
-        manager.enableAddon(installedAddon, onError = errorCallback)
-        verify(engine).enableWebExtension(eq(extension), any(), any(), onErrorCaptor.capture())
+        manager.enableAddon(installedAddon, source = EnableSource.APP_SUPPORT, onError = errorCallback)
+        verify(engine).enableWebExtension(eq(extension), eq(EnableSource.APP_SUPPORT), any(), onErrorCaptor.capture())
         onErrorCaptor.value.invoke(IllegalStateException("test"))
         assertNotNull(throwable!!)
         assertEquals("test", throwable!!.localizedMessage)
@@ -648,11 +649,11 @@ class AddonManagerTest {
 
         var disabledAddon: Addon? = null
         val manager = AddonManager(mock(), engine, mock(), mock())
-        manager.disableAddon(addon, onSuccess = {
+        manager.disableAddon(addon, source = EnableSource.APP_SUPPORT, onSuccess = {
             disabledAddon = it
         })
 
-        verify(engine).disableWebExtension(eq(extension), any(), onSuccessCaptor.capture(), any())
+        verify(engine).disableWebExtension(eq(extension), eq(EnableSource.APP_SUPPORT), onSuccessCaptor.capture(), any())
         onSuccessCaptor.value.invoke(extension)
         assertNotNull(disabledAddon)
         assertEquals(addon.id, disabledAddon!!.id)

diff --git a/...ns/src/test/java/mozilla/components/feature/addons/migration/SupportedAddonsWorkerTest.kt b/...ns/src/test/java/mozilla/components/feature/addons/migration/SupportedAddonsWorkerTest.kt
@@ -12,6 +12,7 @@ import androidx.work.await
 import androidx.work.testing.TestListenableWorkerBuilder
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.runBlocking
+import mozilla.components.concept.engine.webextension.EnableSource
 import mozilla.components.feature.addons.Addon
 import mozilla.components.feature.addons.AddonManager
 import mozilla.components.feature.addons.AddonManagerException
@@ -22,18 +23,24 @@ import mozilla.components.feature.addons.ui.translatedName
 import mozilla.components.support.base.ids.SharedIdsHelper
 import mozilla.components.support.ktx.android.content.appName
 import mozilla.components.support.test.mock
+import mozilla.components.support.test.eq
+import mozilla.components.support.test.any
+import mozilla.components.support.test.argumentCaptor
 import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.rule.MainCoroutineRule
 import mozilla.components.support.test.whenever
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
+import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertFalse
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mockito.verify
 import java.io.IOException
+import java.lang.Exception
 
 @RunWith(AndroidJUnit4::class)
 class SupportedAddonsWorkerTest {
@@ -56,15 +63,19 @@ class SupportedAddonsWorkerTest {
     fun `doWork - will return Result_success and create a notification when a new supported add-on is found`() {
         val addonManager = mock<AddonManager>()
         val worker = TestListenableWorkerBuilder<SupportedAddonsWorker>(testContext).build()
-
+        var throwable: Throwable? = null
+        val onCrash = { caught: Throwable ->
+            throwable = caught
+        }
         val unsupportedAddon = mock<Addon> {
             whenever(translatableName).thenReturn(mapOf(Addon.DEFAULT_LOCALE to "one"))
             whenever(isSupported()).thenReturn(true)
             whenever(isDisabledAsUnsupported()).thenReturn(true)
             whenever(defaultLocale).thenReturn(Addon.DEFAULT_LOCALE)
         }
 
-        GlobalAddonDependencyProvider.initialize(addonManager, mock())
+        GlobalAddonDependencyProvider.initialize(addonManager, mock(), onCrash)
+        val onErrorCaptor = argumentCaptor<((Throwable) -> Unit)>()
 
         runBlocking {
             whenever(addonManager.getAddons()).thenReturn(listOf(unsupportedAddon))
@@ -74,6 +85,10 @@ class SupportedAddonsWorkerTest {
 
             val notificationId = SharedIdsHelper.getIdForTag(testContext, NOTIFICATION_TAG)
             assertTrue(isNotificationVisible(notificationId))
+            verify(addonManager).enableAddon(eq(unsupportedAddon), source = eq(EnableSource.APP_SUPPORT), onSuccess = any(), onError = onErrorCaptor.capture())
+
+            onErrorCaptor.value.invoke(Exception())
+            assertNotNull(throwable!!)
         }
     }
 

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -13,8 +13,10 @@ permalink: /changelog/
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt)
 
 * **feature-addons**
+  * Fixed [issue #6685](https://github.com/mozilla-mobile/android-components/issues/6685), now `DefaultSupportedAddonsChecker` will marked any newly supported add-on as enabled.
   * Added `Addon.translatedSummary` and `Addon.translatedDescription` to ease add-on translations.
   * Added `Addon.defaultLocale` Indicates which locale will be always available to display translatable fields.
+  * ⚠️ **This is a breaking change**: `AddonManager.enableAddon` and `AddonManager.disableAddon` have a new optional parameter  `source` that indicates why the extension is enabled/disabled.
   * ⚠️ **This is a breaking change**: `Map<String, String>.translate` now is marked as internal, if you are trying to translate the summary or the description of an add-on, use `Addon.translatedSummary` and `Addon.translatedDescription`.
 
 * **feature-media**