Skip to content

Commit

Permalink
fix(websocket): remove "skip frames" logic (#4435)
Browse files Browse the repository at this point in the history
This optimization turned out to be racy, so better remove it for now.
  • Loading branch information
dgozman committed Nov 13, 2020
1 parent cd18ddb commit e69315f
Show file tree
Hide file tree
Showing 7 changed files with 0 additions and 38 deletions.
10 changes: 0 additions & 10 deletions src/client/page.ts
Expand Up @@ -579,17 +579,13 @@ export class Page extends ChannelOwner<channels.PageChannel, channels.PageInitia
on(event: string | symbol, listener: Listener): this {
if (event === Events.Page.FileChooser && !this.listenerCount(event))
this._channel.setFileChooserInterceptedNoReply({ intercepted: true });
if (event === Events.Page.WebSocket && !this.listenerCount(event))
this._channel.setWebSocketFramesReportingEnabledNoReply({ enabled: true });
super.on(event, listener);
return this;
}

addListener(event: string | symbol, listener: Listener): this {
if (event === Events.Page.FileChooser && !this.listenerCount(event))
this._channel.setFileChooserInterceptedNoReply({ intercepted: true });
if (event === Events.Page.WebSocket && !this.listenerCount(event))
this._channel.setWebSocketFramesReportingEnabledNoReply({ enabled: true });
super.addListener(event, listener);
return this;
}
Expand All @@ -598,19 +594,13 @@ export class Page extends ChannelOwner<channels.PageChannel, channels.PageInitia
super.off(event, listener);
if (event === Events.Page.FileChooser && !this.listenerCount(event))
this._channel.setFileChooserInterceptedNoReply({ intercepted: false });
// Note: we do not stop reporting web socket frames, since
// user might not listen to 'websocket' anymore, but still have
// a functioning WebSocket object.
return this;
}

removeListener(event: string | symbol, listener: Listener): this {
super.removeListener(event, listener);
if (event === Events.Page.FileChooser && !this.listenerCount(event))
this._channel.setFileChooserInterceptedNoReply({ intercepted: false });
// Note: we do not stop reporting web socket frames, since
// user might not listen to 'websocket' anymore, but still have
// a functioning WebSocket object.
return this;
}

Expand Down
4 changes: 0 additions & 4 deletions src/dispatchers/pageDispatcher.ts
Expand Up @@ -158,10 +158,6 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageInitializer> i
await this._page._setFileChooserIntercepted(params.intercepted);
}

async setWebSocketFramesReportingEnabledNoReply(params: channels.PageSetWebSocketFramesReportingEnabledNoReplyParams): Promise<void> {
this._page._setWebSocketFramesReportingEnabled(params.enabled);
}

async keyboardDown(params: channels.PageKeyboardDownParams): Promise<void> {
await this._page.keyboard.down(params.key);
}
Expand Down
8 changes: 0 additions & 8 deletions src/protocol/channels.ts
Expand Up @@ -728,7 +728,6 @@ export interface PageChannel extends Channel {
setDefaultNavigationTimeoutNoReply(params: PageSetDefaultNavigationTimeoutNoReplyParams, metadata?: Metadata): Promise<PageSetDefaultNavigationTimeoutNoReplyResult>;
setDefaultTimeoutNoReply(params: PageSetDefaultTimeoutNoReplyParams, metadata?: Metadata): Promise<PageSetDefaultTimeoutNoReplyResult>;
setFileChooserInterceptedNoReply(params: PageSetFileChooserInterceptedNoReplyParams, metadata?: Metadata): Promise<PageSetFileChooserInterceptedNoReplyResult>;
setWebSocketFramesReportingEnabledNoReply(params: PageSetWebSocketFramesReportingEnabledNoReplyParams, metadata?: Metadata): Promise<PageSetWebSocketFramesReportingEnabledNoReplyResult>;
addInitScript(params: PageAddInitScriptParams, metadata?: Metadata): Promise<PageAddInitScriptResult>;
close(params: PageCloseParams, metadata?: Metadata): Promise<PageCloseResult>;
emulateMedia(params: PageEmulateMediaParams, metadata?: Metadata): Promise<PageEmulateMediaResult>;
Expand Down Expand Up @@ -840,13 +839,6 @@ export type PageSetFileChooserInterceptedNoReplyOptions = {

};
export type PageSetFileChooserInterceptedNoReplyResult = void;
export type PageSetWebSocketFramesReportingEnabledNoReplyParams = {
enabled: boolean,
};
export type PageSetWebSocketFramesReportingEnabledNoReplyOptions = {

};
export type PageSetWebSocketFramesReportingEnabledNoReplyResult = void;
export type PageAddInitScriptParams = {
source: string,
};
Expand Down
4 changes: 0 additions & 4 deletions src/protocol/protocol.yml
Expand Up @@ -625,10 +625,6 @@ Page:
parameters:
intercepted: boolean

setWebSocketFramesReportingEnabledNoReply:
parameters:
enabled: boolean

addInitScript:
parameters:
source: string
Expand Down
3 changes: 0 additions & 3 deletions src/protocol/validator.ts
Expand Up @@ -330,9 +330,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
scheme.PageSetFileChooserInterceptedNoReplyParams = tObject({
intercepted: tBoolean,
});
scheme.PageSetWebSocketFramesReportingEnabledNoReplyParams = tObject({
enabled: tBoolean,
});
scheme.PageAddInitScriptParams = tObject({
source: tString,
});
Expand Down
4 changes: 0 additions & 4 deletions src/server/frames.ts
Expand Up @@ -359,16 +359,12 @@ export class FrameManager {
}

onWebSocketFrameSent(requestId: string, opcode: number, data: string) {
if (!this._page._webSocketFramesReportingEnabled)
return;
const ws = this._webSockets.get(requestId);
if (ws)
ws.frameSent(opcode, data);
}

webSocketFrameReceived(requestId: string, opcode: number, data: string) {
if (!this._page._webSocketFramesReportingEnabled)
return;
const ws = this._webSockets.get(requestId);
if (ws)
ws.frameReceived(opcode, data);
Expand Down
5 changes: 0 additions & 5 deletions src/server/page.ts
Expand Up @@ -147,7 +147,6 @@ export class Page extends EventEmitter {
_ownedContext: BrowserContext | undefined;
readonly selectors: Selectors;
_video: Video | null = null;
_webSocketFramesReportingEnabled = false;

constructor(delegate: PageDelegate, browserContext: BrowserContext) {
super();
Expand Down Expand Up @@ -441,10 +440,6 @@ export class Page extends EventEmitter {
await this._delegate.setFileChooserIntercepted(enabled);
}

_setWebSocketFramesReportingEnabled(enabled: boolean) {
this._webSocketFramesReportingEnabled = enabled;
}

videoStarted(video: Video) {
this._video = video;
this.emit(Page.Events.VideoStarted, video);
Expand Down

0 comments on commit e69315f

Please sign in to comment.