这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Sep 23, 2025

Fixes #140684
Fixes #175260

@mdebbar mdebbar requested a review from nshahan September 23, 2025 15:20
@github-actions github-actions bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Sep 23, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes an assertion error that occurs during a hot restart on the web when an animation is in progress. The fix involves adding a disposal mechanism to the FrameService. Now, when a hot restart is triggered, the service instance is marked as disposed, and any pending animation frame callbacks are safely skipped. The change is accompanied by a new test case that correctly verifies this behavior. My review includes one suggestion to improve test hygiene by ensuring listeners are properly cleaned up between tests.

Copy link
Contributor

@nshahan nshahan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look at this!

// See:
// - https://github.com/flutter/flutter/issues/175260
// - https://github.com/flutter/flutter/issues/140684#issuecomment-3251179364
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some cleanup that should also be done so this FrameService can eventually be garbage collected or will it stay alive, silently skipping all frames from now on?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, what was happening before this PR is both generations kept pumping frames. The old generation is supposed to die out after the hot restart, but (unlike timers and microtasks) this callback/closure isn't cleaned up so it calls into the old generation code keeping it alive.

This return; is short-circuiting the code that's keeping the old generation alive.

I can't confirm everything I said, it's just my understanding based on observation and vague understanding. The one thing I confirmed is by adding a print right before the return;, that print is only executed once for each hot restart.

The code in _dispose also clears FrameService._instance forcing the next frame to create a new instance. The old FrameService instance becomes an orphan and is GC'd normally by the browser I presume.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding here is vague as well but that makes sense. Glad to hear we only hit that short circuiting return once. Thanks!

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Sep 24, 2025
Merged via the queue into flutter:master with commit 6c5df59 Sep 24, 2025
186 of 187 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2025
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
@mdebbar mdebbar deleted the infinite_raf_disposed branch September 24, 2025 20:18
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 25, 2025
Roll Flutter from 4a042046a0f1 to b1a28bc065b0 (44 revisions)

flutter/flutter@4a04204...b1a28bc

