Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Jan 22, 2024

#49872 introduced a crash in debug mode if the platform thread starts a window resize in between OnFrameGenerated and OnFramePresented.

Fixes flutter/flutter#141855.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

@flutter flutter deleted a comment from flutter-dashboard bot Jan 22, 2024
@loic-sharma loic-sharma marked this pull request as ready for review January 22, 2024 18:06
// Called by |FlutterWindow| on the platform thread.
//
// Returns true if the delegate completed the window resize synchronously.
// The return value should only be used for unit testing.
Copy link
Member

Choose a reason for hiding this comment

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

Not that I can think of a great use for this but any reason we shouldn't use it for anything other than unit tests? If so, consider documenting why that's a bad idea if it can be stated succinctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this comment in case someone wonders why we discard the return value in the implementation. The implementation could use this if needed. I'll tweak the comment to make this clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the comment!

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 22, 2024
@auto-submit auto-submit bot merged commit d87e742 into flutter:main Jan 22, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2024
@loic-sharma loic-sharma deleted the windows_resize_crash branch January 22, 2024 20:48
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 22, 2024
…141993)

flutter/engine@b2762f4...b069d7f

2024-01-22 [email protected] Roll Skia from cd6b0ff3596e to bbb0d6feaf1c (1 revision) (flutter/engine#49943)
2024-01-22 [email protected] [Impeller] fixed CanRenderClippedRuntimeEffects for vulkan (flutter/engine#49912)
2024-01-22 [email protected] [Windows] Fix resize crash (flutter/engine#49935)
2024-01-22 [email protected] Roll Skia from be066a6524ab to cd6b0ff3596e (3 revisions) (flutter/engine#49939)

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

To file a bug in Flutter: 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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

affects: desktop autosubmit Merge PR when tree becomes green via auto submit App platform-windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ERROR:flutter/shell/platform/windows/flutter_windows_view.cc(603)] Reached unreachable code.

3 participants