Fix background ANR from connectivity broadcast receiver

WordPress/src/main/java/org/wordpress/android/AppInitializer.kt

@@ -11,12 +11,9 @@ import android.app.NotificationManager
 import android.app.SyncNotedAppOp
 import android.content.ComponentCallbacks2
 import android.content.Context
-import android.content.ContextWrapper
-import android.content.IntentFilter
 import android.content.res.Configuration
 import android.database.SQLException
 import android.database.sqlite.SQLiteException
-import android.net.ConnectivityManager
 import android.net.http.HttpResponseCache
 import android.os.Build
 import android.os.Build.VERSION_CODES
@@ -67,7 +64,7 @@ import org.wordpress.android.fluxc.store.StatsStore
 import org.wordpress.android.fluxc.tools.FluxCImageLoader
 import org.wordpress.android.fluxc.utils.ErrorUtils.OnUnexpectedError
 import org.wordpress.android.modules.APPLICATION_SCOPE
-import org.wordpress.android.networking.ConnectionChangeReceiver
+import org.wordpress.android.networking.NetworkConnectionMonitor
 import org.wordpress.android.networking.OAuthAuthenticator
 import org.wordpress.android.networking.RestClientUtils
 import org.wordpress.android.push.GCMRegistrationScheduler
@@ -92,7 +89,6 @@ import org.wordpress.android.ui.uploads.UploadService
 import org.wordpress.android.ui.uploads.UploadStarter
 import org.wordpress.android.util.AppLog
 import org.wordpress.android.util.AppLog.T
-import org.wordpress.android.util.AppLog.T.MAIN
 import org.wordpress.android.util.AppThemeUtils
 import org.wordpress.android.util.BitmapLruCache
 import org.wordpress.android.util.BuildConfigWrapper
