Remove the remaining CONNECTIVITY_ACTION broadcast receivers

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<SiteCreationStep> provideWizardManager(
     }
 
     @Provides
-    static LiveData<ConnectionStatus> provideConnectionStatusLiveData(@ApplicationContext Context context) {
-        return new ConnectionStatusLiveData.Factory(context).create();
+    static LiveData<ConnectionStatus> provideConnectionStatusLiveData(NetworkConnectionMonitor monitor) {
+        return new ConnectionStatusLiveData(monitor.isConnected());
     }
 
     @Provides

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

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<ConnectionStatus> 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);

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<ConnectionStatus>() {
-    @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<Boolean>) : MediatorLiveData<ConnectionStatus>() {
+    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

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)
+    }
 }