Fix widget, animation, and controller lifecycle leaks - #227
Open
Xare123 wants to merge 7 commits into
Open
Conversation
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
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.
Part 2 of 3. This replaces #225, which mixed too many unrelated themes to review. Split into focused PRs so each can be evaluated on its own.
Series: Part 1 - delivery integrity - Part 2 - lifecycle leaks - Part 3 - reply threads and keyboard
What this fixes
Scrolling a long transcript produced gray error blocks where messages should be, and the app leaked subscriptions and animation tickers as rows were recycled.
Recycled rows crashed on remount.
updateObxwas alate finalcallback that got re-initialized when a widget was recycled, throwingLateErrorand rendering an error block instead of the message. On a long transcript this cascaded: once one row failed, scrolling produced more. Controller state is now torn down on recycle and re-initialized cleanly on remount, with a regression test that mounts, disposes, and remounts the same controller.Subscriptions outlived their widgets. Event listeners on conversation tiles, message holders, attachment and interactive holders, text bubbles, delivered indicators, and the tablet-mode wrapper were never cancelled. They are now cancelled on dispose.
Animation tickers were not disposed. The balloon, celebration, fireworks, laser, love, and spotlight effect classes each leaked their ticker. Screen effects and bubble effects held controllers past their widget's life.
Partial initialization could be observed. Widgets could build against half-initialized state during profile and contact hydration. Initialization is now guarded so a partially built widget cannot be rendered.
Full-resolution images were decoded eagerly. Conversation view decoded attachments at full resolution before they were needed, spiking memory on image-heavy chats. Decoding is now deferred and size-bounded.
Group avatars churned background controllers. Refreshing a group avatar spun up and discarded controllers repeatedly; it now refreshes in place.
Malformed shared profiles could disturb delivery. A bad CloudKit profile payload could escape an unawaited task. Those failures are now isolated and rate-limited with a bounded retry map, so a repeatedly replayed bad payload cannot consume CPU.
Validation
flutter analyze: no errors in any file this PR touches. The analyzer reports 31 pre-existing errors inlib/database/html/, vendoredrust_builder/cargokit/, and thetelephony_plusexample, plus two intest_driver/; all are unchanged by this PR and present on the base branch.LateErrorrender failures while scrolling a long transcript. After the change, the same device and same build type rendered 1,788 frames with 10 janky frames (0.56%), p50 5 ms, p95 6 ms, p99 12 ms, and zero render errors.Notes for review
rustpushsubmodule pointer.messages_view.dart,rustpush_service.dart,conversation_tile.dart) and three with Part 3 (conversation_view.dart,message_holder.dart,conversation_view_controller.dart), but edits different regions of each. Whichever merges first, I will rebase the others.