Skip to content

Commit

Permalink
feat: support goog:channel along with channel (#2996)
Browse files Browse the repository at this point in the history
Step towards addressing
#2840
  • Loading branch information
sadym-chromium authored Jan 17, 2025
1 parent 15a96c7 commit d7ed911
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 75 deletions.
7 changes: 2 additions & 5 deletions src/bidiMapper/BidiServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,8 @@ export class BidiServer extends EventEmitter<BidiServerEvent> {
};

#processOutgoingMessage = async (messageEntry: OutgoingMessage) => {
const message = messageEntry.message;

if (messageEntry.channel !== null) {
message['channel'] = messageEntry.channel;
}
// Enrich message with channel data.
const message = {...messageEntry.message, ...messageEntry.channel};

await this.#transport.sendMessage(message);
};
Expand Down
7 changes: 2 additions & 5 deletions src/bidiMapper/OutgoingMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ export class OutgoingMessage {
readonly #message: ChromiumBidi.Message;
readonly #channel: BidiPlusChannel;

private constructor(
message: ChromiumBidi.Message,
channel: BidiPlusChannel = null,
) {
private constructor(message: ChromiumBidi.Message, channel: BidiPlusChannel) {
this.#message = message;
this.#channel = channel;
}
Expand All @@ -48,7 +45,7 @@ export class OutgoingMessage {

static createResolved(
message: ChromiumBidi.Message,
channel?: BidiPlusChannel,
channel: BidiPlusChannel,
): Promise<Result<OutgoingMessage>> {
return Promise.resolve({
kind: 'success',
Expand Down
2 changes: 1 addition & 1 deletion src/bidiMapper/modules/network/NetworkStorage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('NetworkStorage', () => {
// Verify that the Request send the message
// To the correct context
[MockCdpNetworkEvents.defaultFrameId],
null,
{},
);
eventManager.on(EventManagerEvents.Event, ({message, event}) => {
processingQueue.add(message, event);
Expand Down
17 changes: 11 additions & 6 deletions src/bidiMapper/modules/session/EventManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ export class EventManager extends EventEmitter<EventManagerEventsMap> {
*/
#eventBuffers = new Map<string, Buffer<EventWrapper>>();
/**
* Maps `eventName` + `browsingContext` to Map of channel to last id
* Maps `eventName` + `browsingContext` to Map of json stringified channel to last id.
* Used to avoid sending duplicated events when user
* subscribes -> unsubscribes -> subscribes.
*/
#lastMessageSent = new Map<string, Map<string | null, number>>();
#lastMessageSent = new Map<string, Map<string, number>>();
#subscriptionManager: SubscriptionManager;
#browsingContextStorage: BrowsingContextStorage;
/**
Expand Down Expand Up @@ -366,15 +366,19 @@ export class EventManager extends EventEmitter<EventManagerEventsMap> {
);

const lastId = Math.max(
this.#lastMessageSent.get(lastSentMapKey)?.get(channel) ?? 0,
this.#lastMessageSent.get(lastSentMapKey)?.get(JSON.stringify(channel)) ??
0,
eventWrapper.id,
);

const channelMap = this.#lastMessageSent.get(lastSentMapKey);
if (channelMap) {
channelMap.set(channel, lastId);
channelMap.set(JSON.stringify(channel), lastId);
} else {
this.#lastMessageSent.set(lastSentMapKey, new Map([[channel, lastId]]));
this.#lastMessageSent.set(
lastSentMapKey,
new Map([[JSON.stringify(channel), lastId]]),
);
}
}

Expand All @@ -388,7 +392,8 @@ export class EventManager extends EventEmitter<EventManagerEventsMap> {
): EventWrapper[] {
const bufferMapKey = EventManager.#getMapKey(eventName, contextId);
const lastSentMessageId =
this.#lastMessageSent.get(bufferMapKey)?.get(channel) ?? -Infinity;
this.#lastMessageSent.get(bufferMapKey)?.get(JSON.stringify(channel)) ??
-Infinity;

const result: EventWrapper[] =
this.#eventBuffers
Expand Down
4 changes: 2 additions & 2 deletions src/bidiMapper/modules/session/SessionProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export class SessionProcessor {

async subscribe(
params: Session.SubscriptionRequest,
channel: BidiPlusChannel = null,
channel: BidiPlusChannel = {},
): Promise<Session.SubscribeResult> {
const subscription = await this.#eventManager.subscribe(
params.events as ChromiumBidi.EventNames[],
Expand All @@ -158,7 +158,7 @@ export class SessionProcessor {

async unsubscribe(
params: Session.SubscriptionRequest,
channel: BidiPlusChannel = null,
channel: BidiPlusChannel = {},
): Promise<EmptyResult> {
await this.#eventManager.unsubscribe(
params.events as ChromiumBidi.EventNames[],
Expand Down
4 changes: 2 additions & 2 deletions src/bidiMapper/modules/session/SubscriptionManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ const SOME_NESTED_CONTEXT = 'SOME_NESTED_CONTEXT';
const ANOTHER_CONTEXT = 'ANOTHER_CONTEXT';
const ANOTHER_NESTED_CONTEXT = 'ANOTHER_NESTED_CONTEXT';

const SOME_CHANNEL = 'SOME_CHANNEL';
const ANOTHER_CHANNEL = 'ANOTHER_CHANNEL';
const SOME_CHANNEL = {'goog:channel': 'SOME_CHANNEL'};
const ANOTHER_CHANNEL = {'goog:channel': 'ANOTHER_CHANNEL'};

describe('SubscriptionManager', () => {
let subscriptionManager: SubscriptionManager;
Expand Down
25 changes: 18 additions & 7 deletions src/bidiMapper/modules/session/SubscriptionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,29 +95,39 @@ export class SubscriptionManager {
eventName: ChromiumBidi.EventNames,
contextId: BrowsingContext.BrowsingContext,
): BidiPlusChannel[] {
const channels = new Set<BidiPlusChannel>();
// Maps JSON stringified channel to a channel.
// TODO: switch to `Set` of `goog:channel` once legacy `channel` is removed.
const channels = new Map<string, BidiPlusChannel>();

for (const subscription of this.#subscriptions) {
if (this.#isSubscribedTo(subscription, eventName, contextId)) {
channels.add(subscription.channel);
channels.set(
JSON.stringify(subscription.channel),
subscription.channel,
);
}
}

return Array.from(channels);
return Array.from(channels.values());
}

getChannelsSubscribedToEventGlobally(
eventName: ChromiumBidi.EventNames,
): BidiPlusChannel[] {
const channels = new Set<BidiPlusChannel>();
// Maps JSON stringified channel to a channel.
// TODO: switch to `Set` of `goog:channel` once legacy `channel` is removed.
const channels = new Map<string, BidiPlusChannel>();

for (const subscription of this.#subscriptions) {
if (this.#isSubscribedTo(subscription, eventName)) {
channels.add(subscription.channel);
channels.set(
JSON.stringify(subscription.channel),
subscription.channel,
);
}
}

return Array.from(channels);
return Array.from(channels.values());
}

#isSubscribedTo(
Expand Down Expand Up @@ -249,7 +259,8 @@ export class SubscriptionManager {
const eventsMatched = new Set<ChromiumBidi.EventNames>();
const contextsMatched = new Set<BrowsingContext.BrowsingContext>();
for (const subscription of this.#subscriptions) {
if (subscription.channel !== channel) {
// `channel` is undefined or an object with 1 field, so `JSON.stringify` is stable.
if (JSON.stringify(subscription.channel) !== JSON.stringify(channel)) {
newSubscriptions.push(subscription);
continue;
}
Expand Down
49 changes: 31 additions & 18 deletions src/bidiTab/Transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class WindowBidiTransport implements BidiTransport {
} catch (e: unknown) {
const error = e instanceof Error ? e : new Error(e as string);
// Transport-level error does not provide channel.
this.#respondWithError(message, ErrorCode.InvalidArgument, error, null);
this.#respondWithError(message, ErrorCode.InvalidArgument, error, {});
}
};
}
Expand Down Expand Up @@ -73,14 +73,10 @@ export class WindowBidiTransport implements BidiTransport {
error,
);

if (channel) {
this.sendMessage({
...errorResponse,
channel,
});
} else {
this.sendMessage(errorResponse);
}
this.sendMessage({
...errorResponse,
...(channel ?? {}),
});
}

static #getJsonType(value: unknown) {
Expand Down Expand Up @@ -120,7 +116,13 @@ export class WindowBidiTransport implements BidiTransport {
}

static #parseBidiMessage(message: string): ChromiumBidi.Command {
let command: ChromiumBidi.Command;
let command: {
id?: unknown;
method?: unknown;
params?: unknown;
channel?: unknown;
'goog:channel'?: unknown;
};
try {
command = JSON.parse(message);
} catch {
Expand All @@ -136,7 +138,7 @@ export class WindowBidiTransport implements BidiTransport {
const {id, method, params} = command;

const idType = WindowBidiTransport.#getJsonType(id);
if (idType !== 'number' || !Number.isInteger(id) || id < 0) {
if (idType !== 'number' || !Number.isInteger(id) || (id as number) < 0) {
// TODO: should uint64_t be the upper limit?
// https://tools.ietf.org/html/rfc7049#section-2.1
throw new Error(`Expected unsigned integer but got ${idType}`);
Expand All @@ -152,15 +154,26 @@ export class WindowBidiTransport implements BidiTransport {
throw new Error(`Expected object params but got ${paramsType}`);
}

let channel = command.channel;
if (channel !== undefined) {
const channelType = WindowBidiTransport.#getJsonType(channel);
let channel: BidiPlusChannel = {};
if (command['goog:channel'] !== undefined) {
const channelType = WindowBidiTransport.#getJsonType(
command['goog:channel'],
);
if (channelType !== 'string') {
throw new Error(
`Expected string value of 'goog:channel' but got ${channelType}`,
);
}
if (command['goog:channel'] !== '') {
channel = {'goog:channel': command['goog:channel'] as string};
}
} else if (command.channel !== undefined) {
const channelType = WindowBidiTransport.#getJsonType(command.channel);
if (channelType !== 'string') {
throw new Error(`Expected string channel but got ${channelType}`);
throw new Error(`Expected string 'channel' but got ${channelType}`);
}
// Empty string channel is considered as no channel provided.
if (channel === '') {
channel = undefined;
if (command.channel !== '') {
channel = {channel: command.channel as string};
}
}

Expand Down
22 changes: 17 additions & 5 deletions src/protocol/chromium-bidi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,7 @@ export type Command = (
// not re-define it. Therefore, it's not part of generated types.
id: WebDriverBidi.JsUint;
} & WebDriverBidiBluetooth.Bluetooth.SimulateAdvertisement)
) & {
channel?: WebDriverBidi.Script.Channel;
};
) & {channel: BidiPlusChannel};

export type CommandResponse =
| WebDriverBidi.CommandResponse
Expand All @@ -153,8 +151,22 @@ export const EVENT_NAMES = new Set([

export type ResultData = WebDriverBidi.ResultData | Cdp.ResultData;

export type BidiPlusChannel = string | null;
// TODO: replace with optional string once legacy `channel` is removed.
export type BidiPlusChannel =
| {
'goog:channel': string;
channel?: never;
}
| {
'goog:channel'?: never;
channel: string;
}
| {
'goog:channel'?: never;
channel?: never;
};

export type Message = (WebDriverBidi.Message | Cdp.Message | BluetoothEvent) & {
channel?: BidiPlusChannel;
channel?: string;
'goog:channel'?: string;
};
Loading

0 comments on commit d7ed911

Please sign in to comment.