From c3ba6ed9467fbcd04171e9e54e23e6a646d60f2f Mon Sep 17 00:00:00 2001 From: ekager Date: Wed, 9 Sep 2020 14:58:11 -0700 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/14680 - Revert ObserverRegistry changes to WifiConnectionMonitor --- .../mozilla/fenix/components/Components.kt | 4 +- .../wifi/SitePermissionsWifiIntegration.kt | 32 ++++--- .../fenix/wifi/WifiConnectionMonitor.kt | 53 +++++++---- .../SitePermissionsWifiIntegrationTest.kt | 89 ------------------ .../fenix/wifi/WifiConnectionMonitorTest.kt | 91 ------------------- 5 files changed, 59 insertions(+), 210 deletions(-) delete mode 100644 app/src/test/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegrationTest.kt delete mode 100644 app/src/test/java/org/mozilla/fenix/wifi/WifiConnectionMonitorTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index 5a313401c1..f6e94701b2 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -4,9 +4,9 @@ package org.mozilla.fenix.components +import android.app.Application import android.content.Context import android.content.Intent -import androidx.core.content.getSystemService import androidx.core.net.toUri import mozilla.components.feature.addons.AddonManager import mozilla.components.feature.addons.amo.AddonCollectionProvider @@ -111,7 +111,7 @@ class Components(private val context: Context) { val migrationStore by lazy { MigrationStore() } val performance by lazy { PerformanceComponent() } val push by lazy { Push(context, analytics.crashReporter) } - val wifiConnectionMonitor by lazy { WifiConnectionMonitor(context.getSystemService()!!) } + val wifiConnectionMonitor by lazy { WifiConnectionMonitor(context as Application) } val settings by lazy { Settings(context) } diff --git a/app/src/main/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegration.kt b/app/src/main/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegration.kt index cee0a6a034..eec7d25e6b 100644 --- a/app/src/main/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegration.kt +++ b/app/src/main/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegration.kt @@ -18,21 +18,29 @@ import org.mozilla.fenix.utils.Settings class SitePermissionsWifiIntegration( private val settings: Settings, private val wifiConnectionMonitor: WifiConnectionMonitor -) : LifecycleAwareFeature, WifiConnectionMonitor.Observer { +) : LifecycleAwareFeature { /** * Adds listener for autoplay setting [AUTOPLAY_ALLOW_ON_WIFI]. Sets all autoplay to allowed when * WIFI is connected, blocked otherwise. */ - override fun onWifiConnectionChanged(connected: Boolean) { - val setting = - if (connected) SitePermissionsRules.Action.ALLOWED else SitePermissionsRules.Action.BLOCKED - if (settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) == AUTOPLAY_ALLOW_ON_WIFI) { - settings.setSitePermissionsPhoneFeatureAction(PhoneFeature.AUTOPLAY_AUDIBLE, setting) - settings.setSitePermissionsPhoneFeatureAction(PhoneFeature.AUTOPLAY_INAUDIBLE, setting) - } else { - // The autoplay setting has changed, we can remove the listener - removeWifiConnectedListener() + private 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) { + settings.setSitePermissionsPhoneFeatureAction( + PhoneFeature.AUTOPLAY_AUDIBLE, + setting + ) + settings.setSitePermissionsPhoneFeatureAction( + PhoneFeature.AUTOPLAY_INAUDIBLE, + setting + ) + } else { + // The autoplay setting has changed, we can remove the listener + removeWifiConnectedListener() + } } } @@ -47,11 +55,11 @@ class SitePermissionsWifiIntegration( } fun addWifiConnectedListener() { - wifiConnectionMonitor.register(this) + wifiConnectionMonitor.addOnWifiConnectedChangedListener(wifiConnectedListener) } fun removeWifiConnectedListener() { - wifiConnectionMonitor.unregister(this) + wifiConnectionMonitor.removeOnWifiConnectedChangedListener(wifiConnectedListener) } // Until https://bugzilla.mozilla.org/show_bug.cgi?id=1621825 is fixed, AUTOPLAY_ALLOW_ALL diff --git a/app/src/main/java/org/mozilla/fenix/wifi/WifiConnectionMonitor.kt b/app/src/main/java/org/mozilla/fenix/wifi/WifiConnectionMonitor.kt index b8e0c2ee9f..34a6e5f7a5 100644 --- a/app/src/main/java/org/mozilla/fenix/wifi/WifiConnectionMonitor.kt +++ b/app/src/main/java/org/mozilla/fenix/wifi/WifiConnectionMonitor.kt @@ -5,12 +5,11 @@ package org.mozilla.fenix.wifi import android.app.Application +import android.content.Context import android.net.ConnectivityManager import android.net.Network import android.net.NetworkCapabilities import android.net.NetworkRequest -import mozilla.components.support.base.observer.Observable -import mozilla.components.support.base.observer.ObserverRegistry /** * Attaches itself to the [Application] and listens for WIFI available/not available events. This @@ -26,28 +25,30 @@ import mozilla.components.support.base.observer.ObserverRegistry * app.components.wifiConnectionListener.start() * ``` */ -class WifiConnectionMonitor( - private val connectivityManager: ConnectivityManager -) : Observable by ObserverRegistry() { +class WifiConnectionMonitor(app: Application) { + private val callbacks = mutableSetOf<(Boolean) -> Unit>() + private val connectivityManager = app.getSystemService(Context.CONNECTIVITY_SERVICE) as + ConnectivityManager - private var callbackReceived: Boolean = false + private var lastKnownStateWasAvailable: Boolean? = null private var isRegistered = false private val frameworkListener = object : ConnectivityManager.NetworkCallback() { override fun onLost(network: Network?) { - notifyAtLeastOneObserver { onWifiConnectionChanged(connected = false) } - callbackReceived = true + callbacks.forEach { it(false) } + lastKnownStateWasAvailable = false } override fun onAvailable(network: Network?) { - notifyAtLeastOneObserver { onWifiConnectionChanged(connected = true) } - callbackReceived = true + callbacks.forEach { it(true) } + lastKnownStateWasAvailable = true } } /** * Attaches the [WifiConnectionMonitor] to the application. After this has been called, callbacks - * added via [register] will be called until either the app exits, or [stop] is called. + * added via [addOnWifiConnectedChangedListener] will be called until either the app exits, or + * [stop] is called. * * Any existing callbacks will be called with the current state when this is called. */ @@ -61,8 +62,10 @@ class WifiConnectionMonitor( // AFAICT, the framework does not send an event when a new NetworkCallback is registered // while the WIFI is not connected, so we push this manually. If the WIFI is on, it will send // a follow up event shortly - if (!callbackReceived) { - notifyAtLeastOneObserver { onWifiConnectionChanged(connected = false) } + val noCallbacksReceivedYet = lastKnownStateWasAvailable == null + if (noCallbacksReceivedYet) { + lastKnownStateWasAvailable = false + callbacks.forEach { it(false) } } connectivityManager.registerNetworkCallback(request, frameworkListener) @@ -71,7 +74,7 @@ class WifiConnectionMonitor( /** * Detatches the [WifiConnectionMonitor] from the app. No callbacks added via - * [register] will be called after this has been called. + * [addOnWifiConnectedChangedListener] will be called after this has been called. */ fun stop() { // Framework code will throw if an unregistered listener attempts to unregister. @@ -80,7 +83,25 @@ class WifiConnectionMonitor( isRegistered = false } - interface Observer { - fun onWifiConnectionChanged(connected: Boolean) + /** + * Adds [onWifiChanged] to a list of listeners that will be called whenever WIFI connects or + * disconnects. + * + * If [onWifiChanged] is successfully added (i.e., it is a new listener), it will be immediately + * called with the last known state. + */ + fun addOnWifiConnectedChangedListener(onWifiChanged: (Boolean) -> Unit) { + val lastKnownState = lastKnownStateWasAvailable + if (callbacks.add(onWifiChanged) && lastKnownState != null) { + onWifiChanged(lastKnownState) + } + } + + /** + * Removes [onWifiChanged] from the list of listeners to be called whenever WIFI connects or + * disconnects. + */ + fun removeOnWifiConnectedChangedListener(onWifiChanged: (Boolean) -> Unit) { + callbacks.remove(onWifiChanged) } } diff --git a/app/src/test/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegrationTest.kt b/app/src/test/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegrationTest.kt deleted file mode 100644 index 77c481bf29..0000000000 --- a/app/src/test/java/org/mozilla/fenix/wifi/SitePermissionsWifiIntegrationTest.kt +++ /dev/null @@ -1,89 +0,0 @@ -/* 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.wifi - -import io.mockk.Called -import io.mockk.Runs -import io.mockk.every -import io.mockk.just -import io.mockk.mockk -import io.mockk.verify -import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action -import org.junit.Before -import org.junit.Test -import org.mozilla.fenix.settings.PhoneFeature.AUTOPLAY_AUDIBLE -import org.mozilla.fenix.settings.PhoneFeature.AUTOPLAY_INAUDIBLE -import org.mozilla.fenix.settings.sitepermissions.AUTOPLAY_ALLOW_ALL -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 - -class SitePermissionsWifiIntegrationTest { - - private lateinit var settings: Settings - private lateinit var wifiConnectionMonitor: WifiConnectionMonitor - private lateinit var wifiIntegration: SitePermissionsWifiIntegration - - @Before - fun setup() { - settings = mockk() - wifiConnectionMonitor = mockk(relaxed = true) - wifiIntegration = SitePermissionsWifiIntegration(settings, wifiConnectionMonitor) - - every { settings.setSitePermissionsPhoneFeatureAction(any(), any()) } just Runs - } - - @Test - fun `add and remove wifi connected listener`() { - wifiIntegration.addWifiConnectedListener() - verify { wifiConnectionMonitor.register(any()) } - - wifiIntegration.removeWifiConnectedListener() - verify { wifiConnectionMonitor.unregister(any()) } - } - - @Test - fun `start and stop wifi connection monitor`() { - wifiIntegration.start() - verify { wifiConnectionMonitor.start() } - - wifiIntegration.stop() - verify { wifiConnectionMonitor.stop() } - } - - @Test - fun `add only if autoplay is only allowed on wifi`() { - every { settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) } returns AUTOPLAY_ALLOW_ALL - wifiIntegration.maybeAddWifiConnectedListener() - verify { wifiConnectionMonitor wasNot Called } - - every { settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) } returns AUTOPLAY_ALLOW_ON_WIFI - wifiIntegration.maybeAddWifiConnectedListener() - verify { wifiConnectionMonitor.register(any()) } - } - - @Test - fun `listener removes itself if autoplay is not only allowed on wifi`() { - every { settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) } returns AUTOPLAY_ALLOW_ALL - wifiIntegration.onWifiConnectionChanged(connected = true) - verify { wifiConnectionMonitor.unregister(any()) } - } - - @Test - fun `listener sets audible and inaudible settings to allowed on connect`() { - every { settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) } returns AUTOPLAY_ALLOW_ON_WIFI - wifiIntegration.onWifiConnectionChanged(connected = true) - verify { settings.setSitePermissionsPhoneFeatureAction(AUTOPLAY_AUDIBLE, Action.ALLOWED) } - verify { settings.setSitePermissionsPhoneFeatureAction(AUTOPLAY_INAUDIBLE, Action.ALLOWED) } - } - - @Test - fun `listener sets audible and inaudible settings to blocked on disconnected`() { - every { settings.getAutoplayUserSetting(default = AUTOPLAY_BLOCK_ALL) } returns AUTOPLAY_ALLOW_ON_WIFI - wifiIntegration.onWifiConnectionChanged(connected = false) - verify { settings.setSitePermissionsPhoneFeatureAction(AUTOPLAY_AUDIBLE, Action.BLOCKED) } - verify { settings.setSitePermissionsPhoneFeatureAction(AUTOPLAY_INAUDIBLE, Action.BLOCKED) } - } -} diff --git a/app/src/test/java/org/mozilla/fenix/wifi/WifiConnectionMonitorTest.kt b/app/src/test/java/org/mozilla/fenix/wifi/WifiConnectionMonitorTest.kt deleted file mode 100644 index 94b8a7f10c..0000000000 --- a/app/src/test/java/org/mozilla/fenix/wifi/WifiConnectionMonitorTest.kt +++ /dev/null @@ -1,91 +0,0 @@ -/* 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.wifi - -import android.net.ConnectivityManager -import android.net.NetworkRequest -import io.mockk.Runs -import io.mockk.every -import io.mockk.just -import io.mockk.mockk -import io.mockk.mockkConstructor -import io.mockk.slot -import io.mockk.unmockkConstructor -import io.mockk.verify -import org.junit.After -import org.junit.Before -import org.junit.Test - -class WifiConnectionMonitorTest { - - private lateinit var connectivityManager: ConnectivityManager - private lateinit var wifiConnectionMonitor: WifiConnectionMonitor - - @Before - fun setup() { - mockkConstructor(NetworkRequest.Builder::class) - connectivityManager = mockk(relaxUnitFun = true) - wifiConnectionMonitor = WifiConnectionMonitor(connectivityManager) - - every { - anyConstructed().addTransportType(any()) - } answers { self as NetworkRequest.Builder } - } - - @After - fun teardown() { - unmockkConstructor(NetworkRequest.Builder::class) - } - - @Test - fun `start runs only once`() { - wifiConnectionMonitor.start() - wifiConnectionMonitor.start() - - verify(exactly = 1) { - connectivityManager.registerNetworkCallback(any(), any()) - } - } - - @Test - fun `stop only runs after start`() { - wifiConnectionMonitor.stop() - verify(exactly = 0) { - connectivityManager.unregisterNetworkCallback(any()) - } - - wifiConnectionMonitor.start() - wifiConnectionMonitor.stop() - verify { - connectivityManager.unregisterNetworkCallback(any()) - } - } - - @Test - fun `passes results from connectivity manager to observers`() { - val slot = slot() - every { connectivityManager.registerNetworkCallback(any(), capture(slot)) } just Runs - - wifiConnectionMonitor.start() - - // Immediately notifies observer when registered - val observer = mockk(relaxed = true) - wifiConnectionMonitor.register(observer) - verify { observer.onWifiConnectionChanged(connected = false) } - - // Notifies observer when network is available or lost - slot.captured.onAvailable(mockk()) - verify { observer.onWifiConnectionChanged(connected = true) } - - slot.captured.onLost(mockk()) - verify { observer.onWifiConnectionChanged(connected = false) } - } - - private fun captureNetworkCallback(): ConnectivityManager.NetworkCallback { - val slot = slot() - verify { connectivityManager.registerNetworkCallback(any(), capture(slot)) } - return slot.captured - } -}