@@ -148,6 +144,9 @@ class AppInitializer @Inject constructor(
     @Inject
     lateinit var uploadStarter: UploadStarter
 
+    @Inject
+    lateinit var networkConnectionMonitor: NetworkConnectionMonitor
+
     @Inject
     lateinit var statsWidgetUpdaters: StatsWidgetUpdaters
 
@@ -339,6 +338,11 @@ class AppInitializer @Inject constructor(
         // Make the UploadStarter observe the app process so it can auto-start uploads
         uploadStarter.activateAutoUploading(ProcessLifecycleOwner.get() as ProcessLifecycleOwner)
 
+        // Monitor default-network connectivity for the whole process lifetime. Uses a NetworkCallback on a
+        // background thread instead of the deprecated CONNECTIVITY_ACTION broadcast, so it can't cause a
+        // background ANR. start() is idempotent, so it's safe if init() runs more than once.
+        networkConnectionMonitor.start(application)
+
         initAnalytics(SystemClock.elapsedRealtime() - startDate)
 
         updateNotificationSettings()
@@ -792,7 +796,6 @@ class AppInitializer @Inject constructor(
     inner class ApplicationLifecycleMonitor {
         private var lastPingDate: Date? = null
         private var applicationOpenedDate: Date? = null
-        private var connectionReceiverRegistered = false
         var firstActivityResumed = true
 
         private val isPushNotificationPingNeeded: Boolean
@@ -845,18 +848,6 @@ class AppInitializer @Inject constructor(
 
             AnalyticsTracker.track(Stat.APPLICATION_CLOSED, properties)
             AnalyticsTracker.endSession(false)
-            // Methods onAppComesFromBackground and onAppGoesToBackground are only workarounds to track when the app
-            // goes to or comes from background. The workarounds are not 100% reliable, so avoid unregistering the
-            // receiver twice.
-            if (connectionReceiverRegistered) {
-                connectionReceiverRegistered = false
-                try {
-                    application.unregisterReceiver(ConnectionChangeReceiver.getInstance())
-                    AppLog.d(MAIN, "ConnectionChangeReceiver successfully unregistered")
-                } catch (e: IllegalArgumentException) {
-                    AppLog.e(MAIN, "ConnectionChangeReceiver was already unregistered")
-                }
-            }
 
             // Disable the widgets if needed
             disableWidgetReceiversIfNeeded()
@@ -876,25 +867,6 @@ class AppInitializer @Inject constructor(
             }
             WordPress.appIsInTheBackground = false
 
-            // https://developer.android.com/reference/android/net/ConnectivityManager.html
-            // Apps targeting Android 7.0 (API level 24) and higher do not receive this broadcast if the broadcast
-            // receiver is declared in their manifest. Apps will still receive broadcasts if BroadcastReceiver is
-            // registered with Context.registerReceiver() and that context is still valid.
-            if (!connectionReceiverRegistered) {
-                connectionReceiverRegistered = true
-                if (Build.VERSION.SDK_INT >= VERSION_CODES.UPSIDE_DOWN_CAKE) {
-                    application.registerReceiver(
-                        ConnectionChangeReceiver.getInstance(),
-                        IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION),
-                        ContextWrapper.RECEIVER_EXPORTED
-                    )
-                } else {
-                    application.registerReceiver(
-                        ConnectionChangeReceiver.getInstance(),
-                        IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION)
-                    )
-                }
-            }
             AnalyticsUtils.refreshMetadata(accountStore, siteStore)
             applicationOpenedDate = Date()
             // This stat is part of a funnel that provides critical information. Before making ANY modification to this

WordPress/src/main/java/org/wordpress/android/networking/NetworkConnectionMonitor.kt

@@ -0,0 +1,84 @@
+package org.wordpress.android.networking
+
+import android.content.Context
+import android.net.ConnectivityManager
+import android.net.Network
+import android.net.NetworkCapabilities
+import android.net.NetworkRequest
+import android.os.Handler
+import android.os.HandlerThread
+import androidx.annotation.VisibleForTesting
+import androidx.lifecycle.LiveData
+import androidx.lifecycle.MutableLiveData
+import org.wordpress.android.util.AppLog
+import org.wordpress.android.util.AppLog.T
+import javax.inject.Inject
+import javax.inject.Singleton
+
+/**
+ * Global monitor for changes to the device's network connectivity.
+ *
+ * Uses [ConnectivityManager.NetworkCallback] registered on a background [HandlerThread] rather than the
+ * deprecated CONNECTIVITY_ACTION broadcast. Because a NetworkCallback is not a broadcast it is not subject to
+ * background-broadcast ANR timeouts, and all connectivity work runs off the main thread. The connected state
+ * is exposed as [isConnected]; observers are notified on the main thread via LiveData whenever the connected
+ * state changes.
+ *
+ * Connectivity is tracked as the set of currently-available internet-capable networks rather than a single
+ * 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.
+ */
+@Singleton
+class NetworkConnectionMonitor @Inject constructor() {
+    private var started = false
+    private var wasConnected = false
+    private var isFirstCallback = true
+    private val availableNetworks = mutableSetOf<Network>()
+
+    private val _isConnected = MutableLiveData<Boolean>()
+    val isConnected: LiveData<Boolean> = _isConnected
+
+    @Synchronized
+    fun start(context: Context) {
+        if (started) return
+        val manager = context.applicationContext
+            .getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager ?: return
+
+        val thread = HandlerThread("NetworkConnectionMonitor").apply { start() }
+        val callback = object : ConnectivityManager.NetworkCallback() {
+            override fun onAvailable(network: Network) = onNetworkAvailable(network)
+            override fun onLost(network: Network) = onNetworkLost(network)
+        }
+        val request = NetworkRequest.Builder()
+            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+            .build()
+        manager.registerNetworkCallback(request, callback, Handler(thread.looper))
+        started = true
+    }
+
+    @VisibleForTesting
+    internal fun onNetworkAvailable(network: Network) {
+        availableNetworks.add(network)
+        onConnectivityChanged(availableNetworks.isNotEmpty())
+    }
+
+    @VisibleForTesting
+    internal fun onNetworkLost(network: Network) {
+        availableNetworks.remove(network)
+        onConnectivityChanged(availableNetworks.isNotEmpty())
+    }
+
+    /**
+     * 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.
+     */
+    private fun onConnectivityChanged(isConnected: Boolean) {
+        if (isFirstCallback || isConnected != wasConnected) {
+            isFirstCallback = false
+            wasConnected = isConnected
+            AppLog.i(T.UTILS, "Connection status changed, isConnected=$isConnected")
+            _isConnected.postValue(isConnected)
+        }
+    }
+}

WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java

@@ -62,7 +62,7 @@
 import org.wordpress.android.inappupdate.IInAppUpdateManager;
 import org.wordpress.android.inappupdate.InAppUpdateListener;
 import org.wordpress.android.ui.accounts.login.LoginAnalyticsListener;
-import org.wordpress.android.networking.ConnectionChangeReceiver;
+import org.wordpress.android.networking.NetworkConnectionMonitor;
 import org.wordpress.android.push.GCMMessageHandler;
 import org.wordpress.android.push.GCMMessageService;
 import org.wordpress.android.push.GCMRegistrationScheduler;
@@ -279,6 +279,8 @@ public class WPMainActivity extends BaseAppCompatActivity implements
 
     @Inject WpAppNotifierHandler mWpAppNotifierHandler;
 
+    @Inject NetworkConnectionMonitor mNetworkConnectionMonitor;
+
     /*
      * fragments implement this if their contents can be scrolled, called when user
      * requests to scroll to the top
@@ -414,6 +416,8 @@ && getIntent().getExtras().getBoolean(ARG_CONTINUE_JETPACK_CONNECT, false)) {
         mDispatcher.register(this);
         EventBus.getDefault().register(this);
 
+        mNetworkConnectionMonitor.isConnected().observe(this, this::updateConnectionBar);
+
         if (authTokenToSet != null) {
             // Save Token to the AccountStore. This will trigger a onAuthenticationChanged.
             UpdateTokenPayload payload = new UpdateTokenPayload(authTokenToSet);
@@ -1488,12 +1492,6 @@ public void onEventMainThread(NotificationEvents.NotificationsUnseenStatus event
         if (mBottomNav != null) mBottomNav.showNoteBadge(event.hasUnseenNotes);
     }
 
-    @SuppressWarnings("unused")
-    @Subscribe(threadMode = ThreadMode.MAIN)
-    public void onEventMainThread(ConnectionChangeReceiver.ConnectionChangeEvent event) {
-        updateConnectionBar(event.isConnected());
-    }
-
     private void checkConnection() {
         updateConnectionBar(NetworkUtils.isNetworkAvailable(this));
     }

WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.kt

@@ -119,7 +119,7 @@ import org.wordpress.android.fluxc.store.bloggingprompts.BloggingPromptsStore
 import org.wordpress.android.fluxc.tools.FluxCImageLoader
 import org.wordpress.android.imageeditor.preview.PreviewImageFragment
 import org.wordpress.android.imageeditor.preview.PreviewImageFragment.Companion.EditImageData.InputData
-import org.wordpress.android.networking.ConnectionChangeReceiver.ConnectionChangeEvent
+import org.wordpress.android.networking.NetworkConnectionMonitor
 import org.wordpress.android.support.ZendeskHelper
 import org.wordpress.android.ui.ActivityId
 import org.wordpress.android.ui.ActivityLauncher
@@ -412,6 +412,8 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor
     @Inject lateinit var storageUtilsViewModel: StorageUtilsViewModel
     @Inject lateinit var editorBloggingPromptsViewModel: EditorBloggingPromptsViewModel
     @Inject lateinit var editorJetpackSocialViewModel: EditorJetpackSocialViewModel
+
+    @Inject lateinit var networkConnectionMonitor: NetworkConnectionMonitor
     private lateinit var editPostNavigationViewModel: EditPostNavigationViewModel
     private lateinit var editPostSettingsViewModel: EditPostSettingsViewModel
     private lateinit var prepublishingViewModel: PrepublishingViewModel
@@ -558,6 +560,9 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor
         }
         editorMedia.start(siteModel, this)
         startObserving()
+        networkConnectionMonitor.isConnected.observe(this) { connected ->
+            (editorFragment as? GutenbergNetworkConnectionListener)?.onConnectionStatusChange(connected)
+        }
         editorFragment?.let {
             hasSetPostContent = true
             it.setImageLoader(imageLoader)
@@ -3858,11 +3863,6 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor
         }
     }
 
-    @Subscribe(threadMode = ThreadMode.MAIN)
-    fun onEventMainThread(event: ConnectionChangeEvent) {
-        (editorFragment as? GutenbergNetworkConnectionListener)?.onConnectionStatusChange(event.isConnected)
-    }
-
     private fun refreshEditorTheme() {
         val payload = FetchEditorThemePayload(siteModel, gssEnabled = true)
         dispatcher.dispatch(EditorThemeActionBuilder.newFetchEditorThemeAction(payload))