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

Add Windows exit requests and responses #40400

Merged
yaakovschectman merged 7 commits into from
Mar 21, 2023
Merged

Add Windows exit requests and responses #40400

yaakovschectman merged 7 commits into from
Mar 21, 2023

Conversation

@yaakovschectman
Copy link
Contributor

@yaakovschectman yaakovschectman commented Mar 17, 2023

Adds support for program exit request for Windows. This does not allow canceling closing the application by closing the window, yet.

Addresses windows portion of flutter/flutter#30735

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 Hixie said 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.

@yaakovschectman yaakovschectman self-assigned this Mar 17, 2023
@yaakovschectman
Copy link
Contributor Author

yaakovschectman commented Mar 17, 2023

Currently, when the application exits through PostQuitMessage, [ERROR:flutter/shell/gpu/gpu_surface_gl_skia.cc(97)] Could not make the context current to destroy the GrDirectContext resources. is reported on the shell.

@yaakovschectman yaakovschectman marked this pull request as ready for review March 17, 2023 21:36
@yaakovschectman yaakovschectman changed the title Windows exit Add Window exit requests and responses Mar 17, 2023
@yaakovschectman yaakovschectman changed the title Add Window exit requests and responses Add Windows exit requests and responses Mar 17, 2023
gspencergoog
gspencergoog reviewed Mar 17, 2023
View reviewed changes
gspencergoog
gspencergoog approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

@yaakovschectman yaakovschectman merged commit 8325c2c into flutter:main Mar 21, 2023
@yaakovschectman yaakovschectman deleted the windows_exit branch March 21, 2023 12:48
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 21, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023
@engine-flutter-autoroll
8325c2c Add Windows exit requests and responses (flutter/engine#40400)
aad6e34
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 21, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023
@engine-flutter-autoroll
8325c2c Add Windows exit requests and responses (flutter/engine#40400)
bcddae7
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 21, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023
@engine-flutter-autoroll
8325c2c Add Windows exit requests and responses (flutter/engine#40400)
4bad8da
stuartmorgan-g
stuartmorgan-g reviewed Mar 21, 2023
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, but some post-landing review comments.

This does not allow canceling closing the application by closing the window, yet.

Addresses windows portion of flutter/flutter#30735

Sounds like this only addresses part of the Windows portion of that issue then?

@yaakovschectman
Copy link
Contributor Author

yaakovschectman commented Mar 21, 2023

Sounds like this only addresses part of the Windows portion of that issue then?

Correct, this PR only addresses part of the windows portion of the issue. I am currently putting together a PR to address the missing portion.

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 21, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023
@engine-flutter-autoroll
8325c2c Add Windows exit requests and responses (flutter/engine#40400)
16a4614
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 21, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023
@engine-flutter-autoroll
8325c2c Add Windows exit requests and responses (flutter/engine#40400)
0d9b9c2
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 21, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023
@engine-flutter-autoroll
8325c2c Add Windows exit requests and responses (flutter/engine#40400)
737048b
@HansMuller HansMuller mentioned this pull request Mar 24, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cbracken cbracken Awaiting requested review from cbracken

@loic-sharma loic-sharma Awaiting requested review from loic-sharma

2 more reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g left review comments

@gspencergoog gspencergoog gspencergoog approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@yaakovschectman yaakovschectman

Labels

affects: desktop platform-windows

Projects

Flutter Desktop
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yaakovschectman @stuartmorgan-g @gspencergoog