API Review: ProcessId for ProcessFailedEventArgs #5401
Conversation
Add ProcessId property to CoreWebView2ProcessFailedEventArgs4 This API extends ProcessFailedEventArgs to include the process ID of the failed process, enabling applications to: - Correlate process failures with running process data from the ProcessInfo API - Collect process-specific diagnostic information for logging and telemetry - Analyze crash dumps for specific processes - Better track and respond to failures in multi-renderer scenarios The process ID is only available if the process has started and ended unexpectedly. For other failure types, the process ID value will be 0.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a ProcessId property to CoreWebView2ProcessFailedEventArgs to expose the process ID of failed WebView2 processes. This enhancement enables applications to correlate process failures with runtime process information, collect process-specific diagnostics, and better handle failures in multi-renderer scenarios.
Key Changes:
- Added
ICoreWebView2ProcessFailedEventArgs4interface withProcessIdproperty - The process ID returns 0 for failure types where the process never started successfully
- Provides correlation capability with existing ProcessInfo API
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@microsoft-github-policy-service agree company="Microsoft" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| process starts successfully (for example, GPU process hangs, browser process | ||
| exits, utility process exits, renderer process hangs), the process ID is | ||
| available so apps can correlate diagnostics. If the process never starts or if | ||
| the main frame renderer process is terminated externally (for example, by Task | ||
| Manager or taskkill) the associated process information is unavailable and the | ||
| reported process ID is 0. |
There was a problem hiding this comment.
The Remarks section describes when "the process ID is available" but doesn't clearly state what happens when it's not available. Based on line 163, it mentions "the reported process ID is 0", but the relationship between ProcessInfo being unavailable versus ProcessId being 0 is unclear. The documentation should clarify whether ProcessInfo is null in these cases or if ProcessInfo exists but ProcessId is 0.
| /// The process info of the failed process, which can be used to | ||
| /// correlate the failing process with the running process data or to | ||
| /// analyze crash dumps for that process. The process ID is available when the | ||
| /// process starts successfully (GPU process hangs, browser process exits, | ||
| /// utility process exits, renderer process hangs). If the process never | ||
| /// started or when the main frame renderer process is terminated externally | ||
| /// (for example by Task Manager or taskkill), the process ID will be set to 0. |
There was a problem hiding this comment.
The API documentation comment states "The process info of the failed process" but doesn't clearly document that this property can return null in certain scenarios. Based on the C# example (lines 130-138) and the remarks, ProcessInfo can be unavailable. The COM API should explicitly document the nullability of this return value and under what conditions it returns null versus a valid object with ProcessId set to 0.
specs/api-get_ProcessId.md
Outdated
| /// process starts successfully (GPU process hangs, browser process exits, | ||
| /// utility process exits, renderer process hangs). If the process never | ||
| /// started or when the main frame renderer process is terminated externally | ||
| /// (for example by Task Manager or taskkill), the process ID will be set to 0. |
There was a problem hiding this comment.
The documentation has inconsistent punctuation. Lines 184 and 204 end with a period while they are part of longer explanatory text, but there's no period at the end of line 163 in the Remarks section which has similar content. Documentation comments should have consistent punctuation.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add ProcessId property to CoreWebView2ProcessFailedEventArgs4
This API extends ProcessFailedEventArgs to include the process ID of the failed process, enabling applications to:
The process ID is only available if the process has started and ended unexpectedly. For other failure types, the process ID value will be 0.