Closes #6685 enable previously unsupported add-ons when available

mozilla-mobile · Apr 16, 2020 · a5cdf61 · a5cdf61
1 parent e796fa7
commit a5cdf61
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 5 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,12 @@ internal class SupportedAddonsWorker(
                 addon.isSupported() && addon.isDisabledAsUnsupported()
             }
 
+            withContext(Dispatchers.Main) {
+                newSupportedAddons.map {
+                    addonManager.enableAddon(it, source = EnableSource.APP_SUPPORT)
+                }
+            }
+
             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
@@ -33,6 +34,7 @@ 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
 
 @RunWith(AndroidJUnit4::class)
@@ -74,6 +76,7 @@ class SupportedAddonsWorkerTest {
 
             val notificationId = SharedIdsHelper.getIdForTag(testContext, NOTIFICATION_TAG)
             assertTrue(isNotificationVisible(notificationId))
+            verify(addonManager).enableAddon(unsupportedAddon, source = EnableSource.APP_SUPPORT)
         }
     }
 

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**