Skip to content

[windows] Fix null-deref in HostWindowPopup::UpdatePosition (Fixes #191478) - #191479

Merged
mattkae merged 1 commit into
flutter:masterfrom
kryali:fix/windows-popup-position-null-check
Aug 31, 2026
Merged

mattkae merged 1 commit into
flutter:masterfrom
kryali:fix/windows-popup-position-null-check

Conversation

@kryali

@kryali kryali commented Aug 21, 2026 •

Copy link
Copy Markdown
Contributor

Hi there,

I recently upgraded from flutter 3.44.9 to flutter 3.47.0 and noticed that the upgrade introduced a new crash on windows. This surfaced as a EXCEPTION_ACCESS_VIOLATION_READ crash in my application.

I was able to trace the issue to HostWindowPopup and have proposed a fix with this change. This fix is modeled after the existing code in host window tooltip.

  • Guard against a null WindowRect* from the position callback, matching HostWindowTooltip::UpdatePosition's existing check for the same call. Fixes a reproducible access-violation crash when the callback returns null.

Fixes Issue #191478

Pre-launch Checklist

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

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

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.

@kryali
kryali requested a review from a team as a code owner August 21, 2026 14:39
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically a: desktop Running on desktop team-windows Owned by the Windows platform team labels Aug 21, 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 introduces a null check for the window position rectangle in HostWindowPopup::UpdatePosition to prevent crashes when the positioner fails, and adds a corresponding unit test to verify this behavior. The review feedback suggests initializing the RECT structures and asserting the success of the GetWindowRect Win32 API calls in the new unit test to avoid using uninitialized memory and prevent test flakiness.

Comment thread engine/src/flutter/shell/platform/windows/window_manager_unittests.cc Outdated
Comment thread engine/src/flutter/shell/platform/windows/window_manager_unittests.cc Outdated

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

Thanks for the contribution! I think Gemini's suggestion is worthwhile, then I will approve once that's in

@kryali
kryali force-pushed the fix/windows-popup-position-null-check branch from 293aaa8 to a5d200d Compare August 26, 2026 18:29
@kryali

kryali commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@mattkae thanks for the review! I applied the suggested edits from gemini as requested.

@mattkae mattkae added the CICD Run CI/CD label Aug 26, 2026
mattkae
mattkae previously approved these changes Aug 26, 2026

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

Thanks for the contribution!

@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Aug 26, 2026
@mattkae mattkae added autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD labels Aug 26, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2026
@auto-submit

auto-submit Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/191479, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@mattkae mattkae added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2026
@auto-submit

auto-submit Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/191479, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2026
loic-sharma
loic-sharma previously approved these changes Aug 26, 2026

@loic-sharma loic-sharma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2026
@auto-submit

auto-submit Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/191479, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Aug 27, 2026
@mattkae mattkae added the CICD Run CI/CD label Aug 27, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 27, 2026
@auto-submit

auto-submit Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/191479, because - The status or check suite Mac_arm64_verify_binaries has failed. Please fix the issues identified (or deflake) before re-applying this label.

@kryali
kryali dismissed stale reviews from loic-sharma and mattkae via 41c679a August 28, 2026 16:17
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Aug 28, 2026
@kryali
kryali force-pushed the fix/windows-popup-position-null-check branch from 41c679a to 4532ad5 Compare August 28, 2026 16:20
Guard against a null WindowRect* from the position callback, matching
HostWindowTooltip::UpdatePosition's existing check for the same call.
@kryali
kryali force-pushed the fix/windows-popup-position-null-check branch from 4532ad5 to 9a57cbb Compare August 28, 2026 16:21
@mattkae mattkae added the CICD Run CI/CD label Aug 31, 2026
@mattkae
mattkae enabled auto-merge August 31, 2026 20:06
@mattkae
mattkae added this pull request to the merge queue Aug 31, 2026
Merged via the queue into flutter:master with commit a26f7ef Aug 31, 2026
24 checks passed
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Sep 1, 2026
flutter/flutter@c8c5e3b...0cbd1a4

2026-09-01 [email protected] Roll Skia from 6be7f8547c3c to 3911a1fe7f7a (1 revision) (flutter/flutter#192110)
2026-09-01 [email protected] Roll Fuchsia Linux SDK from EPzxifoyt36b5qiBy... to idslm9FikVLy2K_A-... (flutter/flutter#192104)
2026-09-01 [email protected] Roll Skia from 47df2ae3226c to 6be7f8547c3c (1 revision) (flutter/flutter#192103)
2026-09-01 [email protected] Roll Skia from e22ebf131e44 to 47df2ae3226c (4 revisions) (flutter/flutter#192099)
2026-09-01 [email protected] Roll Skia from 5489a16a5998 to e22ebf131e44 (5 revisions) (flutter/flutter#192093)
2026-09-01 [email protected] [flutter_tools] Safely handle broken symlinks and existing files during plugin symlink creation (flutter/flutter#191496)
2026-09-01 [email protected] Support wide gamut colors in gradient_generator's CreateGradientTexture (flutter/flutter#191980)
2026-09-01 [email protected] Clean up semantics code (flutter/flutter#191620)
2026-08-31 [email protected] Add website documentation item to PR pre-launch checklist (flutter/flutter#192080)
2026-08-31 [email protected] [windows] Fix null-deref in HostWindowPopup::UpdatePosition (Fixes #191478) (flutter/flutter#191479)
2026-08-31 [email protected] Removes deprecated ignoreSemantics parementers (flutter/flutter#191493)
2026-08-31 [email protected] Roll Dart SDK from b319095e317b to 9164def35347 (1 revision) (flutter/flutter#192073)
2026-08-31 [email protected] test: dynamic _tapOutside helper for TapRegion navigation tests (flutter/flutter#185397)
2026-08-31 [email protected] [Windows] fix: Remove quotes from compiler warning suppression (flutter/flutter#190873)
2026-08-31 [email protected] Migrate to listen package (flutter/flutter#189111)
2026-08-31 [email protected] Roll Skia from 15db98a90bbd to 5489a16a5998 (2 revisions) (flutter/flutter#192070)
2026-08-31 [email protected] Add regression test for plugin compileSdkExtension warning (flutter/flutter#191281)
2026-08-31 [email protected] Explicitly disable HCPP in platform view benchmarks and integration tests (flutter/flutter#191908)
2026-08-31 [email protected] Documentation and script for Gradle Distribution cache for CI (flutter/flutter#190323)
2026-08-31 [email protected] Roll Skia from 5549c93c9a1c to 15db98a90bbd (2 revisions) (flutter/flutter#192068)
2026-08-31 [email protected] Roll Packages from cd4cdd0 to d642322 (7 revisions) (flutter/flutter#192063)
2026-08-31 [email protected] Roll Dart SDK from 48f641e8b249 to b319095e317b (1 revision) (flutter/flutter#192061)
2026-08-31 [email protected] [tool] Migrate dev/tools from dart_skills_lint to skills_lint package (flutter/flutter#191997)

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] 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
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
…r#12712)

flutter/flutter@c8c5e3b...0cbd1a4

2026-09-01 [email protected] Roll Skia from 6be7f8547c3c to 3911a1fe7f7a (1 revision) (flutter/flutter#192110)
2026-09-01 [email protected] Roll Fuchsia Linux SDK from EPzxifoyt36b5qiBy... to idslm9FikVLy2K_A-... (flutter/flutter#192104)
2026-09-01 [email protected] Roll Skia from 47df2ae3226c to 6be7f8547c3c (1 revision) (flutter/flutter#192103)
2026-09-01 [email protected] Roll Skia from e22ebf131e44 to 47df2ae3226c (4 revisions) (flutter/flutter#192099)
2026-09-01 [email protected] Roll Skia from 5489a16a5998 to e22ebf131e44 (5 revisions) (flutter/flutter#192093)
2026-09-01 [email protected] [flutter_tools] Safely handle broken symlinks and existing files during plugin symlink creation (flutter/flutter#191496)
2026-09-01 [email protected] Support wide gamut colors in gradient_generator's CreateGradientTexture (flutter/flutter#191980)
2026-09-01 [email protected] Clean up semantics code (flutter/flutter#191620)
2026-08-31 [email protected] Add website documentation item to PR pre-launch checklist (flutter/flutter#192080)
2026-08-31 [email protected] [windows] Fix null-deref in HostWindowPopup::UpdatePosition (Fixes #191478) (flutter/flutter#191479)
2026-08-31 [email protected] Removes deprecated ignoreSemantics parementers (flutter/flutter#191493)
2026-08-31 [email protected] Roll Dart SDK from b319095e317b to 9164def35347 (1 revision) (flutter/flutter#192073)
2026-08-31 [email protected] test: dynamic _tapOutside helper for TapRegion navigation tests (flutter/flutter#185397)
2026-08-31 [email protected] [Windows] fix: Remove quotes from compiler warning suppression (flutter/flutter#190873)
2026-08-31 [email protected] Migrate to listen package (flutter/flutter#189111)
2026-08-31 [email protected] Roll Skia from 15db98a90bbd to 5489a16a5998 (2 revisions) (flutter/flutter#192070)
2026-08-31 [email protected] Add regression test for plugin compileSdkExtension warning (flutter/flutter#191281)
2026-08-31 [email protected] Explicitly disable HCPP in platform view benchmarks and integration tests (flutter/flutter#191908)
2026-08-31 [email protected] Documentation and script for Gradle Distribution cache for CI (flutter/flutter#190323)
2026-08-31 [email protected] Roll Skia from 5549c93c9a1c to 15db98a90bbd (2 revisions) (flutter/flutter#192068)
2026-08-31 [email protected] Roll Packages from cd4cdd0 to d642322 (7 revisions) (flutter/flutter#192063)
2026-08-31 [email protected] Roll Dart SDK from 48f641e8b249 to b319095e317b (1 revision) (flutter/flutter#192061)
2026-08-31 [email protected] [tool] Migrate dev/tools from dart_skills_lint to skills_lint package (flutter/flutter#191997)

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] 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
victorsanni pushed a commit to victorsanni/packages that referenced this pull request Sep 9, 2026
…r#12712)

flutter/flutter@c8c5e3b...0cbd1a4

2026-09-01 [email protected] Roll Skia from 6be7f8547c3c to 3911a1fe7f7a (1 revision) (flutter/flutter#192110)
2026-09-01 [email protected] Roll Fuchsia Linux SDK from EPzxifoyt36b5qiBy... to idslm9FikVLy2K_A-... (flutter/flutter#192104)
2026-09-01 [email protected] Roll Skia from 47df2ae3226c to 6be7f8547c3c (1 revision) (flutter/flutter#192103)
2026-09-01 [email protected] Roll Skia from e22ebf131e44 to 47df2ae3226c (4 revisions) (flutter/flutter#192099)
2026-09-01 [email protected] Roll Skia from 5489a16a5998 to e22ebf131e44 (5 revisions) (flutter/flutter#192093)
2026-09-01 [email protected] [flutter_tools] Safely handle broken symlinks and existing files during plugin symlink creation (flutter/flutter#191496)
2026-09-01 [email protected] Support wide gamut colors in gradient_generator's CreateGradientTexture (flutter/flutter#191980)
2026-09-01 [email protected] Clean up semantics code (flutter/flutter#191620)
2026-08-31 [email protected] Add website documentation item to PR pre-launch checklist (flutter/flutter#192080)
2026-08-31 [email protected] [windows] Fix null-deref in HostWindowPopup::UpdatePosition (Fixes #191478) (flutter/flutter#191479)
2026-08-31 [email protected] Removes deprecated ignoreSemantics parementers (flutter/flutter#191493)
2026-08-31 [email protected] Roll Dart SDK from b319095e317b to 9164def35347 (1 revision) (flutter/flutter#192073)
2026-08-31 [email protected] test: dynamic _tapOutside helper for TapRegion navigation tests (flutter/flutter#185397)
2026-08-31 [email protected] [Windows] fix: Remove quotes from compiler warning suppression (flutter/flutter#190873)
2026-08-31 [email protected] Migrate to listen package (flutter/flutter#189111)
2026-08-31 [email protected] Roll Skia from 15db98a90bbd to 5489a16a5998 (2 revisions) (flutter/flutter#192070)
2026-08-31 [email protected] Add regression test for plugin compileSdkExtension warning (flutter/flutter#191281)
2026-08-31 [email protected] Explicitly disable HCPP in platform view benchmarks and integration tests (flutter/flutter#191908)
2026-08-31 [email protected] Documentation and script for Gradle Distribution cache for CI (flutter/flutter#190323)
2026-08-31 [email protected] Roll Skia from 5549c93c9a1c to 15db98a90bbd (2 revisions) (flutter/flutter#192068)
2026-08-31 [email protected] Roll Packages from cd4cdd0 to d642322 (7 revisions) (flutter/flutter#192063)
2026-08-31 [email protected] Roll Dart SDK from 48f641e8b249 to b319095e317b (1 revision) (flutter/flutter#192061)
2026-08-31 [email protected] [tool] Migrate dev/tools from dart_skills_lint to skills_lint package (flutter/flutter#191997)

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] 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

a: desktop Running on desktop CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically team-windows Owned by the Windows platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants