Fix WebView2 crash on close by implementing proper disposal pattern

[Copilot]

This PR fixes a crash that occurs when closing HTML views in PerfView. The crash was happening during finalization of the WebView2 control, which involves COM interop cleanup that can fail when done through the finalizer.

Problem

The crash stack trace shows:

Microsoft_Web_WebView2_Wpf!Microsoft.Web.WebView2.Wpf.WebView2.Dispose+0x34
PresentationFramework_ni!System.Windows.Interop.HwndHost.Finalize+0x14
clr!FinalizerThread::DoOneFinalization+0x8f

This indicates the WebView2 control was being cleaned up by the finalizer instead of being explicitly disposed, leading to COM interop crashes.

Solution

Implemented proper IDisposable pattern for WebBrowserWindow:

1. Made WebBrowserWindow implement IDisposable with standard dispose pattern
2. Added explicit disposal in Window_Closing when the window is actually closing (not just hiding)
3. Added safety checks throughout the class to prevent operations on disposed WebView2 controls
4. Protected async initialization from disposal race conditions

Key Changes

• WebBrowserWindow now implements IDisposable with proper dispose pattern
• Window_Closing event handler calls Dispose() to clean up WebView2 before window destruction
• All WebView2 operations check _disposed flag to prevent post-disposal usage
• Async Browser_Loaded method includes disposal checks to avoid unnecessary work

Testing

Created and validated a mock test that confirms:

• WebView2 disposal is called correctly
• Post-disposal operations are safely handled
• Double disposal is safe
• Async operations respect disposal state

The implementation follows PerfView coding standards and uses minimal, surgical changes to fix the crash without affecting existing functionality.

Fixes #2229.

[brianrob]

Copilot Post-Mortem:

• Great example of an issue that requires minimal coaching.
• The stack trace was sufficient for copilot to know what to do. All I had to do was test locally.

[marklio]

I approve, but consider the feedback on going to the trouble of implementing the full verbose IDisposable pattern.

[marklio]

Sort of a NIT: Since you're not asking consumers to manage the lifetime of this window, I wouldn't implement IDisposable. The pattern makes sense as an implementation, but I wouldn't expose it (the comment in the Closing() event handler is evidence of this weirdness. The window manages its disposable resources within the "control lifecycle". I'd get rid of the extra Dispose pattern goo and just have the _disposed bool to control operating on _Browser and call _Browser.Dispose conditionally in the Window_Closing

[Copilot]

Good point! I've simplified the implementation by removing the IDisposable interface and formal dispose pattern. The solution now just uses an internal _disposed bool and calls _Browser.Dispose() conditionally in Window_Closing when the window is actually being closed. This is much cleaner since the window manages its own lifecycle. (8e3e67e)

[Copilot]

I approve, but consider the feedback on going to the trouble of implementing the full verbose IDisposable pattern.

Thanks for the feedback! I've addressed the suggestion by removing the verbose IDisposable pattern and implementing a simpler internal disposal approach that still fixes the crash. (8e3e67e)

[cincuranet]

This is basically what CanGoBack property above is doing and hence it can be used here.

[Copilot]

You're absolutely right! Changed to use the CanGoBack property which already encapsulates the disposal check logic. (d692454)

[cincuranet]

This is basically what CanGoForward property above is doing and hence it can be used here.

[Copilot]

Correct! Updated to use the CanGoForward property to avoid duplicating the disposal check logic. (d692454)

[cincuranet]

I asked for a change (already done by Copilot). PTAL @marklio.

[brianrob]

I asked for a change (already done by Copilot). PTAL @marklio.

Good call @cincuranet.

[cincuranet]

@leculver @mconnew PTAL. The rest of the people are not good enough reviewers in current state. :D

[brianrob]

Additional Copilot Post-Mortem:

It looks like once a maintainer makes a comment that causes copilot to respond with a code change, that maintainer's approval no longer counts. That looks like it's going to be a bit too strong of a limitation, given that we've now had two additional maintainers, in addition to the originator of the PR approve, and that's still not enough. It feels like there is a sweet spot here where the originator of the PR needs to unleash copilot to do the work so that the additional maintainers' approval still counts (or something along those lines).

[brianrob]

It seems like the current behavior for approvals is this:

• The originator of the PR (the person that assigns the issue to copilot) can never approve.
• If another maintainer makes a comment and copilot makes a change as a result, that maintainer cannot approve.
• However, if yet another maintainer makes a comment that copilot reacts to, then the first maintainer (not the originator) can approve and it will count. However, old approvals won't count because they are invalidated when copilot responds to a comment that they made.

We'll see if this guidance sticks, but for now, that seems to be the case. I'm merging this before copilot makes another change. :)