Skip to content

Make sure that a Banner doesn't crash in 0x0 environment#180254

Merged
auto-submit[bot] merged 4 commits into
flutter:masterfrom
ahmedsameha1:handle#6537-Banner
Jan 13, 2026
Merged

Make sure that a Banner doesn't crash in 0x0 environment#180254
auto-submit[bot] merged 4 commits into
flutter:masterfrom
ahmedsameha1:handle#6537-Banner

Conversation

@ahmedsameha1

Copy link
Copy Markdown
Contributor

This is my attempt to handle #6537 for the Banner widget.

@github-actions github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Dec 24, 2025

@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 adds a regression test to ensure the Banner widget does not crash when rendered in a zero-sized area. The test case is well-written and covers the intended scenario.

While reviewing this change, I also looked at the implementation of the Banner widget in packages/flutter/lib/src/widgets/banner.dart and found a couple of issues that could be addressed in a follow-up change:

  1. Painter Lifecycle: In _BannerState, the BannerPainter is recreated and disposed on every build call. This is inefficient and can lead to a race condition where a painter is used after being disposed. A safer approach would be to delay the disposal of the old painter until the next frame.

  2. Incomplete shouldRepaint: The BannerPainter.shouldRepaint method does not check for changes in textDirection, layoutDirection, or shadow. This can lead to incorrect rendering if these properties change.

These are separate issues from the test being added, but are relevant to the overall quality of the Banner widget.

@dkwingsmt dkwingsmt 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.

Checklist:

  • The test is in the correct file
  • The test name goes “does not crash at zero area”
  • The target widget is wrapped by Center (or is fullscreen)
  • The target widget does not have an overlay, or the overlay is tested
  • The target widget is expected to have a size of exactly Size.zero

@justinmc justinmc 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 👍

@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 30, 2025
@auto-submit
auto-submit Bot added this pull request to the merge queue Dec 30, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Dec 30, 2025
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 30, 2025
@dkwingsmt

Copy link
Copy Markdown
Contributor

@ahmedsameha1 Can you resolve the merge conflict? (kinda hard to do on web)

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
@auto-submit

auto-submit Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/180254, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Jan 13, 2026
Merged via the queue into flutter:master with commit beb57ad Jan 13, 2026
71 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 15, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 15, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 15, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jan 15, 2026
Manual roll requested by [email protected]

flutter/flutter@793b0b8...b45a73b

2026-01-13 [email protected] Roll pub packages (flutter/flutter#180900)
2026-01-13 [email protected] Roll Dart SDK from 34318de9874b to ebaf52c13799 (1 revision) (flutter/flutter#180870)
2026-01-13 [email protected] Make sure that an ErrorWidget doesn't crash in 0x0 environment (flutter/flutter#180830)
2026-01-13 [email protected] Make sure that a Banner doesn't crash in 0x0 environment (flutter/flutter#180254)
2026-01-13 [email protected] Adds metal background to engine dart tests (flutter/flutter#180700)
2026-01-13 [email protected] Add TabBar API example for scroll notification integration (flutter/flutter#180728)
2026-01-13 [email protected] Roll Skia from 714d0af2eda7 to 8f3134206e8d (1 revision) (flutter/flutter#180874)
2026-01-13 [email protected] Add build-time checks for ImpellerC's SkSL compiler (flutter/flutter#180861)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
ikramhasan pushed a commit to ikramhasan/flutter that referenced this pull request Jan 15, 2026
)

This is my attempt to handle
flutter#6537 for the Banner widget.

Co-authored-by: Tong Mu <[email protected]>
ivan-vanyusho pushed a commit to ivan-vanyusho/packages that referenced this pull request Jan 26, 2026
)

Manual roll requested by [email protected]

flutter/flutter@793b0b8...b45a73b

2026-01-13 [email protected] Roll pub packages (flutter/flutter#180900)
2026-01-13 [email protected] Roll Dart SDK from 34318de9874b to ebaf52c13799 (1 revision) (flutter/flutter#180870)
2026-01-13 [email protected] Make sure that an ErrorWidget doesn't crash in 0x0 environment (flutter/flutter#180830)
2026-01-13 [email protected] Make sure that a Banner doesn't crash in 0x0 environment (flutter/flutter#180254)
2026-01-13 [email protected] Adds metal background to engine dart tests (flutter/flutter#180700)
2026-01-13 [email protected] Add TabBar API example for scroll notification integration (flutter/flutter#180728)
2026-01-13 [email protected] Roll Skia from 714d0af2eda7 to 8f3134206e8d (1 revision) (flutter/flutter#180874)
2026-01-13 [email protected] Add build-time checks for ImpellerC's SkSL compiler (flutter/flutter#180861)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
)

Manual roll requested by [email protected]

flutter/flutter@793b0b8...b45a73b

2026-01-13 [email protected] Roll pub packages (flutter/flutter#180900)
2026-01-13 [email protected] Roll Dart SDK from 34318de9874b to ebaf52c13799 (1 revision) (flutter/flutter#180870)
2026-01-13 [email protected] Make sure that an ErrorWidget doesn't crash in 0x0 environment (flutter/flutter#180830)
2026-01-13 [email protected] Make sure that a Banner doesn't crash in 0x0 environment (flutter/flutter#180254)
2026-01-13 [email protected] Adds metal background to engine dart tests (flutter/flutter#180700)
2026-01-13 [email protected] Add TabBar API example for scroll notification integration (flutter/flutter#180728)
2026-01-13 [email protected] Roll Skia from 714d0af2eda7 to 8f3134206e8d (1 revision) (flutter/flutter#180874)
2026-01-13 [email protected] Add build-time checks for ImpellerC's SkSL compiler (flutter/flutter#180861)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants