fix(widget): recover from launcher bitmap-budget rejection instead of freezing#38
Conversation
… freezing When updateAppWidget() is rejected for exceeding the launcher's per-widget bitmap-memory budget, the #35 try/catch only swallowed the throw, leaving the widget permanently and silently frozen: every later update recomputed the same over-budget size and was rejected again, fixable only by re-creating the widget (the "widget stops updating, new widget works, app fails to update on tap" report). Add updateWidgetWithRetry(): on rejection, halve both bitmap dimensions (quartering memory) and re-render via the new pure buildWidgetViews(w, h), up to 4 attempts down to a 64px floor. The chart is vector (SVG stretched FIT_XY), so a lower-res raster is visually identical — far better than a freeze. The happy path is unchanged (one render, one update). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesWidget bitmap retry-and-shrink strategy
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.kt (4)
420-457: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResize path re-implements
buildWidgetViewsinstead of reusing it.This callback duplicates the click-intent wiring, native light/dark chart generation, and
applyThemeOverridethat already live inbuildWidgetViews. The only intended difference is returningnullwhen no native bitmap was produced (so the caller can fall back to a weather fetch) versus showing the placeholder. Keeping two near-identical RemoteViews builders invites drift (e.g., placeholder/refresh-indicator/theme handling diverging between resize and update). Consider parameterizingbuildWidgetViews(e.g., anativeOnly/returnNullIfNoChartflag) so both paths share one builder.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.kt` around lines 420 - 457, The resize/update callback in MeteogramWidgetProvider is duplicating the RemoteViews construction logic from buildWidgetViews, which risks the two paths drifting apart. Refactor the shared widget rendering into buildWidgetViews (or a small helper it calls) and add a parameter such as nativeOnly or returnNullIfNoChart so the resize path can return null when both chart bitmaps are missing while still reusing the same light/dark chart generation, applyThemeOverride, and click PendingIntent wiring.
285-303: 🚀 Performance & Scalability | 🔵 TrivialRetry mechanism is sound, but it can amplify synchronous bitmap work on the main thread.
onUpdate(aBroadcastReceivercallback) andonAppWidgetOptionsChangedrun on the app's main thread. On the rejection path, each retry rebuilds the RemoteViews, which regenerates two SVG charts (light + dark) viagenerateChartBitmap. Worst case is roughlyMAX_UPDATE_ATTEMPTS × 2synchronous SVG renders per widget, all on the main thread, which can risk an ANR when several widgets are rejected at once. The happy path is unaffected; consider whether the rejection path should be offloaded (e.g., viagoAsync()/a background dispatcher) if field telemetry shows rejections clustering.As per path instructions: "Watch for RemoteViews restrictions, null safety, and main-thread work."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.kt` around lines 285 - 303, The retry loop in MeteogramWidgetProvider’s widget update path is rebuilding RemoteViews and generating SVG bitmaps synchronously on the main thread for every failed attempt, which can multiply expensive work during rejection retries. Move the rejection-path update work in onUpdate/onAppWidgetOptionsChanged off the BroadcastReceiver main thread, for example by using goAsync() with a background dispatcher or equivalent, and keep buildViews/updateAppWidget retries from blocking the UI thread while preserving the existing retry logic and logs.Source: Path instructions
305-311: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: the “pure function” doc comment overstates the contract.
buildWidgetViewsreadsSharedPreferences, emitsLogcalls, and rasterizes SVGs, so it isn't pure/referentially transparent. The relevant property forupdateWidgetWithRetryis that it can be safely re-invoked at a smaller size; consider rewording to "deterministic for given inputs / safe to re-invoke at a smaller size" to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.kt` around lines 305 - 311, Update the KDoc for buildWidgetViews in MeteogramWidgetProvider to remove the claim that it is a “pure function,” since it also reads SharedPreferences, logs, and rasterizes SVGs. Reword the contract to say it is deterministic for the same inputs and safe to re-invoke at a smaller size, which is the behavior relied on by updateWidgetWithRetry.
334-350: 🚀 Performance & Scalability | 🔵 TrivialNote: both light and dark bitmaps count toward the same budget.
estimateMemoryUsagesums every bitmap in a single RemoteViews, so setting bothwidget_chart_lightandwidget_chart_darkat full size submits ~2× the per-chart memory in one update — which makes the over-budget rejection this PR guards against more likely on smaller screens. The shrink-retry correctly halves both, so this is informational, but if rejections are common you may want to render only the bitmap matching the active/forced theme rather than both.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.kt` around lines 334 - 350, The widget update path in MeteogramWidgetProvider currently generates and submits both widget_chart_light and widget_chart_dark bitmaps in the same RemoteViews, which counts twice against the shared memory budget. If this becomes a problem, adjust the chart rendering logic in the widget update flow to generate only the bitmap that matches the active or forced theme instead of always creating both, while keeping the shrink-retry behavior intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.kt`:
- Around line 420-457: The resize/update callback in MeteogramWidgetProvider is
duplicating the RemoteViews construction logic from buildWidgetViews, which
risks the two paths drifting apart. Refactor the shared widget rendering into
buildWidgetViews (or a small helper it calls) and add a parameter such as
nativeOnly or returnNullIfNoChart so the resize path can return null when both
chart bitmaps are missing while still reusing the same light/dark chart
generation, applyThemeOverride, and click PendingIntent wiring.
- Around line 285-303: The retry loop in MeteogramWidgetProvider’s widget update
path is rebuilding RemoteViews and generating SVG bitmaps synchronously on the
main thread for every failed attempt, which can multiply expensive work during
rejection retries. Move the rejection-path update work in
onUpdate/onAppWidgetOptionsChanged off the BroadcastReceiver main thread, for
example by using goAsync() with a background dispatcher or equivalent, and keep
buildViews/updateAppWidget retries from blocking the UI thread while preserving
the existing retry logic and logs.
- Around line 305-311: Update the KDoc for buildWidgetViews in
MeteogramWidgetProvider to remove the claim that it is a “pure function,” since
it also reads SharedPreferences, logs, and rasterizes SVGs. Reword the contract
to say it is deterministic for the same inputs and safe to re-invoke at a
smaller size, which is the behavior relied on by updateWidgetWithRetry.
- Around line 334-350: The widget update path in MeteogramWidgetProvider
currently generates and submits both widget_chart_light and widget_chart_dark
bitmaps in the same RemoteViews, which counts twice against the shared memory
budget. If this becomes a problem, adjust the chart rendering logic in the
widget update flow to generate only the bitmap that matches the active or forced
theme instead of always creating both, while keeping the shrink-retry behavior
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d77c8ee8-adc4-40a0-b9c0-3cc2fdc6f50a
📒 Files selected for processing (1)
android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.kt
Address PR review: - Resize callback no longer re-implements the click intent + native chart generation + theme override; it now calls buildWidgetViews(..., nativeOnly = true). The nativeOnly flag skips the saved-SVG-file fallback and placeholder and returns null when no fresh chart is produced, preserving the resize path's "render now or fetch" behaviour while removing the duplicate builder (no more drift risk between the resize and update paths). - Reword buildWidgetViews doc: it reads prefs, logs, and rasterizes, so it isn't "pure" — the relevant contract is deterministic-for-inputs / safe to re-invoke at a smaller size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Problem
A field report on v1.2.3: the widget stops updating permanently in some conditions, fixed
only by re-creating it; a newly created widget works; and tapping the stuck widget opens the app
but it fails to update.
Root cause: the widget sets two chart bitmaps per update (
widget_chart_light+widget_chart_dark). When their combined size exceeds the launcher's per-widget RemoteViewsbudget (
~1.5 × screen × 4bytes),updateAppWidget()throwsIllegalArgumentException. Becausethe rejection depends on the widget's size (per-id state), one widget can be permanently stuck
while a fresh, smaller one renders fine.
This was largely addressed in #35 (v1.2.6) with
WidgetUtils.clampChartDimensions()+ atry/catcharoundupdateAppWidget(). But thetry/catchonly swallows the rejection — if theclamp ever under-protects (split-screen / multi-window, or a launcher whose budget basis differs
from our
displayMetrics), the widget freezes permanently and silently: every later updaterecomputes the same over-budget size → rejected again → fixable only by re-creating the widget.
That residual silent-freeze is exactly the reported symptom, minus the crash #35 removed.
Fix
Add
updateWidgetWithRetry()around bothupdateAppWidget()call sites. On a rejection ithalves both bitmap dimensions (quartering memory) and re-renders, up to 4 attempts down to a
64px floor, so the update eventually fits instead of freezing. The per-widget RemoteViews assembly
is extracted into a pure
buildWidgetViews(w, h)so it can be rebuilt at a smaller size.The chart is vector (SVG stretched
FIT_XY), so a lower-res raster on retry is visually identical— far better than a frozen widget. The happy path is unchanged (one render, one update); only
a rejection triggers a retry.
Verification
./gradlew :app:compileDebugKotlin✅./gradlew :app:test(Kotlin unit tests) ✅./gradlew :app:lintDebug— noNewApierrors; the only Error is a pre-existingMissingTranslation(app_name) unrelated to this change.Notes
adb logcat | grep -iE "MeteogramWidget|exceeds maximum bitmap"willshow
update rejected at WxH … Updated widget N at reduced …on a retry.🤖 Generated with [Claude Code]
Summary by CodeRabbit