Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revoke HandleAcceleratorKeyActivated when a WebView2 is closed #7117

Merged
merged 7 commits into from
May 26, 2022

Conversation

krschau
Copy link
Contributor

@krschau krschau commented May 17, 2022

Description

When a WebView2 is closed, we no longer need to forward tab messages to the CoreWebView2. However, the event handler will still try even when the WebView2 is closed if it has not been destroyed, since the auto-revoker won't have kicked in. We should explicitly revoke this handler when Close() is called on the WebView2. Once the webview has been closed, when we get a KeyDown event, we should no longer mark it handled, and instead let normal Xaml tab handling happen.

I also moved some code so some registering and revoking was closer together, and in matching order. (First commit)

Motivation and Context

Without this change, a closed WebView2 will still try to forward the tab to a no-longer-existing hwnd owned by the CoreWebView2, and the app will crash.

How Has This Been Tested?

Added LifetimeStatesTabTest, which tabs past the webview 1) before a core webview is created, 2) after a core webview is created but before any navigation, and 3) after the webview has been closed. I've also verified manually.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label May 17, 2022
@krschau krschau added area-WebView team-Rendering Issue for the Rendering team and removed needs-triage Issue needs to be triaged by the area owners labels May 18, 2022
@krschau
Copy link
Contributor Author

krschau commented May 18, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@DmitriyKomin DmitriyKomin left a comment

Choose a reason for hiding this comment

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

🕐

@krschau
Copy link
Contributor Author

krschau commented May 19, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@krschau krschau requested a review from DmitriyKomin May 19, 2022 22:12
Copy link
Contributor

@DmitriyKomin DmitriyKomin left a comment

Choose a reason for hiding this comment

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

:shipit:

@krschau
Copy link
Contributor Author

krschau commented May 24, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@krschau
Copy link
Contributor Author

krschau commented May 24, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@krschau
Copy link
Contributor Author

krschau commented May 25, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@krschau
Copy link
Contributor Author

krschau commented May 25, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@krschau krschau merged commit 0b6e07d into main May 26, 2022
@krschau krschau deleted the user/krschau/revoke-HandleAcceleratorKeyActivated branch May 26, 2022 15:11
@ghost
Copy link

ghost commented Jun 2, 2022

🎉Microsoft.UI.Xaml v2.8.0-prerelease.220601001 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-WebView team-Rendering Issue for the Rendering team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants