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

by default

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

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

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

@@ -159,7 +159,7 @@ class SettingsSubMenuSitePermissionsRobot {
         onView(withText("Autoplay"))
             .check(matches(withEffectiveVisibility(Visibility.VISIBLE)))
 
-        val autoplayText = "Block audio and video"
+        val autoplayText = "Block audio only"
         onView(withText(autoplayText))
             .check(matches(withEffectiveVisibility(Visibility.VISIBLE)))
 

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

@@ -799,12 +799,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
         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

@@ -535,7 +535,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

@@ -46,7 +46,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_AUDIBLE) {
                         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

@@ -77,14 +77,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),
@@ -107,10 +105,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 {
@@ -127,7 +122,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)
                 }
@@ -142,10 +140,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)
                 }
@@ -164,7 +160,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()
         }
@@ -185,9 +181,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,9 +40,10 @@ 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_AUDIBLE
 import java.security.InvalidParameterException
 
-private const val AUTOPLAY_USER_SETTING = "AUTOPLAY_USER_SETTING"
+private const val AUTOPLAY_USER_SETTING = "AUTOPLAY_USER_SETTING_v1"
 
 /**
  * A simple wrapper for SharedPreferences that makes reading preference a little bit easier.
@@ -722,9 +723,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_AUDIBLE)
 
     private fun getSitePermissionsPhoneFeatureAutoplayAction(
         feature: PhoneFeature,
@@ -745,7 +744,10 @@ class Settings(private val appContext: Context) : PreferencesHolder {
             location = getSitePermissionsPhoneFeatureAction(PhoneFeature.LOCATION),
             camera = getSitePermissionsPhoneFeatureAction(PhoneFeature.CAMERA),
             autoplayAudible = getSitePermissionsPhoneFeatureAutoplayAction(PhoneFeature.AUTOPLAY_AUDIBLE),
-            autoplayInaudible = getSitePermissionsPhoneFeatureAutoplayAction(PhoneFeature.AUTOPLAY_INAUDIBLE),
+            autoplayInaudible = getSitePermissionsPhoneFeatureAutoplayAction(
+                PhoneFeature.AUTOPLAY_INAUDIBLE,
+                AutoplayAction.ALLOWED
+            ),
             persistentStorage = getSitePermissionsPhoneFeatureAction(PhoneFeature.PERSISTENT_STORAGE)
         )
     }

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,21 @@ 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
 
-    private val frameworkListener = object : ConnectivityManager.NetworkCallback() {
+    @VisibleForTesting
+    internal val frameworkListener = object : ConnectivityManager.NetworkCallback() {
         override fun onLost(network: Network?) {
             notifyListeners(false)
             lastKnownStateWasAvailable = false
@@ -86,6 +94,8 @@ class WifiConnectionMonitor(app: Application) {
         if (!isRegistered) return
         connectivityManager.unregisterNetworkCallback(frameworkListener)
         isRegistered = false
+        lastKnownStateWasAvailable = null
+        callbacks.clear()
     }
 
     /**

app/src/main/res/values/preference_keys.xml

@@ -108,8 +108,8 @@
     <string name="pref_key_show_site_exceptions" translatable="false">pref_key_show_site_exceptions</string>
     <string name="pref_key_recommended_settings" translatable="false">pref_key_recommended_settings</string>
     <string name="pref_key_custom_settings" translatable="false">pref_key_custom_settings</string>
-    <string name="pref_key_browser_feature_autoplay_audible" translatable="false">pref_key_browser_feature_autoplay</string>
-    <string name="pref_key_browser_feature_autoplay_inaudible" translatable="false">pref_key_browser_feature_autoplay_inaudible</string>
+    <string name="pref_key_browser_feature_autoplay_audible" translatable="false">pref_key_browser_feature_autoplay_v1</string>
+    <string name="pref_key_browser_feature_autoplay_inaudible" translatable="false">pref_key_browser_feature_autoplay_inaudible_v1</string>
     <string name="pref_key_browser_feature_persistent_storage" translatable="false">pref_key_browser_feature_persistent_storage</string>
     <string name="pref_key_phone_feature_camera" translatable="false">pref_key_phone_feature_camera</string>
     <string name="pref_key_phone_feature_location" translatable="false">pref_key_phone_feature_location</string>

app/src/test/java/org/mozilla/fenix/settings/PhoneFeatureTest.kt

@@ -77,7 +77,7 @@ class PhoneFeatureTest {
         assertEquals(R.string.pref_key_browser_feature_autoplay_inaudible, PhoneFeature.AUTOPLAY_INAUDIBLE.getPreferenceId())
 
         assertEquals(
-            "pref_key_browser_feature_autoplay_inaudible",
+            "pref_key_browser_feature_autoplay_inaudible_v1",
             PhoneFeature.AUTOPLAY_INAUDIBLE.getPreferenceKey(testContext)
         )
     }

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

@@ -27,7 +27,7 @@ import org.mozilla.fenix.settings.PhoneFeature
 import org.mozilla.fenix.settings.quicksettings.QuickSettingsFragmentStore.Companion.toWebsitePermission
 import org.mozilla.fenix.settings.quicksettings.ext.shouldBeEnabled
 import org.mozilla.fenix.settings.quicksettings.ext.shouldBeVisible
-import org.mozilla.fenix.settings.sitepermissions.AUTOPLAY_BLOCK_ALL
+import org.mozilla.fenix.settings.sitepermissions.AUTOPLAY_BLOCK_AUDIBLE
 import org.mozilla.fenix.utils.Settings
 
 @RunWith(FenixRobolectricTestRunner::class)
@@ -97,7 +97,7 @@ class QuickSettingsFragmentStoreTest {
         every { permissions.localStorage } returns SitePermissions.Status.ALLOWED
         every { permissions.autoplayAudible } returns SitePermissions.Status.BLOCKED
         every { permissions.autoplayInaudible } returns SitePermissions.Status.BLOCKED
-        every { appSettings.getAutoplayUserSetting(any()) } returns AUTOPLAY_BLOCK_ALL
+        every { appSettings.getAutoplayUserSetting() } returns AUTOPLAY_BLOCK_AUDIBLE
 
         val state = QuickSettingsFragmentStore.createWebsitePermissionState(
             context, permissions, appSettings