You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request adds support for starting and stopping screencasts in the BiDi BrowsingContext module, including new API methods, supporting types, and a test. The main changes introduce the ability to programmatically record the browser context and manage screencast sessions.
New screencast functionality:
Added StartScreencastAsync and StopScreencastAsync methods to the IBrowsingContextModule interface and implemented them in BrowsingContextModule, enabling clients to start and stop screencasts on a browsing context. [1][2]
Added StartScreencastOptions, StartScreencastResult, and related parameter/result types in the new StartScreencast.cs file, and similarly for stopping screencasts in StopScreencast.cs. These types define the options and results for screencast commands. [1][2]
Introduced the Screencast record type, representing an active screencast session with a StopAsync method to end the session.
Integration and serialization:
Registered the new screencast parameter and result types with the JSON serializer to ensure correct serialization/deserialization of screencast commands.
Testing:
Added a new test, CanStartAndStopScreencast, to verify that screencast sessions can be started and stopped, and that the output file is managed correctly. The test currently ignores Chrome, Edge, and Firefox as they do not support this feature yet. [1][2]
🔧 Implementation Notes
Following existing pattern.
🔄 Types of changes
New feature (non-breaking change which adds functionality and tests!)
• Add BiDi commands and .NET APIs to start/stop browsing-context screencasts.
• Introduce strongly-typed screencast handle plus options/results for session control.
• Add an integration test for start/stop (ignored on Chrome/Edge pending support).
Diagram
graph TD
A(["Client code"]) --> B(["BrowsingContext API"]) --> C[["BrowsingContextModule"]] --> D(["BiDi ExecuteAsync"]) --> E{{"Browser (BiDi)"}}
B --> F[("Screencast handle")]
F --> C
subgraph Legend
direction LR
_api(["Public API"]) ~~~ _mod[["Module/Command"]] ~~~ _db[("Handle/ID")] ~~~ _ext{{"External"}}
end
Loading
High-Level Assessment
The following are alternative approaches to this PR:
1. Use string-based screencast id instead of Screencast type
➕ Less public surface area and fewer types
➕ Avoids carrying IBiDi reference inside the handle
➖ Worse ergonomics (callers must thread ids around)
➖ Easier to misuse across sessions; less type safety
2. Expose StopScreencastAsync on BrowsingContext as a convenience overload
➕ More discoverable stop API (symmetry with StartScreencastAsync)
➕ Callers don’t need to learn Screencast.StopAsync pattern
➖ Adds additional public overloads to maintain
➖ Encourages using context even when handle-centric API is cleaner
Recommendation: The chosen approach (strongly-typed Screencast handle with StopAsync plus module-level stop command) matches existing BiDi patterns and provides good ergonomics/type-safety. Consider optionally adding a BrowsingContext.StopScreencastAsync(Screencast) convenience overload if API discoverability becomes a concern, but it’s not required for correctness.
Files changed (7) +183 / -0
Enhancement (6) +167 / -0
BrowsingContext.csExpose StartScreencastAsync on BrowsingContext+5/-0
Expose StartScreencastAsync on BrowsingContext
• Adds a public StartScreencastAsync method that delegates to the BiDi BrowsingContext module implementation. Stop is intentionally modeled via the returned Screencast handle rather than a context method.
BrowsingContextModule.csRegister and execute start/stop screencast BiDi commands+24/-0
Register and execute start/stop screencast BiDi commands
• Introduces command registrations for browsingContext.startScreencast and browsingContext.stopScreencast and implements corresponding async methods. Updates source-generated JSON serialization metadata to include new parameter/result types.
Screencast.csAdd Screencast identifiable handle with StopAsync+68/-0
Add Screencast identifiable handle with StopAsync
• Adds a strongly-typed Screencast record implementing IIdentifiable, including JSON conversion support and equality semantics by Id. Provides a StopAsync convenience method that delegates to the module’s stop command.
StartScreencast.csDefine start screencast parameters, options, and result types+42/-0
Define start screencast parameters, options, and result types
• Introduces StartScreencastParameters plus a public StartScreencastOptions record supporting mime type, video constraints, and audio toggle. Defines MediaTrackConstraints and StartScreencastResult returning the screencast handle and output path.
StopScreencast.csDefine stop screencast parameters, options, and result types+26/-0
Define stop screencast parameters, options, and result types
• Adds StopScreencastParameters and public StopScreencastOptions, plus StopScreencastResult that returns the output path and an optional error string from the remote end.
BrowsingContextTests.csAdd start/stop screencast test (ignored on Chrome/Edge)+16/-0
Add start/stop screencast test (ignored on Chrome/Edge)
• Adds CanStartAndStopScreencast verifying that start returns a handle/path and stop returns the same path with no error. The test is ignored on Chrome and Edge due to current lack of support.
1. Flaky screencast test skips✗ Dismissed🐞 Bug☼ Reliability
Description
The new CanStartAndStopScreencast test is only ignored for Chrome and Edge, but Selenium’s own BiDi
integration coverage documents startScreencast as unsupported on Safari and failing on Firefox on
Linux. This will likely make the .NET test suite fail/flake on those environments.
The .NET test is only ignored for Chrome/Edge, while Selenium’s BiDi integration expectations in
this repo explicitly call out Safari as not implementing the command and Firefox-on-Linux as
failing, indicating the test will not be reliable across the project’s supported environments.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`CanStartAndStopScreencast` is enabled for browsers/platforms where the command is known to be unsupported or known to fail, which can break CI.
## Issue Context
In this repo’s BiDi integration tests, `browsingContext.startScreencast` is marked pending/unsupported on Safari, and known to fail on Firefox on Linux.
## Fix Focus Areas
- dotnet/test/webdriver/BiDi/BrowsingContext/BrowsingContextTests.cs[345-359]
## Suggested fix
- Add `[IgnoreBrowser(Browser.Safari, "Safari does not implement browsingContext.startScreencast")]` to the test.
- Add a targeted skip for the known Firefox/Linux failure. Since `IgnoreBrowserAttribute` cannot be combined with platform, consider either:
- adding a small new attribute that supports browser+platform filtering (preferred), or
- as an interim, add `[IgnorePlatform("linux", "Firefox startScreencast fails on Linux")`] if your CI for this test runs only on Linux (note this skips for all browsers on Linux).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Screencast cleanup leaks on failure 📘 Rule violation☼ Reliability
Description
The test records the screencast path only after StopAsync() completes, so an assertion failure
after startup or an exception during stopping leaves the screencast handle unavailable and only a
path eligible for deletion. The active screencast and its artifact can therefore remain, leaking
session resources and contaminating later test runs or the browser process.
+ if (Path.Exists(stopScreencastResult?.Path))+ {+ File.Delete(stopScreencastResult.Path);
Evidence
The test starts a screencast and receives its path before assertions or stopping, but initializes
and assigns stopScreencastResult only after awaiting StopAsync(), while finally conditionally
deletes only stopScreencastResult?.Path. If an assertion fails or stopping throws, assignment
never occurs; because the Screencast returned by startup is the handle whose StopAsync() sends
the stop command, losing that handle prevents both stopping the browser-side screencast and reliably
cleaning up its artifact.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The `finally` block deletes only `stopScreencastResult?.Path`, and the stop result is assigned only after `StopAsync()` completes. If startup succeeds but a later assertion fails or `StopAsync()` throws, the screencast handle is unavailable, no stop command is sent, and the screencast and its output artifact can remain active and leak resources into subsequent tests or the browser process.
## Issue Context
The screencast path is returned by `StartScreencastAsync` at lines 355-358, while the current cleanup depends on successful completion of `StopAsync()` at line 360. Keep the `Screencast` returned by `StartScreencastAsync` in a variable outside the `try` block; in `finally`, if that handle exists and stopping has not completed, attempt to stop it before deleting the resulting file. Cleanup should handle failures during both stopping and assertions without masking the original test failure.
## Fix Focus Areas
- dotnet/test/webdriver/BiDi/BrowsingContext/BrowsingContextTests.cs[353-371]
- dotnet/test/webdriver/BiDi/BrowsingContext/BrowsingContextTests.cs[351-371]
- dotnet/src/webdriver/BiDi/BrowsingContext/Screencast.cs[42-45]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. Screencast public types undocumented✗ Dismissed📘 Rule violation✧ Quality
Description
Multiple new public screencast-related types and interface/API members were introduced without XML
documentation that includes a non-empty <summary>. This violates the public API documentation
requirement and degrades IntelliSense and generated API docs for consumers.
+[JsonConverter(typeof(Converter))]+public sealed record Screencast : IIdentifiable+{+ public Screencast(IBiDi bidi, string id)
Evidence
PR Compliance ID 389245 requires XML <summary> documentation for all public members/types in the
diff. In the cited changes, the new public types (Screencast, StartScreencastOptions,
MediaTrackConstraints, StartScreencastResult, StopScreencastOptions, StopScreencastResult) are
declared without any preceding XML doc comment blocks with <summary>, and the public method
declarations StartScreencastAsync and StopScreencastAsync (including the added StartScreencastAsync
member specifically) likewise have no preceding /// <summary> documentation, demonstrating the rule
violation across both types and members.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
New public screencast-related API surface (types and members, including interface methods like `StartScreencastAsync`/`StopScreencastAsync`) was added without XML documentation blocks containing a non-empty `<summary>`, violating the public API documentation requirement.
## Issue Context
PR Compliance ID 389245 requires that all public members/types introduced in the diff include `///` XML documentation with a non-empty `<summary>` so the .NET binding public surface is properly discoverable via IntelliSense and complete in generated documentation.
## Fix Focus Areas
- dotnet/src/webdriver/BiDi/BrowsingContext/Screencast.cs[27-45]
- dotnet/src/webdriver/BiDi/BrowsingContext/StartScreencast.cs[24-42]
- dotnet/src/webdriver/BiDi/BrowsingContext/StopScreencast.cs[24-26]
- dotnet/src/webdriver/BiDi/BrowsingContext/IBrowsingContextModule.cs[49-50]
- dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs[108-112]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Context sources
Review mode: 🚀 Fast: This push is a small, localized test-only cleanup that changes exception-path cleanup behavior without touching production logic or high-risk areas.
Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt
1. Flaky screencast test skips✗ Dismissed🐞 Bug☼ Reliability
Description
The new CanStartAndStopScreencast test is only ignored for Chrome and Edge, but Selenium’s own BiDi
integration coverage documents startScreencast as unsupported on Safari and failing on Firefox on
Linux. This will likely make the .NET test suite fail/flake on those environments.
The .NET test is only ignored for Chrome/Edge, while Selenium’s BiDi integration expectations in
this repo explicitly call out Safari as not implementing the command and Firefox-on-Linux as
failing, indicating the test will not be reliable across the project’s supported environments.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`CanStartAndStopScreencast` is enabled for browsers/platforms where the command is known to be unsupported or known to fail, which can break CI.
## Issue Context
In this repo’s BiDi integration tests, `browsingContext.startScreencast` is marked pending/unsupported on Safari, and known to fail on Firefox on Linux.
## Fix Focus Areas
- dotnet/test/webdriver/BiDi/BrowsingContext/BrowsingContextTests.cs[345-359]
## Suggested fix
- Add `[IgnoreBrowser(Browser.Safari, "Safari does not implement browsingContext.startScreencast")]` to the test.
- Add a targeted skip for the known Firefox/Linux failure. Since `IgnoreBrowserAttribute` cannot be combined with platform, consider either:
- adding a small new attribute that supports browser+platform filtering (preferred), or
- as an interim, add `[IgnorePlatform("linux", "Firefox startScreencast fails on Linux")`] if your CI for this test runs only on Linux (note this skips for all browsers on Linux).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Screencast public types undocumented✗ Dismissed📘 Rule violation✧ Quality
Description
Multiple new public screencast-related types and interface/API members were introduced without XML
documentation that includes a non-empty <summary>. This violates the public API documentation
requirement and degrades IntelliSense and generated API docs for consumers.
+[JsonConverter(typeof(Converter))]+public sealed record Screencast : IIdentifiable+{+ public Screencast(IBiDi bidi, string id)
Evidence
PR Compliance ID 389245 requires XML <summary> documentation for all public members/types in the
diff. In the cited changes, the new public types (Screencast, StartScreencastOptions,
MediaTrackConstraints, StartScreencastResult, StopScreencastOptions, StopScreencastResult) are
declared without any preceding XML doc comment blocks with <summary>, and the public method
declarations StartScreencastAsync and StopScreencastAsync (including the added StartScreencastAsync
member specifically) likewise have no preceding /// <summary> documentation, demonstrating the rule
violation across both types and members.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
New public screencast-related API surface (types and members, including interface methods like `StartScreencastAsync`/`StopScreencastAsync`) was added without XML documentation blocks containing a non-empty `<summary>`, violating the public API documentation requirement.
## Issue Context
PR Compliance ID 389245 requires that all public members/types introduced in the diff include `///` XML documentation with a non-empty `<summary>` so the .NET binding public surface is properly discoverable via IntelliSense and complete in generated documentation.
## Fix Focus Areas
- dotnet/src/webdriver/BiDi/BrowsingContext/Screencast.cs[27-45]
- dotnet/src/webdriver/BiDi/BrowsingContext/StartScreencast.cs[24-42]
- dotnet/src/webdriver/BiDi/BrowsingContext/StopScreencast.cs[24-26]
- dotnet/src/webdriver/BiDi/BrowsingContext/IBrowsingContextModule.cs[49-50]
- dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContext.cs[108-112]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1. Screencast cleanup leaks on failure 📘 Rule violation☼ Reliability
Description
The test records the screencast path only after StopAsync() completes, so an assertion failure
after startup or an exception during stopping leaves the screencast handle unavailable and only a
path eligible for deletion. The active screencast and its artifact can therefore remain, leaking
session resources and contaminating later test runs or the browser process.
+ if (Path.Exists(stopScreencastResult?.Path))+ {+ File.Delete(stopScreencastResult.Path);
Evidence
The test starts a screencast and receives its path before assertions or stopping, but initializes
and assigns stopScreencastResult only after awaiting StopAsync(), while finally conditionally
deletes only stopScreencastResult?.Path. If an assertion fails or stopping throws, assignment
never occurs; because the Screencast returned by startup is the handle whose StopAsync() sends
the stop command, losing that handle prevents both stopping the browser-side screencast and reliably
cleaning up its artifact.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The `finally` block deletes only `stopScreencastResult?.Path`, and the stop result is assigned only after `StopAsync()` completes. If startup succeeds but a later assertion fails or `StopAsync()` throws, the screencast handle is unavailable, no stop command is sent, and the screencast and its output artifact can remain active and leak resources into subsequent tests or the browser process.
## Issue Context
The screencast path is returned by `StartScreencastAsync` at lines 355-358, while the current cleanup depends on successful completion of `StopAsync()` at line 360. Keep the `Screencast` returned by `StartScreencastAsync` in a variable outside the `try` block; in `finally`, if that handle exists and stopping has not completed, attempt to stop it before deleting the resulting file. Cleanup should handle failures during both stopping and assertions without masking the original test failure.
## Fix Focus Areas
- dotnet/test/webdriver/BiDi/BrowsingContext/BrowsingContextTests.cs[353-371]
- dotnet/test/webdriver/BiDi/BrowsingContext/BrowsingContextTests.cs[351-371]
- dotnet/src/webdriver/BiDi/BrowsingContext/Screencast.cs[42-45]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
StartSceencastStopScreencast💥 What does this PR do?
This pull request adds support for starting and stopping screencasts in the BiDi
BrowsingContextmodule, including new API methods, supporting types, and a test. The main changes introduce the ability to programmatically record the browser context and manage screencast sessions.New screencast functionality:
StartScreencastAsyncandStopScreencastAsyncmethods to theIBrowsingContextModuleinterface and implemented them inBrowsingContextModule, enabling clients to start and stop screencasts on a browsing context. [1] [2]StartScreencastOptions,StartScreencastResult, and related parameter/result types in the newStartScreencast.csfile, and similarly for stopping screencasts inStopScreencast.cs. These types define the options and results for screencast commands. [1] [2]Screencastrecord type, representing an active screencast session with aStopAsyncmethod to end the session.Integration and serialization:
Testing:
CanStartAndStopScreencast, to verify that screencast sessions can be started and stopped, and that the output file is managed correctly. The test currently ignores Chrome, Edge, and Firefox as they do not support this feature yet. [1] [2]🔧 Implementation Notes
Following existing pattern.
🔄 Types of changes