Skip to content
Open
47 changes: 26 additions & 21 deletions dotnet/src/webdriver/BiDi/BiDi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,13 @@ namespace OpenQA.Selenium.BiDi;

public sealed class BiDi : IAsyncDisposable
{
private readonly Broker _broker;
private readonly JsonSerializerOptions _jsonOptions;
internal Broker Broker { get; }
internal JsonSerializerOptions JsonOptions { get; }
private readonly BiDiJsonSerializerContext _jsonContext;

private BiDi(string url)
public JsonSerializerOptions DefaultBiDiOptions()
{
var uri = new Uri(url);

_jsonOptions = new JsonSerializerOptions
return new JsonSerializerOptions
{
PropertyNameCaseInsensitive = true,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
Expand All @@ -61,20 +59,27 @@ private BiDi(string url)
new WebExtensionConverter(this),
}
};
}

private BiDi(string url)
{
var uri = new Uri(url);

_jsonContext = new BiDiJsonSerializerContext(_jsonOptions);

_broker = new Broker(this, uri, _jsonOptions);
SessionModule = Module.Create<Session.SessionModule>(this, _broker, _jsonOptions, _jsonContext);
BrowsingContext = Module.Create<BrowsingContext.BrowsingContextModule>(this, _broker, _jsonOptions, _jsonContext);
Browser = Module.Create<Browser.BrowserModule>(this, _broker, _jsonOptions, _jsonContext);
Network = Module.Create<Network.NetworkModule>(this, _broker, _jsonOptions, _jsonContext);
InputModule = Module.Create<Input.InputModule>(this, _broker, _jsonOptions, _jsonContext);
Script = Module.Create<Script.ScriptModule>(this, _broker, _jsonOptions, _jsonContext);
Log = Module.Create<Log.LogModule>(this, _broker, _jsonOptions, _jsonContext);
Storage = Module.Create<Storage.StorageModule>(this, _broker, _jsonOptions, _jsonContext);
WebExtension = Module.Create<WebExtension.WebExtensionModule>(this, _broker, _jsonOptions, _jsonContext);
Emulation = Module.Create<Emulation.EmulationModule>(this, _broker, _jsonOptions, _jsonContext);
JsonOptions = DefaultBiDiOptions();

_jsonContext = new BiDiJsonSerializerContext(JsonOptions);

Broker = new Broker(this, uri, JsonOptions);
SessionModule = Module.Create<Session.SessionModule>(this, JsonOptions, _jsonContext);
BrowsingContext = Module.Create<BrowsingContext.BrowsingContextModule>(this, JsonOptions, _jsonContext);
Browser = Module.Create<Browser.BrowserModule>(this, JsonOptions, _jsonContext);
Network = Module.Create<Network.NetworkModule>(this, JsonOptions, _jsonContext);
InputModule = Module.Create<Input.InputModule>(this, JsonOptions, _jsonContext);
Script = Module.Create<Script.ScriptModule>(this, JsonOptions, _jsonContext);
Log = Module.Create<Log.LogModule>(this, JsonOptions, _jsonContext);
Storage = Module.Create<Storage.StorageModule>(this, JsonOptions, _jsonContext);
WebExtension = Module.Create<WebExtension.WebExtensionModule>(this, JsonOptions, _jsonContext);
Emulation = Module.Create<Emulation.EmulationModule>(this, JsonOptions, _jsonContext);
}

