Skip to content

fix(web): call ui.Picture.onDispose for the original picture only#184348

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
harryterkelsen:fix-picture-dispose-reporting
Mar 30, 2026
Merged

fix(web): call ui.Picture.onDispose for the original picture only#184348
auto-submit[bot] merged 1 commit into
flutter:masterfrom
harryterkelsen:fix-picture-dispose-reporting

Conversation

@harryterkelsen

Copy link
Copy Markdown
Contributor

This reverts a recent change where ui.Picture.onDispose is only called when the original picture and all clones have been disposed. This change broke leak tracker tests.

Fixes #184312

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

This reverts a recent change where ui.Picture.onDispose is only called
when the original picture and all clones have been disposed. This change
broke leak tracker tests.
@harryterkelsen harryterkelsen requested a review from mdebbar March 30, 2026 17:44
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Mar 30, 2026

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

Copy link
Copy Markdown
Contributor

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 modifies the CkPicture and SkwasmPicture classes to track whether an instance is a clone using a new _isClone field. This change ensures that the ui.Picture.onDispose callback is triggered only by the original picture instance during disposal, rather than by clones or the internal reference counter. Corresponding tests were updated to verify that the original picture triggers the disposal callback regardless of live clones. Feedback was provided regarding the placement of the _isClone field in SkwasmPicture to ensure consistency with CkPicture and adhere to style guidelines for logical member ordering.

_init();
}

final bool _isClone;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For consistency and improved readability, it would be better to group all member fields together. In the CkPicture class, which is also modified in this PR, all fields are declared together after the constructors and before any methods. Here, the _isClone field is declared, followed by the _init method, with other fields presumably declared elsewhere. Grouping all fields in one place makes the class structure easier to understand at a glance.

References
  1. The style guide suggests that class members should be ordered logically, for example by grouping related fields and methods. Grouping all fields together improves readability and maintainability. (link)

@mdebbar mdebbar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@harryterkelsen harryterkelsen added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Mar 30, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Mar 30, 2026
Merged via the queue into flutter:master with commit dcc3ab2 Mar 30, 2026
187 of 188 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 30, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Apr 2, 2026
Roll Flutter from fb03253e32ce to 3d69471c0bf9 (69 revisions)

flutter/flutter@fb03253...3d69471

2026-04-01 [email protected] Roll Dart SDK from 1308a3076402 to 043a2bfd56ff (1 revision) (flutter/flutter#184453)
2026-04-01 [email protected] Roll Skia from a657b5446209 to c2363c39c283 (2 revisions) (flutter/flutter#184448)
2026-04-01 [email protected] Fix layout overflowed in small screen in SensitiveContent's example (flutter/flutter#184179)
2026-04-01 [email protected] Roll Skia from e0b25041a5d5 to a657b5446209 (1 revision) (flutter/flutter#184445)
2026-04-01 [email protected] Roll Dart SDK from 75a089eb6bf9 to 1308a3076402 (3 revisions) (flutter/flutter#184444)
2026-04-01 [email protected] [AGP 9] Bumping KGP error minimum to 2.0.0 (flutter/flutter#184385)
2026-04-01 [email protected] [ Tool ] Migrate `flutter analyze` to use LSP (flutter/flutter#183785)
2026-04-01 [email protected] Adds uber sdf shader gradients with blend (flutter/flutter#184090)
2026-04-01 [email protected] Add bottom safe area padding to licenses package license page (flutter/flutter#182425)
2026-04-01 [email protected] Handle#6537 third grouped tests (flutter/flutter#183059)
2026-04-01 [email protected] Roll Skia from f37239a7a689 to e0b25041a5d5 (9 revisions) (flutter/flutter#184436)
2026-03-31 [email protected] [Impeller] Do not log an error when wrapping an empty texture as a TextureGLES (flutter/flutter#184377)
2026-03-31 [email protected] Remove the default_git_folder GN argument (flutter/flutter#184152)
2026-03-31 [email protected] Remove the cupertino_icons dependency from the spell_check integration test (flutter/flutter#184398)
2026-03-31 [email protected] Roll Fuchsia Linux SDK from pM94cWC9cSgao0CG0... to fV-JIWUt4FQGeDtEe... (flutter/flutter#184383)
2026-03-31 [email protected] Roll Dart SDK from eaeccf98848d to 75a089eb6bf9 (1 revision) (flutter/flutter#184379)
2026-03-31 [email protected] [web] Fix autofill in iOS 26 Safari (flutter/flutter#182024)
2026-03-31 [email protected] Roll Fuchsia GN SDK from SEfYx3xgueX3aFAY3... to JLBh4Z9PKsjIJcqDU... (flutter/flutter#184368)
2026-03-31 [email protected] [Windows] Restore and enable IAccessibleEx implementation (flutter/flutter#175406)
2026-03-31 [email protected] Revert "Even more awaits (#184042)" (flutter/flutter#184429)
2026-03-31 [email protected] Roll Skia from dfd8f8002800 to f37239a7a689 (2 revisions) (flutter/flutter#184374)
2026-03-31 [email protected] Remove workaround for fake impeller images in iOS simulator. (flutter/flutter#184264)
2026-03-31 [email protected] Even more awaits (flutter/flutter#184042)
2026-03-31 [email protected] Roll Packages from 582f0e7 to b04f3e5 (6 revisions) (flutter/flutter#184393)
2026-03-30 [email protected] Fixes a flake in reload shaders tests (flutter/flutter#184268)
2026-03-30 [email protected] Remove an obsolete script for setting up remote GDB sessions on Android devices (flutter/flutter#184357)
2026-03-30 [email protected] Roll Skia from 8dcde79fef2a to dfd8f8002800 (10 revisions) (flutter/flutter#184363)
2026-03-30 [email protected] Roll Dart SDK from 0aaccc3c8004 to eaeccf98848d (2 revisions) (flutter/flutter#184362)
2026-03-30 [email protected] [ Tool ] Remove `flutter running-apps` command (flutter/flutter#183742)
2026-03-30 [email protected] Roll pub packages (flutter/flutter#184045)
2026-03-30 [email protected] Rick roll triagers on/near April 1st (flutter/flutter#184355)
2026-03-30 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 5.5.0 to 6.0.0 in the all-github-actions group (flutter/flutter#184364)
2026-03-30 [email protected] fix(web): call ui.Picture.onDispose for the original picture only (flutter/flutter#184348)
2026-03-30 [email protected] Roll Skia from e001e6901e3b to 8dcde79fef2a (7 revisions) (flutter/flutter#184356)
2026-03-30 [email protected] [web] Make it safe to call dispose multiple times on a CkSurface (flutter/flutter#184270)
2026-03-30 [email protected] Roll HarfBuzz to 13.2.1 (flutter/flutter#184210)
2026-03-30 [email protected] web_ui: Remove unused parameters in a few places (flutter/flutter#183156)
2026-03-30 [email protected] Update TabBar documentation to clarify indicatorWeight behavior (flutter/flutter#184104)
2026-03-30 [email protected] Add title evaluation (flutter/flutter#184084)
2026-03-30 [email protected] fixes crash when invisible semantics nodes dropped from semantics tree (flutter/flutter#184226)
2026-03-30 [email protected] Roll Skia from cdaae3e3fdef to e001e6901e3b (4 revisions) (flutter/flutter#184345)
2026-03-30 [email protected] Roll Packages from 7ae082a to 582f0e7 (8 revisions) (flutter/flutter#184341)
2026-03-30 [email protected] Add alwaysSizeToContent argument to Overlay. (flutter/flutter#182009)
2026-03-30 [email protected] Roll Dart SDK from 598088a8a67f to 0aaccc3c8004 (1 revision) (flutter/flutter#184331)
2026-03-30 [email protected] Roll Skia from 6d7ade938643 to cdaae3e3fdef (2 revisions) (flutter/flutter#184329)
2026-03-30 [email protected] Roll Fuchsia Linux SDK from EnoD6zNQebz4EYuLk... to pM94cWC9cSgao0CG0... (flutter/flutter#184323)
...
ahmedsameha1 pushed a commit to ahmedsameha1/flutter that referenced this pull request Apr 14, 2026
…utter#184348)

This reverts a recent change where ui.Picture.onDispose is only called
when the original picture and all clones have been disposed. This change
broke leak tracker tests.

Fixes flutter#184312

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter/flutter->flutter/packages roll failing unit test due to leaks

2 participants