Skip to content

Commit

Permalink
feat(scopes): make page a scope (#4300)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman authored Nov 2, 2020
1 parent 1255289 commit d117d0b
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 22 deletions.
20 changes: 8 additions & 12 deletions src/dispatchers/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class Dispatcher<Type, Initializer> extends EventEmitter implements chann

(object as any)[dispatcherSymbol] = this;
if (this._parent)
this._connection.sendMessageToClient(this._parent._guid, '__create__', { type, initializer, guid }, !!isScope);
this._connection.sendMessageToClient(this._parent._guid, '__create__', { type, initializer, guid });
}

_dispatchEvent(method: string, params: Dispatcher<any, any> | any = {}) {
Expand Down Expand Up @@ -126,9 +126,8 @@ export class DispatcherConnection {
private _validateParams: (type: string, method: string, params: any) => any;
private _validateMetadata: (metadata: any) => any;

sendMessageToClient(guid: string, method: string, params: any, disallowDispatchers?: boolean) {
const allowDispatchers = !disallowDispatchers;
this.onmessage({ guid, method, params: this._replaceDispatchersWithGuids(params, allowDispatchers) });
sendMessageToClient(guid: string, method: string, params: any) {
this.onmessage({ guid, method, params: this._replaceDispatchersWithGuids(params) });
}

constructor() {
Expand Down Expand Up @@ -178,26 +177,23 @@ export class DispatcherConnection {
try {
const validated = this._validateParams(dispatcher._type, method, params);
const result = await (dispatcher as any)[method](validated, this._validateMetadata(metadata));
this.onmessage({ id, result: this._replaceDispatchersWithGuids(result, true) });
this.onmessage({ id, result: this._replaceDispatchersWithGuids(result) });
} catch (e) {
this.onmessage({ id, error: serializeError(e) });
}
}

private _replaceDispatchersWithGuids(payload: any, allowDispatchers: boolean): any {
private _replaceDispatchersWithGuids(payload: any): any {
if (!payload)
return payload;
if (payload instanceof Dispatcher) {
if (!allowDispatchers)
throw new Error(`Channels are not allowed in the scope's initialzier`);
if (payload instanceof Dispatcher)
return { guid: payload._guid };
}
if (Array.isArray(payload))
return payload.map(p => this._replaceDispatchersWithGuids(p, allowDispatchers));
return payload.map(p => this._replaceDispatchersWithGuids(p));
if (typeof payload === 'object') {
const result: any = {};
for (const key of Object.keys(payload))
result[key] = this._replaceDispatchersWithGuids(payload[key], allowDispatchers);
result[key] = this._replaceDispatchersWithGuids(payload[key]);
return result;
}
return payload;
Expand Down
9 changes: 6 additions & 3 deletions src/dispatchers/pageDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,17 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageInitializer> i
videoRelativePath: page._video ? page._video._relativePath : undefined,
viewportSize: page.viewportSize() || undefined,
isClosed: page.isClosed()
});
}, true);
this._page = page;
page.on(Page.Events.Close, () => this._dispatchEvent('close'));
page.on(Page.Events.Close, () => {
this._dispatchEvent('close');
this._dispose();
});
page.on(Page.Events.Console, message => this._dispatchEvent('console', { message: new ConsoleMessageDispatcher(this._scope, message) }));
page.on(Page.Events.Crash, () => this._dispatchEvent('crash'));
page.on(Page.Events.DOMContentLoaded, () => this._dispatchEvent('domcontentloaded'));
page.on(Page.Events.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this._scope, dialog) }));
page.on(Page.Events.Download, download => this._dispatchEvent('download', { download: new DownloadDispatcher(this._scope, download) }));
page.on(Page.Events.Download, download => this._dispatchEvent('download', { download: new DownloadDispatcher(scope, download) }));
this._page.on(Page.Events.FileChooser, (fileChooser: FileChooser) => this._dispatchEvent('fileChooser', {
element: new ElementHandleDispatcher(this._scope, fileChooser.element()),
isMultiple: fileChooser.isMultiple()
Expand Down
3 changes: 2 additions & 1 deletion src/server/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,8 @@ export class FrameManager {
this.removeChildFramesRecursively(frame);
frame._onDetached();
this._frames.delete(frame._id);
this._page.emit(Page.Events.FrameDetached, frame);
if (!this._page.isClosed())
this._page.emit(Page.Events.FrameDetached, frame);
}

private _inflightRequestFinished(request: network.Request) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class CustomError extends Error {
export class TimeoutError extends CustomError {}

export const kBrowserClosedError = 'Browser has been closed';
export const kBrowserOrContextClosedError = 'Target browser or context has been closed';
export const kBrowserOrContextClosedError = 'Target page, context or browser has been closed';

export function isSafeCloseError(error: Error) {
return error.message.endsWith(kBrowserClosedError) || error.message.endsWith(kBrowserOrContextClosedError);
Expand Down
7 changes: 4 additions & 3 deletions test/channels.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ it('should scope context handles', async ({browser, server}) => {
{ _guid: 'Browser', objects: [
{ _guid: 'BrowserContext', objects: [
{ _guid: 'Frame', objects: [] },
{ _guid: 'Page', objects: [] },
{ _guid: 'Request', objects: [] },
{ _guid: 'Response', objects: [] },
{ _guid: 'Page', objects: [
{ _guid: 'Request', objects: [] },
{ _guid: 'Response', objects: [] },
]},
]},
] },
] },
Expand Down
2 changes: 1 addition & 1 deletion test/chromium/session.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('session', (suite, { browserName }) => {
} catch (e) {
error = e;
}
expect(error.message).toContain('Target browser or context has been closed');
expect(error.message).toContain('Target page, context or browser has been closed');
});

it('should throw nice errors', async function({page}) {
Expand Down
12 changes: 11 additions & 1 deletion test/page-close.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { it } from './fixtures';
import { it, expect } from './fixtures';

it('should close page with active dialog', (test, { browserName, platform }) => {
test.fixme(browserName === 'webkit' && platform === 'darwin', 'WebKit hangs on a Mac');
Expand Down Expand Up @@ -43,3 +43,13 @@ it('should access page after beforeunload', (test, { browserName }) => {
await dialog.dismiss();
await page.evaluate(() => document.title);
});

it('should not accept after close', (test, { browserName, platform }) => {
test.fixme(browserName === 'webkit' && platform === 'darwin', 'WebKit hangs on a Mac');
}, async ({page}) => {
page.evaluate(() => alert()).catch(() => {});
const dialog = await page.waitForEvent('dialog');
await page.close();
const e = await dialog.dismiss().catch(e => e);
expect(e.message).toContain('Target page, context or browser has been closed');
});

0 comments on commit d117d0b

Please sign in to comment.