Drop EventBus from connectivity delivery (use LiveData) - #23141
Closed
nbradbury wants to merge 8 commits into
Closed
Drop EventBus from connectivity delivery (use LiveData)#23141nbradbury wants to merge 8 commits into
nbradbury wants to merge 8 commits into
Conversation
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
- 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.
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.
Collapse hasActiveConnection's guard clauses into a safe-call chain so it has two returns instead of four. No behavior 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.
Clarify that the available-networks set only stays non-empty during a handover when the replacement network is already up; no code change.
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.
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## issue/network-changed-anr #23141 +/- ##
=============================================================
- Coverage 37.82% 37.82% -0.01%
=============================================================
Files 2344 2344
Lines 127449 127450 +1
Branches 17694 17694
=============================================================
- Hits 48210 48209 -1
- Misses 75282 75284 +2
Partials 3957 3957 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Closing this in favor of including these changes in #23140 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stacked on #23140. Removes the
ConnectionChangeEvent-over-EventBus delivery that #23140 kept for connectivity, and exposes the connected state as a lifecycle-awareLiveData<Boolean>onNetworkConnectionMonitorthat subscribers observe directly.Why
EventBus for a single connectivity signal is legacy global-bus plumbing: untyped dispatch, manual register/unregister with leak risk, and no current-value semantics. The monitor is already a Dagger
@Singletonand every subscriber is aLifecycleOwnerwith field injection, soLiveDatais a better fit — automatic lifecycle-scoped observation and current-value replay on subscribe.Testing
Smoke-tested on an emulator (toggle airplane mode):