[dotnet] [bidi] Serialize base64 encoded string directly to bytes

[nvborisenko]

User description

💥 What does this PR do?

There is no reason to keep base64 encoded string as System.String type, in any case it will be materialized to byte[]. So let's do it.

💡 Additional Considerations

It also reduces memory allocation, capturing screenshot: before 824.87 KB, after 655.86 KB

🔄 Types of changes

• Cleanup (formatting, renaming)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Enhancement

---

Description

• Replace base64 string handling with direct byte arrays

• Reduce memory allocation by ~20% for screenshot operations

• Update BiDi commands to use ReadOnlyMemory<byte> type

• Eliminate unnecessary base64 string conversions

---

Diagram Walkthrough

flowchart LR
  A["Base64 String"] --> B["ReadOnlyMemory<byte>"]
  B --> C["Reduced Memory Usage"]
  B --> D["Direct Byte Operations"]

Loading

File Walkthrough

	Relevant files
Enhancement	CaptureScreenshotCommand.cs Update screenshot result to use byte memory                            dotnet/src/webdriver/BiDi/BrowsingContext/CaptureScreenshotCommand.cs Changed CaptureScreenshotResult.Data from string to ReadOnlyMemory Updated ToByteArray() to use direct array conversion Added System namespace import +3/-2      PrintCommand.cs Update print result to use byte memory                                      dotnet/src/webdriver/BiDi/BrowsingContext/PrintCommand.cs Changed PrintResult.Data from string to ReadOnlyMemory Updated ToByteArray() method implementation +2/-2      BytesValue.cs Update bytes value to use memory type                                        dotnet/src/webdriver/BiDi/Network/BytesValue.cs Changed Base64BytesValue.Value from string to ReadOnlyMemory Updated implicit operator to work with byte arrays directly +2/-2      InstallCommand.cs Update extension data to use byte memory                                  dotnet/src/webdriver/BiDi/WebExtension/InstallCommand.cs Changed ExtensionBase64Encoded.Value from string to ReadOnlyMemory Added System namespace import +2/-1
Tests	WebExtensionTest.cs Update test for direct byte usage                                                dotnet/test/common/BiDi/WebExtension/WebExtensionTest.cs Updated test to pass byte array directly instead of base64 string Removed unnecessary base64 conversion step +2/-2

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Serialization Compatibility Changing CaptureScreenshotResult.Data from base64 string to ReadOnlyMemory<byte> alters the wire contract; ensure serializers/deserializers and all callers expect raw bytes and not base64 strings, otherwise BiDi protocol compliance or interop may break. public sealed record CaptureScreenshotResult(ReadOnlyMemory<byte> Data) : EmptyResult { public static implicit operator byte[](CaptureScreenshotResult captureScreenshotResult) => captureScreenshotResult.ToByteArray(); public byte[] ToByteArray() => Data.ToArray(); Discriminator Semantics Base64BytesValue now holds raw bytes but still uses the "base64" type discriminator; confirm that upstream/downstream components do not expect a base64-encoded string for this variant, and that custom converters handle ReadOnlyMemory<byte> correctly. public static implicit operator BytesValue(byte[] value) => new Base64BytesValue(value); } public sealed record StringBytesValue(string Value) : BytesValue; public sealed record Base64BytesValue(ReadOnlyMemory<byte> Value) : BytesValue; JSON Format Expectations ExtensionBase64Encoded now accepts ReadOnlyMemory<byte>; validate that JSON serialization uses the expected format (e.g., base64 in JSON by default) and that BiDi endpoints accept this representation without additional encoding steps. public sealed record ExtensionBase64Encoded(ReadOnlyMemory<byte> Value) : ExtensionData; public sealed record ExtensionPath(string Path) : ExtensionData;

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve base64 on the wire The JSON discriminator indicates type "base64", but the payload is now raw bytes; ensure serialization still emits base64 as required by the protocol. Add a custom converter to encode/decode the ReadOnlyMemory as base64 during JSON serialization. dotnet/src/webdriver/BiDi/Network/BytesValue.cs [28-36] public abstract record BytesValue { public static implicit operator BytesValue(string value) => new StringBytesValue(value); public static implicit operator BytesValue(byte[] value) => new Base64BytesValue(value); } public sealed record StringBytesValue(string Value) : BytesValue; +[JsonConverter(typeof(Base64BytesValueJsonConverter))] public sealed record Base64BytesValue(ReadOnlyMemory<byte> Value) : BytesValue; +internal sealed class Base64BytesValueJsonConverter : JsonConverter<Base64BytesValue> +{ + public override Base64BytesValue Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType != JsonTokenType.String) + { + throw new JsonException("Expected base64 string."); + } + var bytes = reader.HasValueSequence + ? Convert.FromBase64String(reader.GetString()!) + : Convert.FromBase64String(reader.GetString()!); + return new Base64BytesValue(bytes); + } + + public override void Write(Utf8JsonWriter writer, Base64BytesValue value, JsonSerializerOptions options) + { + writer.WriteStringValue(Convert.ToBase64String(value.Value.Span)); + } +} + Apply / Chat Suggestion importance[1-10]: 10 __ Why: This suggestion correctly identifies a critical bug where changing the data type to ReadOnlyMemory<byte> would break JSON serialization, as the protocol expects a base64 string. The proposed custom JsonConverter is the correct and necessary fix.	High
General	Avoid unnecessary byte array copies Avoid copying the buffer when the underlying memory is already backed by an array to prevent unnecessary allocations. Use TryGetArray on ReadOnlyMemory to return the underlying array when possible, otherwise fall back to ToArray(). dotnet/src/webdriver/BiDi/BrowsingContext/CaptureScreenshotCommand.cs [60-65] public sealed record CaptureScreenshotResult(ReadOnlyMemory<byte> Data) : EmptyResult { public static implicit operator byte[](CaptureScreenshotResult captureScreenshotResult) => captureScreenshotResult.ToByteArray(); - public byte[] ToByteArray() => Data.ToArray(); + public byte[] ToByteArray() + { +#if NETSTANDARD2_0 || NET461 + return Data.ToArray(); +#else + if (MemoryMarshal.TryGetArray(Data, out ArraySegment<byte> segment) && segment.Offset == 0 && segment.Count == segment.Array!.Length) + { + return segment.Array!; + } + return Data.ToArray(); +#endif + } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential performance improvement by avoiding an unnecessary array allocation and copy, which aligns with the PR's goal of optimizing byte handling.	Low
	Optimize memory-to-array conversion Mirror the optimized conversion to reduce allocations by returning the underlying array when available. This keeps behavior consistent with screenshot conversion and improves performance for large PDFs. dotnet/src/webdriver/BiDi/BrowsingContext/PrintCommand.cs [115-118] public sealed record PrintResult(ReadOnlyMemory<byte> Data) : EmptyResult { - public byte[] ToByteArray() => Data.ToArray(); + public byte[] ToByteArray() + { +#if NETSTANDARD2_0 || NET461 + return Data.ToArray(); +#else + if (MemoryMarshal.TryGetArray(Data, out ArraySegment<byte> segment) && segment.Offset == 0 && segment.Count == segment.Array!.Length) + { + return segment.Array!; + } + return Data.ToArray(); +#endif + } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: This suggestion correctly applies the same valid performance optimization from the first suggestion to a similar record, PrintResult, improving both performance and code consistency.	Low
Learned best practice	Add file and data validation Validate that path exists and bytes is non-empty before attempting to install the extension. Add guards to surface clear test failures instead of file or argument exceptions. dotnet/test/common/BiDi/WebExtension/WebExtensionTest.cs [65-69] var path = LocateRelativePath("common/extensions/webextensions-selenium-example.zip"); +Assert.That(File.Exists(path), Is.True, $"Extension archive not found at '{path}'"); var bytes = File.ReadAllBytes(path); +Assert.That(bytes, Is.Not.Null.And.Length.GreaterThan(0), "Extension archive is empty"); var result = await bidi.WebExtension.InstallAsync(new ExtensionBase64Encoded(bytes)); Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add null/argument validation to prevent runtime errors when using external resources.	Low
Update