fix(cdn): defer artifact refresh UI until app is foregrounded#2260
Closed
cursor[bot] wants to merge 1 commit into
Closed
fix(cdn): defer artifact refresh UI until app is foregrounded#2260cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
PR #2252 fixed the appBundle/translation ordering race on resume, but _refreshIfStale() began calling _handlePendingUpdate() whenever config was set—even when the refresh was triggered from a paused/inactive lifecycle callback. That contradicts the background fetch contract ("update cache but DON'T fire events") and can leave translations stale: _refreshTranslationsAtRuntime() skips when no BuildContext exists, yet _fireManifestRefreshEvent() still runs and clears _hasPendingUpdate, so resume never retries i18n refresh. Only deliver refresh events immediately when lifecycle is resumed (or unknown during cold start). Otherwise set _hasPendingUpdate for the existing resume handler. Co-authored-by: Sharjeel Yunus <sharjeelyunus@users.noreply.github.com>
1 task
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
2 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.
Bug and impact
Trigger: A CDN-backed app with
ENABLE_ARTIFACT_REFRESH=trueis backgrounded while_refreshIfStale()fetches a newer manifest. The fetch completes before the user returns to the app.Impact: Screens can rebuild with updated resources but stale translations. Users see outdated i18n strings until a full app restart. This is significant user-facing breakage for localized CDN apps.
Root cause
PR #2252 correctly ordered
notifyAppBundleChanges()→ translation refresh →ResourceRefreshEventin_handlePendingUpdate(), but_refreshIfStale()started calling_handlePendingUpdate()wheneverEnsemble().getConfig() != null—including when invoked fromonAppLifecycleStateChanged(paused|inactive).That contradicts the existing background contract ("fetch and update cache but DON'T fire events"). In background,
_refreshTranslationsAtRuntime()silently skips when noBuildContextexists, yet_fireManifestRefreshEvent()still runs and clears_hasPendingUpdate. On resume, the pending handler does not run again, so translations never refresh.Fix
_handlePendingUpdate()delivery on app lifecycle: only whenWidgetsBinding.instance.lifecycleStateisnull(cold start) orresumed._hasPendingUpdate = trueso the existing resume path runs the full ordered update with a valid context.cdnShouldDeliverArtifactRefreshImmediately()for testability.Validation
modules/ensemble/test/cdn_provider_test.dartfor lifecycle gating.modules/ensemble:flutter test test/cdn_provider_test.dartDuplicate check
cdn_updates, merged): introduced the immediate-delivery path; this PR corrects its background regression without reverting the cold-start/resume ordering fix.cursor/missing-test-coverage-3ae8): adds CDN/upload tests only; does not fix lifecycle deferral.listenForChangesdedup — different code path.