Skip to content

Remove the remaining CONNECTIVITY_ACTION broadcast receivers - #23271

Open
nbradbury wants to merge 3 commits into
trunkfrom
issue/remove-connectivity-action-receiver
Open

Remove the remaining CONNECTIVITY_ACTION broadcast receivers#23271
nbradbury wants to merge 3 commits into
trunkfrom
issue/remove-connectivity-action-receiver

Conversation

@nbradbury

@nbradbury nbradbury commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

TL/DR

Removes the last two deprecated CONNECTIVITY_ACTION BroadcastReceivers in the app. All connectivity listening now goes through NetworkConnectionMonitor's ConnectivityManager.NetworkCallback, finishing the migration #23140 started.

Note that this a tech debt PR only - this fixes no known bugs and there are no user-facing changes.


Description

#23140 replaced the app's main connectivity receiver with a NetworkCallback but left two behind: ConnectionStatusLiveData registered one in onActive/onInactive and read the deprecated activeNetworkInfo on the main thread, and MediaBrowserActivity registered its own in onStart/onStop to resume pending media deletes. ConnectionStatusLiveData is now a MediatorLiveData over NetworkConnectionMonitor.isConnected, and MediaBrowserActivity injects and observes it. The public ConnectionStatus enum is unchanged, so UploadStarter, PostListViewModel, HistoryViewModel and WPWebViewViewModel are untouched.

NetworkConnectionMonitor now seeds isConnected with the current ConnectivityManager state in start(). A NetworkCallback only reports networks as they appear, so on a device that starts offline nothing is ever delivered and isConnected stayed null — indistinguishable from "offline", and it made the first reconnect look like an initial value rather than a change. CONNECTIVITY_ACTION was a sticky broadcast that delivered the current state at registration, so this restores the behaviour the receivers relied on. Without it, UploadStarter's skip(1) (UploadStarter.kt:100) would swallow the first reconnect after an offline launch and local drafts would sit unuploaded.

One subtlety worth a reviewer's eye: the monitor is a process-lifetime singleton that holds state, and LiveData replays its value to every new observer — so a direct swap would re-run each observer's side effect (loadIfNecessary, fetchRevisions, retryOnConnectionAvailableAfterRefreshError) on every subscribe. ConnectionStatusLiveData therefore seeds a baseline at construction and only emits on an actual change, which matches the old contract.

The MediaBrowserActivity half turns out to be a no-op in practice. Its observer only calls startMediaDeleteService when hasSiteMediaToDelete is true, which needs media in DELETING state; only deleteMedia sets that, and deleteMedia is reachable only from the grid trash button, which is shown only for FAILED/QUEUED items — exactly the states deleteMedia routes to newRemoveMediaAction instead. So the guarded branch is unreachable, and this is kept as a like-for-like migration. Removing the dead path is a reasonable follow-up; the file already carries a TODO about the same drift at MediaBrowserActivity:876.

Note this is not an ANR fix. Both receivers were lifecycle-scoped and unregistered while backgrounded — the background ANRs in CMM-2174 come from Application.onCreate work on background process starts.

Testing instructions

Editor offline banner:

  1. Open a post in the editor.
  2. Turn airplane mode on.
  • Verify "Working offline" appears at the top.
  1. Turn airplane mode off.
  • Verify the banner clears.

Media library still opens:

  1. Open My Site → Media.
  • Verify the library loads without crashing.
  1. Toggle airplane mode on and off with the library open.
  • Verify no crash.

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.
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<ConnectionStatus> 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.
@dangermattic

dangermattic commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator
1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot

wpmobilebot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr23271-217f07a
Build Number1498
Application IDcom.jetpack.android.prealpha
Commit217f07a
Installation URL7pmn49v8uaifg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr23271-217f07a
Build Number1498
Application IDorg.wordpress.android.prealpha
Commit217f07a
Installation URL42eh986fktea0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.80%. Comparing base (e6f795f) to head (217f07a).

Files with missing lines Patch % Lines
...ess/android/networking/NetworkConnectionMonitor.kt 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #23271      +/-   ##
==========================================
- Coverage   37.80%   37.80%   -0.01%     
==========================================
  Files        2348     2348              
  Lines      128445   128441       -4     
  Branches    17805    17807       +2     
==========================================
- Hits        48560    48558       -2     
  Misses      75925    75925              
+ Partials     3960     3958       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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().
@nbradbury
nbradbury marked this pull request as ready for review August 28, 2026 18:04
@nbradbury
nbradbury requested a review from adalpari August 28, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants