For issue mozilla-mobile#16557 set autoplay block audio only

by default

app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuSitePermissionsCommonRobot.kt

@@ -129,12 +129,19 @@ private fun assertVideoAndAudioBlockedRecommended() = onView(withId(R.id.fourth_
 
 private fun assertCheckAutoPayRadioButtonDefault() {
 
+    // Allow audio and video
     onView(withId(R.id.block_radio))
         .assertIsChecked(isChecked = false)
 
+    // Block audio and video on cellular data only
+    onView(withId(R.id.block_radio))
+        .assertIsChecked(isChecked = false)
+
+    // Block audio only
     onView(withId(R.id.third_radio))
         .assertIsChecked(isChecked = false)
 
+    // Block audio and video
     onView(withId(R.id.fourth_radio))
         .assertIsChecked(isChecked = true)
 }

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -847,12 +847,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit
         view: View
     ): List<ContextMenuCandidate>
 
-    @CallSuper
-    override fun onStart() {
-        super.onStart()
-        sitePermissionWifiIntegration.get()?.maybeAddWifiConnectedListener()
-    }
-
     @VisibleForTesting
     internal fun observeRestoreComplete(store: BrowserStore, navController: NavController) {
         val activity = activity as HomeActivity

app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt

@@ -580,7 +580,7 @@ sealed class Event {
 
     data class AutoPlaySettingChanged(val setting: AutoplaySetting) : Event() {
         enum class AutoplaySetting {
-            BLOCK_CELLULAR, BLOCK_AUDIO, BLOCK_ALL
+            BLOCK_CELLULAR, BLOCK_AUDIO, BLOCK_ALL, ALLOW_ALL
         }
 
         override val extras: Map<Autoplay.settingChangedKeys, String>?

app/src/main/java/org/mozilla/fenix/settings/PhoneFeature.kt

@@ -48,7 +48,7 @@ enum class PhoneFeature(val androidPermissionsList: Array<String>) : Parcelable
         @StringRes val stringRes = if (isAndroidPermissionGranted(context)) {
             when (this) {
                 AUTOPLAY_AUDIBLE ->
-                    when (settings?.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) ?: AUTOPLAY_BLOCK_ALL) {
+                    when (settings?.getAutoplayUserSetting() ?: AUTOPLAY_BLOCK_ALL) {
                         AUTOPLAY_ALLOW_ALL -> R.string.preference_option_autoplay_allowed2
                         AUTOPLAY_ALLOW_ON_WIFI -> R.string.preference_option_autoplay_allowed_wifi_only2
                         AUTOPLAY_BLOCK_AUDIBLE -> R.string.preference_option_autoplay_block_audio2

app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsFragment.kt

@@ -59,17 +59,8 @@ class SitePermissionsFragment : PreferenceFragmentCompat() {
         val context = requireContext()
         val settings = context.settings()
 
-        val summary = phoneFeature.getActionLabel(context, settings = settings)
-        // Remove autoplaySummary after https://bugzilla.mozilla.org/show_bug.cgi?id=1621825 is fixed
-        val autoplaySummary =
-            if (summary == context.getString(R.string.preference_option_autoplay_allowed2)) {
-                context.getString(R.string.preference_option_autoplay_allowed_wifi_only2)
-            } else {
-                null
-            }
-
         val cameraPhoneFeatures = requirePreference<Preference>(phoneFeature.getPreferenceId())
-        cameraPhoneFeatures.summary = autoplaySummary ?: summary
+        cameraPhoneFeatures.summary = phoneFeature.getActionLabel(context, settings = settings)
 
         cameraPhoneFeatures.onPreferenceClickListener = OnPreferenceClickListener {
             navigateToPhoneFeature(phoneFeature)

app/src/main/java/org/mozilla/fenix/settings/sitepermissions/SitePermissionsManagePhoneFeatureFragment.kt

@@ -79,14 +79,12 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() {
     private fun initFirstRadio(rootView: View) {
         with(rootView.ask_to_allow_radio) {
             if (args.phoneFeature == AUTOPLAY_AUDIBLE) {
-                // Disabled because GV does not allow this setting. TODO Reenable after
-                // https://bugzilla.mozilla.org/show_bug.cgi?id=1621825 is fixed
-//                text = getString(R.string.preference_option_autoplay_allowed2)
-//                setOnClickListener {
-//                    saveActionInSettings(it.context, AUTOPLAY_ALLOW_ALL)
-//                }
-//                restoreState(AUTOPLAY_ALLOW_ALL)
-                visibility = View.GONE
+                text = getString(R.string.preference_option_autoplay_allowed2)
+                setOnClickListener {
+                    saveActionInSettings(AUTOPLAY_ALLOW_ALL)
+                }
+                restoreState(AUTOPLAY_ALLOW_ALL)
+                visibility = View.VISIBLE
             } else {
                 text = getCombinedLabel(
                     getString(R.string.preference_option_phone_feature_ask_to_allow),
@@ -109,10 +107,7 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() {
                     getString(R.string.preference_option_autoplay_allowed_wifi_subtext)
                 )
                 setOnClickListener {
-                    // TODO replace with AUTOPLAY_ALLOW_ON_WIFI when
-                    // https://bugzilla.mozilla.org/show_bug.cgi?id=1621825 is fixed. This GV bug
-                    // makes ALLOW_ALL behave as ALLOW_ON_WIFI
-                    saveActionInSettings(AUTOPLAY_ALLOW_ALL)
+                    saveActionInSettings(AUTOPLAY_ALLOW_ON_WIFI)
                 }
                 restoreState(AUTOPLAY_ALLOW_ON_WIFI)
             } else {
@@ -129,7 +124,10 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() {
         with(rootView.third_radio) {
             if (args.phoneFeature == AUTOPLAY_AUDIBLE) {
                 visibility = View.VISIBLE
-                text = getString(R.string.preference_option_autoplay_block_audio2)
+                text = getCombinedLabel(
+                    getString(R.string.preference_option_autoplay_block_audio2),
+                    getString(R.string.phone_feature_recommended)
+                )
                 setOnClickListener {
                     saveActionInSettings(AUTOPLAY_BLOCK_AUDIBLE)
                 }
@@ -151,10 +149,8 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() {
         with(rootView.fourth_radio) {
             if (args.phoneFeature == AUTOPLAY_AUDIBLE) {
                 visibility = View.VISIBLE
-                text = getCombinedLabel(
-                    getString(R.string.preference_option_autoplay_blocked3),
-                    getString(R.string.phone_feature_recommended)
-                )
+                text = getString(R.string.preference_option_autoplay_blocked3)
+
                 setOnClickListener {
                     saveActionInSettings(AUTOPLAY_BLOCK_ALL)
                 }
@@ -173,7 +169,7 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() {
     }
 
     private fun RadioButton.restoreState(buttonAutoplaySetting: Int) {
-        if (settings.getAutoplayUserSetting(AUTOPLAY_BLOCK_ALL) == buttonAutoplaySetting) {
+        if (settings.getAutoplayUserSetting() == buttonAutoplaySetting) {
             this.isChecked = true
             this.setStartCheckedIndicator()
         }
@@ -194,9 +190,11 @@ class SitePermissionsManagePhoneFeatureFragment : Fragment() {
         val setting: Event.AutoPlaySettingChanged.AutoplaySetting
 
         val (audible, inaudible) = when (autoplaySetting) {
-            AUTOPLAY_ALLOW_ALL,
+            AUTOPLAY_ALLOW_ALL -> {
+                setting = Event.AutoPlaySettingChanged.AutoplaySetting.ALLOW_ALL
+                ALLOWED to ALLOWED
+            }
             AUTOPLAY_ALLOW_ON_WIFI -> {
-                settings.setAutoplayUserSetting(AUTOPLAY_ALLOW_ON_WIFI)
                 setting = Event.AutoPlaySettingChanged.AutoplaySetting.BLOCK_CELLULAR
                 BLOCKED to BLOCKED
             }

app/src/main/java/org/mozilla/fenix/utils/Settings.kt

@@ -40,6 +40,7 @@ import org.mozilla.fenix.settings.deletebrowsingdata.DeleteBrowsingDataOnQuitTyp
 import org.mozilla.fenix.settings.logins.SavedLoginsSortingStrategyMenu
 import org.mozilla.fenix.settings.logins.SortingStrategy
 import org.mozilla.fenix.settings.registerOnSharedPreferenceChangeListener
+import org.mozilla.fenix.settings.sitepermissions.AUTOPLAY_BLOCK_ALL
 import java.security.InvalidParameterException
 
 private const val AUTOPLAY_USER_SETTING = "AUTOPLAY_USER_SETTING"
@@ -744,9 +745,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {
      * either [AUTOPLAY_ALLOW_ALL] or [AUTOPLAY_BLOCK_ALL]. Because of this, we are forced to save
      * the user selected setting as well.
      */
-    fun getAutoplayUserSetting(
-        default: Int
-    ) = preferences.getInt(AUTOPLAY_USER_SETTING, default)
+    fun getAutoplayUserSetting() = preferences.getInt(AUTOPLAY_USER_SETTING, AUTOPLAY_BLOCK_ALL)
 
     private fun getSitePermissionsPhoneFeatureAutoplayAction(
         feature: PhoneFeature,

app/src/main/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegration.kt

@@ -4,11 +4,12 @@
 
 package org.mozilla.fenix.wifi
 
-import mozilla.components.feature.sitepermissions.SitePermissionsRules
+import androidx.annotation.VisibleForTesting
+import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.ALLOWED
+import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.BLOCKED
 import mozilla.components.support.base.feature.LifecycleAwareFeature
 import org.mozilla.fenix.settings.PhoneFeature
 import org.mozilla.fenix.settings.sitepermissions.AUTOPLAY_ALLOW_ON_WIFI
-import org.mozilla.fenix.settings.sitepermissions.AUTOPLAY_BLOCK_ALL
 import org.mozilla.fenix.utils.Settings
 
 /**
@@ -24,11 +25,11 @@ class SitePermissionsWifiIntegration(
      * Adds listener for autoplay setting [AUTOPLAY_ALLOW_ON_WIFI]. Sets all autoplay to allowed when
      * WIFI is connected, blocked otherwise.
      */
-    private val wifiConnectedListener: ((Boolean) -> Unit) by lazy {
+    @VisibleForTesting
+    internal val wifiConnectedListener: ((Boolean) -> Unit) by lazy {
         { connected: Boolean ->
-            val setting =
-                if (connected) SitePermissionsRules.Action.ALLOWED else SitePermissionsRules.Action.BLOCKED
-            if (settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) == AUTOPLAY_ALLOW_ON_WIFI) {
+            if (settings.getAutoplayUserSetting() == AUTOPLAY_ALLOW_ON_WIFI) {
+                val setting = if (connected) ALLOWED else BLOCKED
                 settings.setSitePermissionsPhoneFeatureAction(
                     PhoneFeature.AUTOPLAY_AUDIBLE,
                     setting
@@ -39,21 +40,11 @@ class SitePermissionsWifiIntegration(
                 )
             } else {
                 // The autoplay setting has changed, we can remove the listener
-                removeWifiConnectedListener()
+                stop()
             }
         }
     }
 
-    /**
-     * If autoplay is only enabled on WIFI, sets a WIFI listener to set them accordingly. Otherwise
-     * noop.
-     */
-    fun maybeAddWifiConnectedListener() {
-        if (settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) == AUTOPLAY_ALLOW_ON_WIFI) {
-            addWifiConnectedListener()
-        }
-    }
-
     fun addWifiConnectedListener() {
         wifiConnectionMonitor.addOnWifiConnectedChangedListener(wifiConnectedListener)
     }
@@ -62,15 +53,15 @@ class SitePermissionsWifiIntegration(
         wifiConnectionMonitor.removeOnWifiConnectedChangedListener(wifiConnectedListener)
     }
 
-    // Until https://bugzilla.mozilla.org/show_bug.cgi?id=1621825 is fixed, AUTOPLAY_ALLOW_ALL
-    // only works while WIFI is active, so we are not using AUTOPLAY_ALLOW_ON_WIFI (or this class).
-    // Once that is fixed, [start] and [maybeAddWifiConnectedListener] will need to be called on
-    // activity startup.
     override fun start() {
-        wifiConnectionMonitor.start()
+        if (settings.getAutoplayUserSetting() == AUTOPLAY_ALLOW_ON_WIFI) {
+            wifiConnectionMonitor.start()
+            addWifiConnectedListener()
+        }
     }
 
     override fun stop() {
         wifiConnectionMonitor.stop()
+        removeWifiConnectedListener()
     }
 }

app/src/main/java/org/mozilla/fenix/wifi/WifiConnectionMonitor.kt

@@ -10,6 +10,7 @@ import android.net.ConnectivityManager
 import android.net.Network
 import android.net.NetworkCapabilities
 import android.net.NetworkRequest
+import androidx.annotation.VisibleForTesting
 
 /**
  * Attaches itself to the [Application] and listens for WIFI available/not available events. This
@@ -26,14 +27,22 @@ import android.net.NetworkRequest
  * ```
  */
 class WifiConnectionMonitor(app: Application) {
-    private val callbacks = mutableListOf<(Boolean) -> Unit>()
-    private val connectivityManager = app.getSystemService(Context.CONNECTIVITY_SERVICE) as
+    @VisibleForTesting
+    internal val callbacks = mutableListOf<(Boolean) -> Unit>()
+
+    @VisibleForTesting
+    internal var connectivityManager = app.getSystemService(Context.CONNECTIVITY_SERVICE) as
             ConnectivityManager
 
-    private var lastKnownStateWasAvailable: Boolean? = null
-    private var isRegistered = false
+    @VisibleForTesting
+    internal var lastKnownStateWasAvailable: Boolean? = null
+
+    @VisibleForTesting
+    internal var isRegistered = false
+        @Synchronized set
 
-    private val frameworkListener = object : ConnectivityManager.NetworkCallback() {
+    @VisibleForTesting
+    internal val frameworkListener = object : ConnectivityManager.NetworkCallback() {
         override fun onLost(network: Network) {
             notifyListeners(false)
             lastKnownStateWasAvailable = false
@@ -86,6 +95,8 @@ class WifiConnectionMonitor(app: Application) {
         if (!isRegistered) return
         connectivityManager.unregisterNetworkCallback(frameworkListener)
         isRegistered = false
+        lastKnownStateWasAvailable = null
+        callbacks.clear()
     }
 
     /**

app/src/test/java/org/mozilla/fenix/settings/quicksettings/QuickSettingsFragmentStoreTest.kt

@@ -125,7 +125,7 @@ class QuickSettingsFragmentStoreTest {
         every { permissions.mediaKeySystemAccess } returns SitePermissions.Status.NO_DECISION
         every { permissions.autoplayAudible } returns SitePermissions.AutoplayStatus.ALLOWED
         every { permissions.autoplayInaudible } returns SitePermissions.AutoplayStatus.BLOCKED
-        every { appSettings.getAutoplayUserSetting(any()) } returns AUTOPLAY_BLOCK_ALL
+        every { appSettings.getAutoplayUserSetting() } returns AUTOPLAY_BLOCK_ALL
 
         val state = QuickSettingsFragmentStore.createWebsitePermissionState(
             context, permissions, permissionHighlights, appSettings