2025-09-25 48625061+muradhossin@users.noreply.github.com web_ui: avoid crash for showPerformanceOverlay; log 'not supported' once (flutter/flutter#173518)
2025-09-25 paulberry@google.com Ignore upcoming `experimental_member_use` warnings. (flutter/flutter#175969)
2025-09-25 engine-flutter-autoroll@skia.org Roll Skia from 753ce2221ce7 to 55436d87e414 (16 revisions) (flutter/flutter#176004)
2025-09-25 tirth@nevercode.io Add google_fonts to team-framework triage guidelines (flutter/flutter#175675)
2025-09-25 bruno.leroux@gmail.com Add tests for InputDecoration borders (M3 and theme normalization) (flutter/flutter#175838)
2025-09-24 737941+loic-sharma@users.noreply.github.com Update Flutter's templates to use dot shorthands (flutter/flutter#175891)
2025-09-24 matt.boetger@gmail.com In Gradle Flutter task, correctly replace '\ ' with ' '. (flutter/flutter#175815)
2025-09-24 47866232+chunhtai@users.noreply.github.com Cleans up navigator pop and remove logic (flutter/flutter#175612)
2025-09-24 rmolivares@renzo-olivares.dev Fix docs in `EditableText` (flutter/flutter#175787)
2025-09-24 47866232+chunhtai@users.noreply.github.com Fixes SemanticsFlags.isLink mis-translated in dart ui ffi (flutter/flutter#175812)
2025-09-24 1063596+reidbaker@users.noreply.github.com Update AGP/Java/Gradle comparison when using analyze --suggestions (flutter/flutter#175808)
2025-09-24 108678139+manu-sncf@users.noreply.github.com Fix SliverMainAxisGroup SliverEnsureSemantics support (flutter/flutter#175671)
2025-09-24 32538273+ValentinVignal@users.noreply.github.com Migrate to `WidgetStateColor` (flutter/flutter#175573)
2025-09-24 ahmedsameha1@gmail.com Make sure that a FlexibleSpaceBar doesn't crash in 0x0 environment (flutter/flutter#175228)
2025-09-24 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from BWj3yYC74ud58QhN0... to APSBP-sS-3FX69Ihf... (flutter/flutter#175944)
2025-09-24 ahmedsameha1@gmail.com Make sure that a MaterialApp doesn't crash in 0x0 environment (flutter/flutter#173090)
2025-09-24 36043466+koukibadr@users.noreply.github.com feat(cupertino): Add selectableDayPredicate parameter to CupertinoDatePicker for selectable day control #171332 (flutter/flutter#171334)
2025-09-24 mohellebiabdessalem@gmail.com Refactor `FlutterInjectorTest` to use lambdas/method reference (flutter/flutter#175777)
2025-09-24 mohellebiabdessalem@gmail.com Replace curly braces with lambdas in `KeyEventChannelTest` (flutter/flutter#175729)
2025-09-24 bkonyi@google.com [ Widget Preview ] Fix filter by file on Windows (flutter/flutter#175783)
2025-09-24 mohellebiabdessalem@gmail.com use lambda expressions /method reference to fix linter issue in `DartMessengerTest.java` (flutter/flutter#175733)
2025-09-24 engine-flutter-autoroll@skia.org Roll Packages from 3413b65 to 117bf63 (9 revisions) (flutter/flutter#175935)
2025-09-24 mohellebiabdessalem@gmail.com refactor code to use method reference and lambdas in `DartMessengerTest.java` (flutter/flutter#175731)
2025-09-24 mohellebiabdessalem@gmail.com Simplify/fix ordering of asserts in `TextInputPluginTest` (flutter/flutter#175784)
2025-09-24 mohellebiabdessalem@gmail.com Introduce a getter for `Project` to get `gradle-wrapper.properties` directly   (flutter/flutter#175485)
2025-09-24 mohellebiabdessalem@gmail.com Change the arguments order in `assertEquals` to fix linter issues (flutter/flutter#175719)
2025-09-24 42980667+srivats22@users.noreply.github.com Broken link in NavigationRail documentation (flutter/flutter#175852)
2025-09-24 mdebbar@google.com Updates to flutter web triage links (flutter/flutter#175791)
2025-09-24 jason-simmons@users.noreply.github.com Do not present textures in FlutterMetalLayer if the drawable size changed and the texture's size does not match the new drawable size (flutter/flutter#175450)
2025-09-24 bkonyi@google.com Remove comment about trailing commas from templates (flutter/flutter#175864)
2025-09-24 engine-flutter-autoroll@skia.org Roll Skia from 1c1b19f2ffc3 to 753ce2221ce7 (4 revisions) (flutter/flutter#175909)
2025-09-24 engine-flutter-autoroll@skia.org Roll Skia from 3191a822cf10 to 1c1b19f2ffc3 (2 revisions) (flutter/flutter#175896)
2025-09-24 engine-flutter-autoroll@skia.org Roll Skia from cabeab8cb22c to 3191a822cf10 (14 revisions) (flutter/flutter#175894)
2025-09-24 engine-flutter-autoroll@skia.org Roll Dart SDK from 14b4ced3022a to 899c7340cc4c (4 revisions) (flutter/flutter#175893)
2025-09-24 paulberry@google.com Roll `package:analyzer` forward to `8.2.0`. (flutter/flutter#175849)
2025-09-24 ahmedsameha1@gmail.com Make sure that a VerticalDivider doesn't crash at 0x0 environment (flutter/flutter#174761)
2025-09-24 ahmedsameha1@gmail.com Make sure that Drawer & DrawerHeader don't crash in 0x0 environment (flutter/flutter#174772)
2025-09-24 rmolivares@renzo-olivares.dev Add an assertion for the relationship between `Visibility.maintainState` and `Visibility.maintainFocusability` (flutter/flutter#175552)
2025-09-24 34465683+rkishan516@users.noreply.github.com fix: remove final class modifier on MenuController (flutter/flutter#174490)
2025-09-24 34465683+rkishan516@users.noreply.github.com fix: cupertino sheet broken example with programatic pop (flutter/flutter#175709)
2025-09-24 mdebbar@google.com [web] Fix assertion thrown when hot restarting during animation (flutter/flutter#175856)
2025-09-24 krca0220@gmail.com Add non uniform TableBorder (flutter/flutter#175773)
2025-09-23 mohellebiabdessalem@gmail.com fix small typo in test docs (flutter/flutter#175776)
2025-09-23 mohellebiabdessalem@gmail.com Use `assertNull` to simplify code (flutter/flutter#175720)

If this roll has caused a breakage, revert this CL and stop the roller
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels. platform-web Web applications specifically

Projects

None yet

2 participants