fix(widget): clamp chart bitmaps to launcher's memory budget on large resize#35
Conversation
… resize
Resizing the widget to cover more than ~75% of the screen area crashed
both the widget and the app. The widget sets two bitmaps per update
(light + dark charts, toggled by night-mode qualifiers), and the launcher
rejects a RemoteViews update whose total bitmap memory exceeds 1.5x the
screen area, throwing IllegalArgumentException from updateAppWidget():
RemoteViews for widget update exceeds maximum bitmap memory usage
(used: 200000000, max: 26956800)
at MeteogramWidgetProvider.onAppWidgetOptionsChanged(...)
The receiver runs in the app process, so the uncaught throw took the app
and widget down together. Stock launchers cap widget size below the
threshold, but third-party launchers (e.g. Smart Launcher) allow resizing
past it.
Fix:
- WidgetUtils.clampChartDimensions() scales requested bitmap dimensions
down (preserving aspect ratio) so two bitmaps fit the budget, targeting
0.6x screen area each for margin. Applied on resize and on update.
- Wrap both updateAppWidget() calls in try/catch as a safety net so an
over-budget update can never crash the in-process receiver.
Covers MeteogramWidgetProvider and the inheriting weekly provider. Adds
unit tests for the clamp math (pass-through, full-screen scaling, very
tall/narrow, non-positive inputs).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 48 minutes and 21 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds ChangesWidget Bitmap Clamping
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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 (1)
android/app/src/test/kotlin/org/bortnik/meteogram/WidgetUtilsTest.kt (1)
228-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the 0.6 safety target, not just the 0.75 hard cap.
These tests only prove the result fits the launcher's theoretical limit. A regression from the intended
0.6x screenAreabudget to0.75xwould still pass, even though that margin is the core safety buffer described in this PR. Add at least one assertion that locks in the0.6target (or the exact expected full-screen clamp result).Proposed test tightening
- /** Two bitmaps of [w]x[h] must fit the launcher's 1.5x-screen budget. */ + /** clampChartDimensions targets a 0.6x-screen area budget per bitmap. */ private fun assertFitsBudget(w: Int, h: Int) { val area = w.toLong() * h.toLong() assertTrue( - "bitmap area $area must be <= 0.75x screen ${screenArea()} so two fit 1.5x", - area <= (screenArea() * 3L) / 4L + "bitmap area $area must be <= 0.6x screen ${screenArea()}", + area <= (screenArea() * 3L) / 5L ) }🤖 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/test/kotlin/org/bortnik/meteogram/WidgetUtilsTest.kt` around lines 228 - 266, The WidgetUtilsTest coverage currently only verifies the relaxed 0.75 screen-area cap, so a regression to the intended 0.6 safety budget would still pass. Tighten the assertions in the clampChartDimensions tests, especially the helper assertFitsBudget and the full-screen case, to validate the 0.6 target directly (or assert the exact expected clamped dimensions) using WidgetUtils.clampChartDimensions and screenArea().
🤖 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/test/kotlin/org/bortnik/meteogram/WidgetUtilsTest.kt`:
- Around line 228-266: The WidgetUtilsTest coverage currently only verifies the
relaxed 0.75 screen-area cap, so a regression to the intended 0.6 safety budget
would still pass. Tighten the assertions in the clampChartDimensions tests,
especially the helper assertFitsBudget and the full-screen case, to validate the
0.6 target directly (or assert the exact expected clamped dimensions) using
WidgetUtils.clampChartDimensions and screenArea().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 85c019db-9d3c-493f-ad08-c9bcbef6188a
📒 Files selected for processing (3)
android/app/src/main/kotlin/org/bortnik/meteogram/MeteogramWidgetProvider.ktandroid/app/src/main/kotlin/org/bortnik/meteogram/WidgetUtils.ktandroid/app/src/test/kotlin/org/bortnik/meteogram/WidgetUtilsTest.kt
assertFitsBudget only checked the launcher's hard 0.75x-per-bitmap limit, so a regression loosening clampChartDimensions' intended 0.6x margin to 0.75x would still pass. Tighten the bound to 0.6x screen area to lock in the safety margin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… freezing (#38) * fix(widget): recover from launcher bitmap-budget rejection instead of 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> * refactor(widget): unify resize path through buildWidgetViews (review) 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> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Resizing the widget to cover more than ~75% of the screen area crashed both the widget and the app. The widget sets two bitmaps per update (light + dark charts, toggled by night-mode qualifiers), and the launcher rejects a RemoteViews update whose total bitmap memory exceeds 1.5x the screen area, throwing IllegalArgumentException from updateAppWidget():
RemoteViews for widget update exceeds maximum bitmap memory usage
(used: 200000000, max: 26956800)
at MeteogramWidgetProvider.onAppWidgetOptionsChanged(...)
The receiver runs in the app process, so the uncaught throw took the app and widget down together. Stock launchers cap widget size below the threshold, but third-party launchers (e.g. Smart Launcher) allow resizing past it.
Fix:
Covers MeteogramWidgetProvider and the inheriting weekly provider. Adds unit tests for the clamp math (pass-through, full-screen scaling, very tall/narrow, non-positive inputs).
Summary by CodeRabbit
Bug Fixes
Tests