Skip to content

Commit 3ab17e4

Browse files
authored
Fix background ANR from connectivity broadcast receiver (#23140)
* Fix background ANR from connectivity broadcast receiver Replace the deprecated CONNECTIVITY_ACTION BroadcastReceiver (ConnectionChangeReceiver) with a ConnectivityManager.NetworkCallback (NetworkConnectionMonitor) registered on a background thread. Connectivity checks and the EventBus dispatch no longer run on the main thread, and a NetworkCallback is not a broadcast so it cannot trip the background-broadcast ANR timeout. The monitor is registered once for the process lifetime, dropping the unreliable foreground/background register/unregister workaround. The ConnectionChangeEvent contract is preserved, so subscribers are unchanged apart from the renamed enclosing type. Sentry: JETPACK-ANDROID-PB9 * Address review: keep connectivity subscribers on-main and fix handover - SuggestionActivity's ConnectionChangeEvent subscriber used a bare @subscribe (POSTING). Since the event is now posted from a background thread, its updateEmptyView() touched Views off the main thread. Pin it to ThreadMode.MAIN. - NetworkConnectionMonitor.onLost no longer assumes the device is offline: during a network handover (e.g. Wi-Fi -> cellular) the replacement is already the default, so re-query the active network's capabilities instead of posting a spurious disconnected event. * Simplify: drop unused stop() from NetworkConnectionMonitor The monitor is registered once for the whole process lifetime and never torn down, so stop() had no callers. Removing it also lets the handlerThread and networkCallback fields go — they were only read by stop(). connectivityManager stays since hasActiveConnection() uses it. * Fix detekt ReturnCount in NetworkConnectionMonitor Collapse hasActiveConnection's guard clauses into a safe-call chain so it has two returns instead of four. No behavior change. * Fix connection bar not updating live on connectivity change Fix 2 re-queried ConnectivityManager.getActiveNetwork() in onLost to avoid misreporting a network handover as a disconnect. On real devices getActiveNetwork() can still return the network being torn down at that instant, so a genuine disconnect was suppressed by the de-dupe, leaving wasConnected stuck true. The "No connection" bar then only updated on the next onResume poll, not live. Track the set of available internet-capable networks instead (registerNetworkCallback with a NET_CAPABILITY_INTERNET request): a handover adds the replacement before removing the old network so the set never empties (no false disconnect), while a real disconnect empties it reliably. Removes the fragile getActiveNetwork() query. * Soften handover doc comment in NetworkConnectionMonitor Clarify that the available-networks set only stays non-empty during a handover when the replacement network is already up; no code change. * Replace connectivity EventBus with LiveData Drop the ConnectionChangeEvent-over-EventBus delivery introduced with NetworkConnectionMonitor and expose the connected state as a LiveData<Boolean> that subscribers observe via their lifecycle owner. Removes EventBus entirely from SuggestionActivity and BlogPreferencesActivity; the other subscribers keep EventBus for unrelated events. * Simplify connectivity transition guards to primitive booleans * Add unit tests for NetworkConnectionMonitor Extract the NetworkCallback logic into @VisibleForTesting onNetworkAvailable/ onNetworkLost so the connected-state model can be tested without the Android framework wiring in start(). Add NetworkConnectionMonitorTest covering: connected on first network, disconnected when the last network is lost, no false disconnect during a handover, de-dupe when unchanged, and re-emit on reconnect. Satisfies the Danger check requiring tests for the new class. * Guard connectivity observers against LiveData replay side effects LiveData replays the current value to each new observer, so a side effect that should only run on an offline->online transition also fired on open. - SuggestionActivity: only call viewModel.onConnectionChanged (which refreshes suggestions) on a genuine offline->online transition. The initial replay was triggering a redundant fetch on top of the one started in viewModel.init(). - ReaderPostListFragment: initialise lastConnected to true so the first replayed "connected" value doesn't re-run the search (e.g. on a restored fragment with an active search); only a real transition resubmits.
1 parent 9e0dba0 commit 3ab17e4

12 files changed

Lines changed: 229 additions & 154 deletions

File tree

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

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,9 @@ import android.app.NotificationManager
1111
import android.app.SyncNotedAppOp
1212
import android.content.ComponentCallbacks2
1313
import android.content.Context
14-
import android.content.ContextWrapper
15-
import android.content.IntentFilter
1614
import android.content.res.Configuration
1715
import android.database.SQLException
1816
import android.database.sqlite.SQLiteException
19-
import android.net.ConnectivityManager
2017
import android.net.http.HttpResponseCache
2118
import android.os.Build
2219
import android.os.Build.VERSION_CODES
@@ -67,7 +64,7 @@ import org.wordpress.android.fluxc.store.StatsStore
6764
import org.wordpress.android.fluxc.tools.FluxCImageLoader
6865
import org.wordpress.android.fluxc.utils.ErrorUtils.OnUnexpectedError
6966
import org.wordpress.android.modules.APPLICATION_SCOPE
70-
import org.wordpress.android.networking.ConnectionChangeReceiver
67+
import org.wordpress.android.networking.NetworkConnectionMonitor
7168
import org.wordpress.android.networking.OAuthAuthenticator
7269
import org.wordpress.android.networking.RestClientUtils
7370
import org.wordpress.android.push.GCMRegistrationScheduler
@@ -92,7 +89,6 @@ import org.wordpress.android.ui.uploads.UploadService
9289
import org.wordpress.android.ui.uploads.UploadStarter
9390
import org.wordpress.android.util.AppLog
9491
import org.wordpress.android.util.AppLog.T
95-
import org.wordpress.android.util.AppLog.T.MAIN
9692
import org.wordpress.android.util.AppThemeUtils
9793
import org.wordpress.android.util.BitmapLruCache
9894
import org.wordpress.android.util.BuildConfigWrapper
@@ -148,6 +144,9 @@ class AppInitializer @Inject constructor(
148144
@Inject
149145
lateinit var uploadStarter: UploadStarter
150146

147+
@Inject
148+
lateinit var networkConnectionMonitor: NetworkConnectionMonitor
149+
151150
@Inject
152151
lateinit var statsWidgetUpdaters: StatsWidgetUpdaters
153152

@@ -339,6 +338,11 @@ class AppInitializer @Inject constructor(
339338
// Make the UploadStarter observe the app process so it can auto-start uploads
340339
uploadStarter.activateAutoUploading(ProcessLifecycleOwner.get() as ProcessLifecycleOwner)
341340

341+
// Monitor default-network connectivity for the whole process lifetime. Uses a NetworkCallback on a
342+
// background thread instead of the deprecated CONNECTIVITY_ACTION broadcast, so it can't cause a
343+
// background ANR. start() is idempotent, so it's safe if init() runs more than once.
344+
networkConnectionMonitor.start(application)
345+
342346
initAnalytics(SystemClock.elapsedRealtime() - startDate)
343347

344348
updateNotificationSettings()
@@ -792,7 +796,6 @@ class AppInitializer @Inject constructor(
792796
inner class ApplicationLifecycleMonitor {
793797
private var lastPingDate: Date? = null
794798
private var applicationOpenedDate: Date? = null
795-
private var connectionReceiverRegistered = false
796799
var firstActivityResumed = true
797800

798801
private val isPushNotificationPingNeeded: Boolean
@@ -845,18 +848,6 @@ class AppInitializer @Inject constructor(
845848

846849
AnalyticsTracker.track(Stat.APPLICATION_CLOSED, properties)
847850
AnalyticsTracker.endSession(false)
848-
// Methods onAppComesFromBackground and onAppGoesToBackground are only workarounds to track when the app
849-
// goes to or comes from background. The workarounds are not 100% reliable, so avoid unregistering the
850-
// receiver twice.
851-
if (connectionReceiverRegistered) {
852-
connectionReceiverRegistered = false
853-
try {
854-
application.unregisterReceiver(ConnectionChangeReceiver.getInstance())
855-
AppLog.d(MAIN, "ConnectionChangeReceiver successfully unregistered")
856-
} catch (e: IllegalArgumentException) {
857-
AppLog.e(MAIN, "ConnectionChangeReceiver was already unregistered")
858-
}
859-
}
860851

861852
// Disable the widgets if needed
862853
disableWidgetReceiversIfNeeded()
@@ -876,25 +867,6 @@ class AppInitializer @Inject constructor(
876867
}
877868
WordPress.appIsInTheBackground = false
878869

879-
// https://developer.android.com/reference/android/net/ConnectivityManager.html
880-
// Apps targeting Android 7.0 (API level 24) and higher do not receive this broadcast if the broadcast
881-
// receiver is declared in their manifest. Apps will still receive broadcasts if BroadcastReceiver is
882-
// registered with Context.registerReceiver() and that context is still valid.
883-
if (!connectionReceiverRegistered) {
884-
connectionReceiverRegistered = true
885-
if (Build.VERSION.SDK_INT >= VERSION_CODES.UPSIDE_DOWN_CAKE) {
886-
application.registerReceiver(
887-
ConnectionChangeReceiver.getInstance(),
888-
IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION),
889-
ContextWrapper.RECEIVER_EXPORTED
890-
)
891-
} else {
892-
application.registerReceiver(
893-
ConnectionChangeReceiver.getInstance(),
894-
IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION)
895-
)
896-
}
897-
}
898870
AnalyticsUtils.refreshMetadata(accountStore, siteStore)
899871
applicationOpenedDate = Date()
900872
// 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/ConnectionChangeReceiver.java

Lines changed: 0 additions & 64 deletions
This file was deleted.
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package org.wordpress.android.networking
2+
3+
import android.content.Context
4+
import android.net.ConnectivityManager
5+
import android.net.Network
6+
import android.net.NetworkCapabilities
7+
import android.net.NetworkRequest
8+
import android.os.Handler
9+
import android.os.HandlerThread
10+
import androidx.annotation.VisibleForTesting
11+
import androidx.lifecycle.LiveData
12+
import androidx.lifecycle.MutableLiveData
13+
import org.wordpress.android.util.AppLog
14+
import org.wordpress.android.util.AppLog.T
15+
import javax.inject.Inject
16+
import javax.inject.Singleton
17+
18+
/**
19+
* Global monitor for changes to the device's network connectivity.
20+
*
21+
* Uses [ConnectivityManager.NetworkCallback] registered on a background [HandlerThread] rather than the
22+
* deprecated CONNECTIVITY_ACTION broadcast. Because a NetworkCallback is not a broadcast it is not subject to
23+
* background-broadcast ANR timeouts, and all connectivity work runs off the main thread. The connected state
24+
* is exposed as [isConnected]; observers are notified on the main thread via LiveData whenever the connected
25+
* state changes.
26+
*
27+
* Connectivity is tracked as the set of currently-available internet-capable networks rather than a single
28+
* network. During a handover (e.g. Wi-Fi -> cellular) where the replacement is already up, it is added before
29+
* the old one is removed, so the set doesn't empty and the handover isn't misreported as a disconnection; a
30+
* genuine disconnect empties the set and is reported reliably.
31+
*/
32+
@Singleton
33+
class NetworkConnectionMonitor @Inject constructor() {
34+
private var started = false
35+
private var wasConnected = false
36+
private var isFirstCallback = true
37+
private val availableNetworks = mutableSetOf<Network>()
38+
39+
private val _isConnected = MutableLiveData<Boolean>()
40+
val isConnected: LiveData<Boolean> = _isConnected
41+
42+
@Synchronized
43+
fun start(context: Context) {
44+
if (started) return
45+
val manager = context.applicationContext
46+
.getSystemService(Context.CONNECTIVITY_SERVICE) as? ConnectivityManager ?: return
47+
48+
val thread = HandlerThread("NetworkConnectionMonitor").apply { start() }
49+
val callback = object : ConnectivityManager.NetworkCallback() {
50+
override fun onAvailable(network: Network) = onNetworkAvailable(network)
51+
override fun onLost(network: Network) = onNetworkLost(network)
52+
}
53+
val request = NetworkRequest.Builder()
54+
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
55+
.build()
56+
manager.registerNetworkCallback(request, callback, Handler(thread.looper))
57+
started = true
58+
}
59+
60+
@VisibleForTesting
61+
internal fun onNetworkAvailable(network: Network) {
62+
availableNetworks.add(network)
63+
onConnectivityChanged(availableNetworks.isNotEmpty())
64+
}
65+
66+
@VisibleForTesting
67+
internal fun onNetworkLost(network: Network) {
68+
availableNetworks.remove(network)
69+
onConnectivityChanged(availableNetworks.isNotEmpty())
70+
}
71+
72+
/**
73+
* Called on the monitor's background thread. Updates [isConnected] on the first callback and whenever the
74+
* connected state actually changes, so observers aren't spammed while connectivity churns.
75+
*/
76+
private fun onConnectivityChanged(isConnected: Boolean) {
77+
if (isFirstCallback || isConnected != wasConnected) {
78+
isFirstCallback = false
79+
wasConnected = isConnected
80+
AppLog.i(T.UTILS, "Connection status changed, isConnected=$isConnected")
81+
_isConnected.postValue(isConnected)
82+
}
83+
}
84+
}

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
import org.wordpress.android.inappupdate.IInAppUpdateManager;
6363
import org.wordpress.android.inappupdate.InAppUpdateListener;
6464
import org.wordpress.android.ui.accounts.login.LoginAnalyticsListener;
65-
import org.wordpress.android.networking.ConnectionChangeReceiver;
65+
import org.wordpress.android.networking.NetworkConnectionMonitor;
6666
import org.wordpress.android.push.GCMMessageHandler;
6767
import org.wordpress.android.push.GCMMessageService;
6868
import org.wordpress.android.push.GCMRegistrationScheduler;
@@ -279,6 +279,8 @@ public class WPMainActivity extends BaseAppCompatActivity implements
279279

280280
@Inject WpAppNotifierHandler mWpAppNotifierHandler;
281281

282+
@Inject NetworkConnectionMonitor mNetworkConnectionMonitor;
283+
282284
/*
283285
* fragments implement this if their contents can be scrolled, called when user
284286
* requests to scroll to the top
@@ -414,6 +416,8 @@ && getIntent().getExtras().getBoolean(ARG_CONTINUE_JETPACK_CONNECT, false)) {
414416
mDispatcher.register(this);
415417
EventBus.getDefault().register(this);
416418

419+
mNetworkConnectionMonitor.isConnected().observe(this, this::updateConnectionBar);
420+
417421
if (authTokenToSet != null) {
418422
// Save Token to the AccountStore. This will trigger a onAuthenticationChanged.
419423
UpdateTokenPayload payload = new UpdateTokenPayload(authTokenToSet);
@@ -1488,12 +1492,6 @@ public void onEventMainThread(NotificationEvents.NotificationsUnseenStatus event
14881492
if (mBottomNav != null) mBottomNav.showNoteBadge(event.hasUnseenNotes);
14891493
}
14901494

1491-
@SuppressWarnings("unused")
1492-
@Subscribe(threadMode = ThreadMode.MAIN)
1493-
public void onEventMainThread(ConnectionChangeReceiver.ConnectionChangeEvent event) {
1494-
updateConnectionBar(event.isConnected());
1495-
}
1496-
14971495
private void checkConnection() {
14981496
updateConnectionBar(NetworkUtils.isNetworkAvailable(this));
14991497
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ import org.wordpress.android.fluxc.store.bloggingprompts.BloggingPromptsStore
119119
import org.wordpress.android.fluxc.tools.FluxCImageLoader
120120
import org.wordpress.android.imageeditor.preview.PreviewImageFragment
121121
import org.wordpress.android.imageeditor.preview.PreviewImageFragment.Companion.EditImageData.InputData
122-
import org.wordpress.android.networking.ConnectionChangeReceiver.ConnectionChangeEvent
122+
import org.wordpress.android.networking.NetworkConnectionMonitor
123123
import org.wordpress.android.support.ZendeskHelper
124124
import org.wordpress.android.ui.ActivityId
125125
import org.wordpress.android.ui.ActivityLauncher
@@ -412,6 +412,8 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor
412412
@Inject lateinit var storageUtilsViewModel: StorageUtilsViewModel
413413
@Inject lateinit var editorBloggingPromptsViewModel: EditorBloggingPromptsViewModel
414414
@Inject lateinit var editorJetpackSocialViewModel: EditorJetpackSocialViewModel
415+
416+
@Inject lateinit var networkConnectionMonitor: NetworkConnectionMonitor
415417
private lateinit var editPostNavigationViewModel: EditPostNavigationViewModel
416418
private lateinit var editPostSettingsViewModel: EditPostSettingsViewModel
417419
private lateinit var prepublishingViewModel: PrepublishingViewModel
@@ -558,6 +560,9 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor
558560
}
559561
editorMedia.start(siteModel, this)
560562
startObserving()
563+
networkConnectionMonitor.isConnected.observe(this) { connected ->
564+
(editorFragment as? GutenbergNetworkConnectionListener)?.onConnectionStatusChange(connected)
565+
}
561566
editorFragment?.let {
562567
hasSetPostContent = true
563568
it.setImageLoader(imageLoader)
@@ -3858,11 +3863,6 @@ class EditPostActivity : BaseAppCompatActivity(), EditorFragmentActivity, Editor
38583863
}
38593864
}
38603865

3861-
@Subscribe(threadMode = ThreadMode.MAIN)
3862-
fun onEventMainThread(event: ConnectionChangeEvent) {
3863-
(editorFragment as? GutenbergNetworkConnectionListener)?.onConnectionStatusChange(event.isConnected)
3864-
}
3865-
38663866
private fun refreshEditorTheme() {
38673867
val payload = FetchEditorThemePayload(siteModel, gssEnabled = true)
38683868
dispatcher.dispatch(EditorThemeActionBuilder.newFetchEditorThemeAction(payload))

0 commit comments

Comments
 (0)