internal Session.SessionModule SessionModule { get; }
Expand Down Expand Up @@ -106,7 +111,7 @@ public static async Task<BiDi> ConnectAsync(string url, BiDiOptions? options = n
{
var bidi = new BiDi(url);

await bidi._broker.ConnectAsync(CancellationToken.None).ConfigureAwait(false);
await bidi.Broker.ConnectAsync(CancellationToken.None).ConfigureAwait(false);

return bidi;
}
Expand All @@ -118,7 +123,7 @@ public Task EndAsync(Session.EndOptions? options = null)

public async ValueTask DisposeAsync()
{
await _broker.DisposeAsync().ConfigureAwait(false);
await Broker.DisposeAsync().ConfigureAwait(false);
GC.SuppressFinalize(this);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="WebDriver.Extensions.cs" company="Selenium Committers">
// <copyright file="BiDiExtensions.cs" company="Selenium Committers">
// Licensed to the Software Freedom Conservancy (SFC) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
Expand All @@ -22,7 +22,7 @@

namespace OpenQA.Selenium.BiDi;

public static class WebDriverExtensions
public static class BiDiExtensions
{
public static async Task<BiDi> AsBiDiAsync(this IWebDriver webDriver, BiDiOptions? options = null)
{
Expand Down
9 changes: 9 additions & 0 deletions dotnet/src/webdriver/BiDi/Browser/BrowserModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@
// </copyright>

using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.BiDi.Communication.Json;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;

namespace OpenQA.Selenium.BiDi.Browser;

public sealed class BrowserModule : Module
{
internal new BiDiJsonSerializerContext JsonContext => (BiDiJsonSerializerContext)base.JsonContext;
Copy link
Member

Choose a reason for hiding this comment

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

@RenderMichael this piece is strange, seems there is no reason to keep JsonContext in base class. If you have the same opinion, I can remake it to private field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The base class is responsible for creating the JSON context. This is the only way to bridge the gap between "strongly-typed JSON context" and the logic in Module.Create

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another way to get around this is by making the Json Context a generic parameter, eg. BrowserModule : Module<BiDiJsonSerializerContext>. That requires making BiDiJsonSerializerContext public, and more publicly exposed. I don't think that's desirable.

Copy link
Member

Choose a reason for hiding this comment

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

I also was thinking about BrowserModule : Module<BiDiJsonSerializerContext>, but it seems even stranger. Module of JsonContext... I am reading it hard, given that BiDiJsonSerializerContext cannot be inherited.

Ideally we should get rid of BiDiJsonSerializerContext, we are moving in this direction.

About internal new BiDiJsonSerializerContext JsonContext => (BiDiJsonSerializerContext)base.JsonContext;: For me it is risky, at least not obvious...

  • is it one time initialization? (question from interview)
  • what is executed first? our void Initialize(...) or this implicit initialization
  • does it mean that we cast each time when we invoke JsonContext getter?

Private field looks really simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we want to go that route, then we will have to undo optimizations such as this one: #16397

Otherwise, we will need a separate JSON context for each module

Ideally we should get rid of BiDiJsonSerializerContext, we are moving in this direction.

What do you have in mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Second optimization that would need to be de-optimized: #16392

Copy link
Member

@nvborisenko nvborisenko Oct 21, 2025

Choose a reason for hiding this comment

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

Internal modules will still use global JsonContext for a while, because of f**g cross-referencing (stateful converters, polymorphic dto). In future, each module wants to:

void Initialize(BiDi bidi, JsonOptions options) // just common options like Naming policy
{
  var moduleOptions = new JsonOptions(options) // make a copy
  {
     Converters = { new MySpecificConverter(bidi) } // bad example, just to highlight possibilities
  }

  _myContext = new MyCoontext(moduleOptions);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we do cannot have a global JsonContext for third-party extensions, it looks like we will need 2 different kinds of modules: extensions and non-extension (core?).

because of f**g cross-referencing (stateful converters, polymorphic dto)

That shouldn't be a problem! We can get rid of stateful converters (very slightly less pleasant usability, I think it is no problem) and polymorphic DTOs got addressed by moving the converters out to attributes.

The only remaining converter is DateTimeOffsetConverter. We could go through to each instance of DateTimeOffset and add that converter as an attribute. Alternatively, we could change the type to long in the DTO, since it is technically magic. If we want to address that, then we will have no converters left!

Copy link
Member

Choose a reason for hiding this comment

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

DateTimeOffsetConverter is not a problem, it is kind of common JsonOptions. it is OK to have global well-known converters.

I don't want to block this PR, we can improve later. But seems private MyJsonContext should be here. Broker requires JsonTypeInfo.

Copy link
Member

Choose a reason for hiding this comment

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

Since we do cannot have a global JsonContext for third-party extensions, it looks like we will need 2 different kinds of modules: extensions and non-extension (core?).

Each module should have own JsonContext, ideally. And it is out of scope of this PR.


public async Task<CloseResult> CloseAsync(CloseOptions? options = null)
{
return await Broker.ExecuteCommandAsync(new CloseCommand(), options, JsonContext.Browser_CloseCommand, JsonContext.Browser_CloseResult).ConfigureAwait(false);
Expand Down Expand Up @@ -74,4 +79,8 @@ public async Task<SetDownloadBehaviorResult> SetDownloadBehaviorDeniedAsync(SetD

return await Broker.ExecuteCommandAsync(new SetDownloadBehaviorCommand(@params), options, JsonContext.SetDownloadBehaviorCommand, JsonContext.SetDownloadBehaviorResult).ConfigureAwait(false);
}
protected override JsonSerializerContext CreateJsonContext(JsonSerializerOptions options)
{
return new BiDiJsonSerializerContext(options);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,19 @@
// under the License.
// </copyright>

using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.BiDi.Communication.Json;
using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using OpenQA.Selenium.BiDi.Communication;

namespace OpenQA.Selenium.BiDi.BrowsingContext;

public sealed class BrowsingContextModule : Module
{
internal new BiDiJsonSerializerContext JsonContext => (BiDiJsonSerializerContext)base.JsonContext;

public async Task<CreateResult> CreateAsync(ContextType type, CreateOptions? options = null)
{
var @params = new CreateParameters(type, options?.ReferenceContext, options?.Background, options?.UserContext);
Expand Down Expand Up @@ -248,4 +253,8 @@ public async Task<Subscription> OnUserPromptClosedAsync(Action<UserPromptClosedE
{
return await Broker.SubscribeAsync("browsingContext.userPromptClosed", handler, options, JsonContext.UserPromptClosedEventArgs).ConfigureAwait(false);
}
protected override JsonSerializerContext CreateJsonContext(JsonSerializerOptions options)
{
return new BiDiJsonSerializerContext(options);
}
}
11 changes: 10 additions & 1 deletion dotnet/src/webdriver/BiDi/Emulation/EmulationModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,18 @@
// under the License.
// </copyright>

using System.Threading.Tasks;
using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.BiDi.Communication.Json;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;

namespace OpenQA.Selenium.BiDi.Emulation;

public sealed class EmulationModule : Module
{
internal new BiDiJsonSerializerContext JsonContext => (BiDiJsonSerializerContext)base.JsonContext;

public async Task<SetTimezoneOverrideResult> SetTimezoneOverrideAsync(string? timezone, SetTimezoneOverrideOptions? options = null)
{
var @params = new SetTimezoneOverrideParameters(timezone, options?.Contexts, options?.UserContexts);
Expand Down Expand Up @@ -88,4 +93,8 @@ public async Task<SetGeolocationOverrideResult> SetGeolocationPositionErrorOverr

return await Broker.ExecuteCommandAsync(new SetGeolocationOverrideCommand(@params), options, JsonContext.SetGeolocationOverrideCommand, JsonContext.SetGeolocationOverrideResult).ConfigureAwait(false);
}
protected override JsonSerializerContext CreateJsonContext(JsonSerializerOptions options)
{
return new BiDiJsonSerializerContext(options);
}
}
9 changes: 9 additions & 0 deletions dotnet/src/webdriver/BiDi/Input/InputModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@
// </copyright>

using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.BiDi.Communication.Json;
using System.Collections.Generic;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;

namespace OpenQA.Selenium.BiDi.Input;

public sealed class InputModule : Module
{
internal new BiDiJsonSerializerContext JsonContext => (BiDiJsonSerializerContext)base.JsonContext;

public async Task<PerformActionsResult> PerformActionsAsync(BrowsingContext.BrowsingContext context, IEnumerable<SourceActions> actions, PerformActionsOptions? options = null)
{
var @params = new PerformActionsParameters(context, actions);
Expand All @@ -45,4 +50,8 @@ public async Task<SetFilesResult> SetFilesAsync(BrowsingContext.BrowsingContext

return await Broker.ExecuteCommandAsync(new SetFilesCommand(@params), options, JsonContext.SetFilesCommand, JsonContext.SetFilesResult).ConfigureAwait(false);
}
protected override JsonSerializerContext CreateJsonContext(JsonSerializerOptions options)
{
return new BiDiJsonSerializerContext(options);
}
}
13 changes: 11 additions & 2 deletions dotnet/src/webdriver/BiDi/Log/LogModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,19 @@
// under the License.
// </copyright>

using System.Threading.Tasks;
using System;
using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.BiDi.Communication.Json;
using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;

namespace OpenQA.Selenium.BiDi.Log;

public sealed class LogModule : Module
{
internal new BiDiJsonSerializerContext JsonContext => (BiDiJsonSerializerContext)base.JsonContext;

public async Task<Subscription> OnEntryAddedAsync(Func<LogEntry, Task> handler, SubscriptionOptions? options = null)
{
return await Broker.SubscribeAsync("log.entryAdded", handler, options, JsonContext.LogEntry).ConfigureAwait(false);
Expand All @@ -34,4 +39,8 @@ public async Task<Subscription> OnEntryAddedAsync(Action<LogEntry> handler, Subs
{
return await Broker.SubscribeAsync("log.entryAdded", handler, options, JsonContext.LogEntry).ConfigureAwait(false);
}
protected override JsonSerializerContext CreateJsonContext(JsonSerializerOptions options)
{
return new BiDiJsonSerializerContext(options);
}
}
13 changes: 6 additions & 7 deletions dotnet/src/webdriver/BiDi/Module.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,28 @@
// </copyright>

using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.BiDi.Communication.Json;
using System.Text.Json;
using System.Text.Json.Serialization;

namespace OpenQA.Selenium.BiDi;

public abstract class Module
{
protected Broker Broker { get; private set; }

internal BiDiJsonSerializerContext JsonContext { get; private set; }
internal JsonSerializerContext JsonContext { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

I am not ready to generalize it in this PR. Still not clear how to move on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have an example in this PR’s description, of how to move forward with these changes.

Copy link
Member

Choose a reason for hiding this comment

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

I though making Module.Create(...) to public would be enough, please give me time to review.

Copy link
Member

Choose a reason for hiding this comment

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

Observed it is theoretical changes. Let be practicable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is, except for the shared JSON context introduced in #16402

It is not theoretical changes.

Copy link
Member

Choose a reason for hiding this comment

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

I came to the following:

  1. BiDi has new method AsModule<T>()
  2. PermissionsModule creates its own context based on options (and keep it as private field)

Then changes looks simple.

Copy link
Member

Choose a reason for hiding this comment

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

I feel introducing new AsModule<T>() method will be good for us:

  • we may introduce cached modules
  • it negotiates use of internal properties

Copy link
Member

Choose a reason for hiding this comment

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

All right, AsModule<T> is required. Built-in modules are cached, third-party modules also want to be cached per bidi instance. Let's do it (here or in separate PR).


protected virtual void Initialize(JsonSerializerOptions options) { }
protected abstract JsonSerializerContext CreateJsonContext(JsonSerializerOptions options);

internal static TModule Create<TModule>(BiDi bidi, Broker broker, JsonSerializerOptions jsonOptions, BiDiJsonSerializerContext context)
public static TModule Create<TModule>(BiDi bidi, JsonSerializerOptions jsonOptions, JsonSerializerContext? cachedContext = null)
where TModule : Module, new()
{
TModule module = new()
{
Broker = broker,
JsonContext = context
Broker = bidi.Broker,
};

module.Initialize(jsonOptions);
module.JsonContext = cachedContext ?? module.CreateJsonContext(jsonOptions);

return module;
}
Expand Down
11 changes: 10 additions & 1 deletion dotnet/src/webdriver/BiDi/Network/NetworkModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,20 @@
// under the License.
// </copyright>

using OpenQA.Selenium.BiDi.Communication;
using OpenQA.Selenium.BiDi.Communication.Json;
using System;
using System.Collections.Generic;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using OpenQA.Selenium.BiDi.Communication;

namespace OpenQA.Selenium.BiDi.Network;

public sealed partial class NetworkModule : Module
{
internal new BiDiJsonSerializerContext JsonContext => (BiDiJsonSerializerContext)base.JsonContext;

public async Task<Collector> AddDataCollectorAsync(IEnumerable<DataType> DataTypes, int MaxEncodedDataSize, AddDataCollectorOptions? options = null)
{
var @params = new AddDataCollectorParameters(DataTypes, MaxEncodedDataSize, options?.CollectorType, options?.Contexts, options?.UserContexts);
Expand Down Expand Up @@ -173,4 +178,8 @@ public async Task<Subscription> OnAuthRequiredAsync(Action<AuthRequiredEventArgs
{
return await Broker.SubscribeAsync("network.authRequired", handler, options, JsonContext.AuthRequiredEventArgs).ConfigureAwait(false);
}
protected override JsonSerializerContext CreateJsonContext(JsonSerializerOptions options)
{
return new BiDiJsonSerializerContext(options);
}
}
22 changes: 22 additions & 0 deletions dotnet/src/webdriver/BiDi/Permissions/PermissionDescriptor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// <copyright file="PermissionDescriptor.cs" company="Selenium Committers">
// Licensed to the Software Freedom Conservancy (SFC) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The SFC licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
// </copyright>

namespace OpenQA.Selenium.BiDi.Permissions;

internal record PermissionDescriptor(string Name);
31 changes: 31 additions & 0 deletions dotnet/src/webdriver/BiDi/Permissions/PermissionState.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// <copyright file="PermissionState.cs" company="Selenium Committers">
// Licensed to the Software Freedom Conservancy (SFC) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The SFC licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
// </copyright>

using OpenQA.Selenium.BiDi.Communication.Json.Converters;
using System.Text.Json.Serialization;

namespace OpenQA.Selenium.BiDi.Permissions;

[JsonConverter(typeof(CamelCaseEnumConverter<PermissionState>))]
public enum PermissionState
{
Granted,
Denied,
Prompt
}
36 changes: 36 additions & 0 deletions dotnet/src/webdriver/BiDi/Permissions/PermissionsExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// <copyright file="PermissionsExtensions.cs" company="Selenium Committers">
// Licensed to the Software Freedom Conservancy (SFC) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The SFC licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
// </copyright>

using OpenQA.Selenium.BiDi.Extensions.Permissions;
using System;

namespace OpenQA.Selenium.BiDi.Permissions;

public static class PermissionsExtensions
{
public static PermissionsModule AsPermissions(this BiDi bidi)
{
if (bidi is null)
{
throw new ArgumentNullException(nameof(bidi));
}

return Module.Create<PermissionsModule>(bidi, bidi.DefaultBiDiOptions());
}
}
Loading