From f36c873ff56e70f854486a69abd33ea1b4634e4b Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 28 Aug 2026 11:43:30 -0400 Subject: [PATCH 1/3] Back ConnectionStatusLiveData with NetworkConnectionMonitor Removes the second CONNECTIVITY_ACTION BroadcastReceiver, left behind when #23140 migrated the app's other connectivity listener to a ConnectivityManager.NetworkCallback. ConnectionStatusLiveData registered its own receiver in onActive/onInactive and read the deprecated activeNetworkInfo on the main thread in onReceive. It is now a MediatorLiveData over NetworkConnectionMonitor.isConnected. The public ConnectionStatus enum is unchanged, so UploadStarter, PostListViewModel, HistoryViewModel and WPWebViewViewModel are untouched. The monitor is a process-lifetime singleton that holds the current state, and LiveData replays its value to every new observer, so a direct swap would have re-run each observer's side effect (loadIfNecessary, fetchRevisions, retryOnConnectionAvailableAfterRefreshError) on every subscribe. The new class seeds a baseline from the source's value at construction and only emits on an actual change, which matches the old contract: under both implementations the first genuine connectivity change is the one UploadStarter's skip(1) swallows. Note this is not an ANR fix. The receiver was ProcessLifecycleOwner-scoped and mostly unregistered while backgrounded; the background ANRs in CMM-2174 come from Application.onCreate work on background process starts. --- .../android/modules/ApplicationModule.java | 5 +- .../helpers/ConnectionStatusLiveData.kt | 63 ++++------- .../helpers/ConnectionStatusLiveDataTest.kt | 106 +++++++----------- 3 files changed, 61 insertions(+), 113 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/modules/ApplicationModule.java b/WordPress/src/main/java/org/wordpress/android/modules/ApplicationModule.java index 0054d21d4227..993b9b694417 100644 --- a/WordPress/src/main/java/org/wordpress/android/modules/ApplicationModule.java +++ b/WordPress/src/main/java/org/wordpress/android/modules/ApplicationModule.java @@ -33,6 +33,7 @@ import org.wordpress.android.util.wizard.WizardManager; import org.wordpress.android.fluxc.network.TrackNetworkRequestsInterceptor; import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpNetworkAvailabilityProvider; +import org.wordpress.android.networking.NetworkConnectionMonitor; import org.wordpress.android.viewmodel.helpers.ConnectionStatus; import org.wordpress.android.viewmodel.helpers.ConnectionStatusLiveData; @@ -71,8 +72,8 @@ public static WizardManager provideWizardManager( } @Provides - static LiveData provideConnectionStatusLiveData(@ApplicationContext Context context) { - return new ConnectionStatusLiveData.Factory(context).create(); + static LiveData provideConnectionStatusLiveData(NetworkConnectionMonitor monitor) { + return new ConnectionStatusLiveData(monitor.isConnected()); } @Provides diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveData.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveData.kt index 699c64962623..548ff683b8dc 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveData.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveData.kt @@ -1,17 +1,7 @@ package org.wordpress.android.viewmodel.helpers -import android.content.BroadcastReceiver -import android.content.Context -import android.content.ContextWrapper -import android.content.Intent -import android.content.IntentFilter -import android.net.ConnectivityManager -import android.os.Build -import android.os.Build.VERSION_CODES import androidx.lifecycle.LiveData -import org.wordpress.android.util.distinct -import org.wordpress.android.viewmodel.helpers.ConnectionStatusLiveData.Factory -import javax.inject.Inject +import androidx.lifecycle.MediatorLiveData enum class ConnectionStatus { AVAILABLE, @@ -21,44 +11,29 @@ enum class ConnectionStatus { /** * A LiveData instance that can be injected to keep track of the network availability. * - * Use [Factory] to create an instance. The Factory guarantees that this only emits if the network availability - * changes and not when the user switches between cellular and wi-fi. + * Backed by [org.wordpress.android.networking.NetworkConnectionMonitor], which observes connectivity through a + * ConnectivityManager.NetworkCallback on a background thread rather than the deprecated CONNECTIVITY_ACTION + * broadcast. + * + * Only emits when the connected state changes. The state the monitor already holds when this instance is created + * is taken as the baseline, so the value LiveData replays to a new observer is swallowed instead of re-running + * that observer's side effect (a refresh, a retry, an upload) every time it starts observing. * * IMPORTANT: It needs to be observed for the changes to be posted. */ -class ConnectionStatusLiveData private constructor(private val context: Context) : LiveData() { - @Suppress("DEPRECATION") - override fun onActive() { - super.onActive() - val intentFilter = IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION) - if (Build.VERSION.SDK_INT >= VERSION_CODES.UPSIDE_DOWN_CAKE) { - context.registerReceiver(networkReceiver, intentFilter, ContextWrapper.RECEIVER_EXPORTED) - } else { - context.registerReceiver(networkReceiver, intentFilter) - } - } - - override fun onInactive() { - super.onInactive() - context.unregisterReceiver(networkReceiver) - } +class ConnectionStatusLiveData(source: LiveData) : MediatorLiveData() { + private var lastStatus: ConnectionStatus? = source.value?.toConnectionStatus() - @Suppress("DEPRECATION") - private val networkReceiver = object : BroadcastReceiver() { - override fun onReceive(context: Context, intent: Intent) { - val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager? - val networkInfo = connectivityManager?.activeNetworkInfo - - val nextValue: ConnectionStatus = if (networkInfo?.isConnected == true) { - ConnectionStatus.AVAILABLE - } else { - ConnectionStatus.UNAVAILABLE + init { + addSource(source) { isConnected -> + val status = isConnected.toConnectionStatus() + if (status != lastStatus) { + lastStatus = status + value = status } - postValue(nextValue) } } - - class Factory @Inject constructor(private val context: Context) { - fun create() = ConnectionStatusLiveData(context).distinct() - } } + +private fun Boolean.toConnectionStatus() = + if (this) ConnectionStatus.AVAILABLE else ConnectionStatus.UNAVAILABLE diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveDataTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveDataTest.kt index d11ceda1485e..ec1c66df7e8e 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveDataTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/helpers/ConnectionStatusLiveDataTest.kt @@ -1,97 +1,69 @@ -@file:Suppress("DEPRECATION") - package org.wordpress.android.viewmodel.helpers -import android.annotation.SuppressLint -import android.content.BroadcastReceiver -import android.content.Context -import android.net.ConnectivityManager -import android.net.NetworkInfo -import androidx.lifecycle.LiveData +import androidx.lifecycle.MutableLiveData import kotlinx.coroutines.ExperimentalCoroutinesApi import org.assertj.core.api.Assertions.assertThat -import org.junit.Before import org.junit.Test -import org.mockito.kotlin.any -import org.mockito.kotlin.argumentCaptor -import org.mockito.kotlin.doReturn -import org.mockito.kotlin.mock import org.wordpress.android.BaseUnitTest +import org.wordpress.android.viewmodel.helpers.ConnectionStatus.AVAILABLE +import org.wordpress.android.viewmodel.helpers.ConnectionStatus.UNAVAILABLE @ExperimentalCoroutinesApi class ConnectionStatusLiveDataTest : BaseUnitTest() { - private lateinit var connectionStatusLiveData: LiveData - private lateinit var broadcastReceiver: BroadcastReceiver - - @Before - fun setUp() { - val captor = argumentCaptor() - @SuppressLint("UnspecifiedRegisterReceiverFlag") - val context = mock { - on { registerReceiver(captor.capture(), any()) } doReturn mock() - } - - connectionStatusLiveData = ConnectionStatusLiveData.Factory(context).create() - // Start observing to capture the broadcastReceiver - connectionStatusLiveData.observeForever { } - - broadcastReceiver = captor.firstValue + private val source = MutableLiveData() + + @Test + fun `it does not emit the state the source already holds when observation starts`() { + source.value = true + + val emitted = observe() + + assertThat(emitted).isEmpty() } @Test - fun `it emits a value when receiving a network info change`() { - assertThat(connectionStatusLiveData.value).isNull() + fun `it emits when the connected state changes`() { + source.value = true + val emitted = observe() - broadcastReceiver.onReceive(mockedBroadcastReceiverContext(connectedNetwork = false), mock()) + source.value = false - assertThat(connectionStatusLiveData.value).isEqualTo(ConnectionStatus.UNAVAILABLE) + assertThat(emitted).containsExactly(UNAVAILABLE) } @Test - fun `it emits a value when the network availability changes`() { - // Arrange - broadcastReceiver.onReceive(mockedBroadcastReceiverContext(connectedNetwork = true), mock()) - assertThat(connectionStatusLiveData.value).isEqualTo(ConnectionStatus.AVAILABLE) + fun `it does not emit when the source repeats the same state`() { + source.value = true + val emitted = observe() - // Act - broadcastReceiver.onReceive(mockedBroadcastReceiverContext(connectedNetwork = false), mock()) + repeat(3) { source.value = true } - // Assert - assertThat(connectionStatusLiveData.value).isEqualTo(ConnectionStatus.UNAVAILABLE) + assertThat(emitted).isEmpty() } @Test - fun `it does not emit a value when the network available didn't change`() { - // Arrange - var emitCount = 0 + fun `it emits every change in both directions`() { + source.value = true + val emitted = observe() + + source.value = false + source.value = true - connectionStatusLiveData.observeForever { - emitCount += 1 - } + assertThat(emitted).containsExactly(UNAVAILABLE, AVAILABLE) + } - broadcastReceiver.onReceive(mockedBroadcastReceiverContext(connectedNetwork = true), mock()) + @Test + fun `it emits the first state when the source has none at creation`() { + val emitted = observe() - // Act - repeat(3) { - broadcastReceiver.onReceive(mockedBroadcastReceiverContext(connectedNetwork = true), mock()) - } + source.value = true - // Assert - assertThat(emitCount).isEqualTo(1) - assertThat(connectionStatusLiveData.value).isEqualTo(ConnectionStatus.AVAILABLE) + assertThat(emitted).containsExactly(AVAILABLE) } - @Suppress("DEPRECATION") - private fun mockedBroadcastReceiverContext(connectedNetwork: Boolean): Context { - val networkInfo = mock { - on { isConnected } doReturn connectedNetwork - } - val connectivityManager = mock { - on { activeNetworkInfo } doReturn networkInfo - } - - return mock { - on { getSystemService(any()) } doReturn connectivityManager - } + private fun observe(): List { + val emitted = mutableListOf() + ConnectionStatusLiveData(source).observeForever { emitted.add(it) } + return emitted } } From 2d238b5517ef5ff694982b81568ef2fd6fea6723 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 28 Aug 2026 11:51:36 -0400 Subject: [PATCH 2/3] Observe connectivity in MediaBrowserActivity instead of a receiver Removes the last CONNECTIVITY_ACTION BroadcastReceiver. MediaBrowserActivity registered one in onStart and unregistered it in onStop to resume pending media deletes when the connection returned; it now injects LiveData and observes it instead, so all connectivity listening goes through NetworkConnectionMonitor's NetworkCallback. observe(this, ...) is active between onStart and onStop, so the scoping is unchanged - it just isn't hand-rolled any more. Two small behaviour changes: - The old onReceive ran on any connectivity broadcast, including disconnects, despite its "Coming from zero connection" comment. The observer only acts on AVAILABLE, which is what the comment intended. startMediaDeleteService already returns early when the network is unavailable, so the disconnect-triggered calls only logged. - If the connection is restored while the Activity is stopped, the observer now fires on return to STARTED rather than missing the change entirely. onResume already called startMediaDeleteService unconditionally, so this only widens an existing path. --- .../ui/media/MediaBrowserActivity.java | 43 ++++++++----------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java index 9a7124acc522..1c0700b2a82d 100755 --- a/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java @@ -2,16 +2,12 @@ import android.Manifest; import android.app.Activity; -import android.content.BroadcastReceiver; import android.content.ComponentName; import android.content.Context; import android.content.Intent; -import android.content.IntentFilter; import android.content.ServiceConnection; -import android.net.ConnectivityManager; import android.net.Uri; import android.os.Build; -import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.IBinder; import android.text.TextUtils; @@ -37,6 +33,7 @@ import androidx.fragment.app.FragmentManager; import androidx.fragment.app.FragmentManager.OnBackStackChangedListener; import androidx.fragment.app.FragmentTransaction; +import androidx.lifecycle.LiveData; import com.google.android.material.tabs.TabLayout; @@ -91,6 +88,7 @@ import org.wordpress.android.util.WPMediaUtils; import org.wordpress.android.util.WPPermissionUtils; import org.wordpress.android.util.analytics.AnalyticsUtils; +import org.wordpress.android.viewmodel.helpers.ConnectionStatus; import org.wordpress.android.widgets.AppReviewManager; import java.util.ArrayList; @@ -130,6 +128,7 @@ public class MediaBrowserActivity extends BaseAppCompatActivity implements Media @Inject JetpackFeatureRemovalHelper mJetpackFeatureRemovalHelper; @Inject ActivityNavigator mActivityNavigator; @Inject WpAppNotifierHandler mWpAppNotifierHandler; + @Inject LiveData mConnectionStatus; private SiteModel mSite; @@ -247,6 +246,20 @@ public void onCreate(@Nullable Bundle savedInstanceState) { doAddMediaItemClicked(AddMenuItem.ITEM_CHOOSE_FILE); mLaunchPhotoPicker = false; } + + observeConnectionStatus(); + } + + /** + * Continue any pending deletes once the connection comes back. The observer is scoped to this Activity, so + * it's only active between onStart and onStop. + */ + private void observeConnectionStatus() { + mConnectionStatus.observe(this, status -> { + if (status == ConnectionStatus.AVAILABLE && mMediaStore.hasSiteMediaToDelete(mSite)) { + startMediaDeleteService(null); + } + }); } @Override @@ -427,15 +440,6 @@ public void onStart() { mWpAppNotifierHandler.addListener(this); - if (Build.VERSION.SDK_INT >= VERSION_CODES.UPSIDE_DOWN_CAKE) { - registerReceiver( - mReceiver, - new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION), - RECEIVER_EXPORTED - ); - } else { - registerReceiver(mReceiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION)); - } mDispatcher.register(this); EventBus.getDefault().register(this); } @@ -461,7 +465,6 @@ protected void onResume() { public void onStop() { mWpAppNotifierHandler.removeListener(this); EventBus.getDefault().unregister(this); - unregisterReceiver(mReceiver); mDispatcher.unregister(this); super.onStop(); } @@ -953,18 +956,6 @@ public void onServiceDisconnected(ComponentName arg0) { } }; - private final BroadcastReceiver mReceiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - if (ConnectivityManager.CONNECTIVITY_ACTION.equals(intent.getAction())) { - // Coming from zero connection. Continue what's pending for delete - if (mMediaStore.hasSiteMediaToDelete(mSite)) { - startMediaDeleteService(null); - } - } - } - }; - public void showAddMediaPopup() { View anchor = findViewById(R.id.menu_new_media); PopupMenu popup = new PopupMenu(this, anchor); From 217f07a4b434d4460eeb958ce95b72923eae00df Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 28 Aug 2026 13:49:53 -0400 Subject: [PATCH 3/3] Seed NetworkConnectionMonitor with the current connectivity state A NetworkCallback only reports networks as they appear, so on a device that starts with no connectivity nothing is ever delivered and isConnected stayed null - indistinguishable from "offline" to observers. That broke UploadStarter. CONNECTIVITY_ACTION was a sticky broadcast, so registering the old receiver delivered the current state immediately, and that is what UploadStarter's skip(1) was written to absorb. With the receiver gone and no seeded value, the first reconnect after an offline launch became ConnectionStatusLiveData's first emission, skip(1) swallowed it, and queueUploadFromAllSites() never ran - leaving local drafts unuploaded until the next background/foreground cycle. start() now publishes the current ConnectivityManager state before registering the callback, routed through onConnectivityChanged so the existing de-dupe still applies: a seeded false followed by a network is a genuine change, while a seeded true followed by onAvailable for that same network is not. onConnectivityChanged becomes @VisibleForTesting internal so the seeded paths can be covered without the framework wiring in start(). --- .../networking/NetworkConnectionMonitor.kt | 23 +++++++++++++--- .../NetworkConnectionMonitorTest.kt | 26 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/networking/NetworkConnectionMonitor.kt b/WordPress/src/main/java/org/wordpress/android/networking/NetworkConnectionMonitor.kt index d32995851773..76834ab71f6c 100644 --- a/WordPress/src/main/java/org/wordpress/android/networking/NetworkConnectionMonitor.kt +++ b/WordPress/src/main/java/org/wordpress/android/networking/NetworkConnectionMonitor.kt @@ -28,6 +28,9 @@ import javax.inject.Singleton * network. During a handover (e.g. Wi-Fi -> cellular) where the replacement is already up, it is added before * the old one is removed, so the set doesn't empty and the handover isn't misreported as a disconnection; a * genuine disconnect empties the set and is reported reliably. + * + * [isConnected] is seeded with the current state in [start], so it always has a value once the monitor is + * running - including when the device starts offline and no callback ever arrives. */ @Singleton class NetworkConnectionMonitor @Inject constructor() { @@ -45,6 +48,13 @@ class NetworkConnectionMonitor @Inject constructor() { val manager = context.applicationContext .getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager ?: return + // Publish the current state before registering. A NetworkCallback only reports networks as they appear, + // so on a device that starts with no connectivity nothing is ever delivered and [isConnected] would stay + // null - indistinguishable from "offline" to observers, and leaving the first reconnect looking like an + // initial value rather than a change. The CONNECTIVITY_ACTION broadcast this replaced was sticky and + // delivered the current state at registration; seeding here restores that. + onConnectivityChanged(manager.hasInternetCapability()) + val thread = HandlerThread("NetworkConnectionMonitor").apply { start() } val callback = object : ConnectivityManager.NetworkCallback() { override fun onAvailable(network: Network) = onNetworkAvailable(network) @@ -57,6 +67,11 @@ class NetworkConnectionMonitor @Inject constructor() { started = true } + private fun ConnectivityManager.hasInternetCapability(): Boolean { + val network = activeNetwork ?: return false + return getNetworkCapabilities(network)?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true + } + @VisibleForTesting internal fun onNetworkAvailable(network: Network) { availableNetworks.add(network) @@ -70,10 +85,12 @@ class NetworkConnectionMonitor @Inject constructor() { } /** - * Called on the monitor's background thread. Updates [isConnected] on the first callback and whenever the - * connected state actually changes, so observers aren't spammed while connectivity churns. + * Called from [start] to publish the seeded state, then on the monitor's background thread for each + * callback. Updates [isConnected] on the first call and whenever the connected state actually changes, so + * observers aren't spammed while connectivity churns. */ - private fun onConnectivityChanged(isConnected: Boolean) { + @VisibleForTesting + internal fun onConnectivityChanged(isConnected: Boolean) { if (isFirstCallback || isConnected != wasConnected) { isFirstCallback = false wasConnected = isConnected diff --git a/WordPress/src/test/java/org/wordpress/android/networking/NetworkConnectionMonitorTest.kt b/WordPress/src/test/java/org/wordpress/android/networking/NetworkConnectionMonitorTest.kt index a340a4524e3d..036048d25157 100644 --- a/WordPress/src/test/java/org/wordpress/android/networking/NetworkConnectionMonitorTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/networking/NetworkConnectionMonitorTest.kt @@ -70,4 +70,30 @@ class NetworkConnectionMonitorTest : BaseUnitTest() { assertThat(monitor.isConnected.value).isTrue assertThat(emissions).containsExactly(true, false, true) } + + @Test + fun `emits the seeded state so isConnected has a value before any callback`() { + monitor.onConnectivityChanged(false) + + assertThat(monitor.isConnected.value).isFalse + assertThat(emissions).containsExactly(false) + } + + @Test + fun `emits connected when a network arrives after seeding offline`() { + // starting offline seeds false; the first network is then a genuine change, not an initial value + monitor.onConnectivityChanged(false) + monitor.onNetworkAvailable(mock()) + + assertThat(monitor.isConnected.value).isTrue + assertThat(emissions).containsExactly(false, true) + } + + @Test + fun `does not re-emit when the first callback matches the seeded state`() { + monitor.onConnectivityChanged(true) + monitor.onNetworkAvailable(mock()) + + assertThat(emissions).containsExactly(true) + } }