Skip to content

Commit

Permalink
refactor: turn Frame into EventEmitter
Browse files Browse the repository at this point in the history
  • Loading branch information
OrKoN committed Aug 8, 2023
1 parent 420421d commit 56930ae
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 59 deletions.
4 changes: 3 additions & 1 deletion docs/api/puppeteer.frame.md
Expand Up @@ -11,9 +11,11 @@ To understand frames, you can think of frames as `<iframe>` elements. Just like
#### Signature:

```typescript
export declare class Frame
export declare class Frame extends EventEmitter
```
**Extends:** [EventEmitter](./puppeteer.eventemitter.md)
## Remarks
Frame lifecycles are controlled by three events that are all dispatched on the parent [page](./puppeteer.frame.page.md):
Expand Down
7 changes: 5 additions & 2 deletions packages/puppeteer-core/src/api/Frame.ts
Expand Up @@ -19,6 +19,7 @@ import {HTTPResponse} from '../api/HTTPResponse.js';
import {Page, WaitTimeoutOptions} from '../api/Page.js';
import {CDPSession} from '../common/Connection.js';
import {DeviceRequestPrompt} from '../common/DeviceRequestPrompt.js';
import {EventEmitter} from '../common/EventEmitter.js';
import {ExecutionContext} from '../common/ExecutionContext.js';
import {getQueryHandlerAndSelector} from '../common/GetQueryHandler.js';
import {
Expand Down Expand Up @@ -223,7 +224,7 @@ export interface FrameAddStyleTagOptions {
*
* @public
*/
export class Frame {
export class Frame extends EventEmitter {
/**
* @internal
*/
Expand Down Expand Up @@ -251,7 +252,9 @@ export class Frame {
/**
* @internal
*/
constructor() {}
constructor() {
super();
}

/**
* The page associated with the frame.
Expand Down
18 changes: 16 additions & 2 deletions packages/puppeteer-core/src/common/Frame.ts
Expand Up @@ -37,6 +37,20 @@ import {LifecycleWatcher, PuppeteerLifeCycleEvent} from './LifecycleWatcher.js';
import {EvaluateFunc, EvaluateFuncWith, HandleFor, NodeFor} from './types.js';
import {withSourcePuppeteerURLIfNone} from './util.js';

/**
* We use symbols to prevent external parties listening to these events.
* They are internal to Puppeteer.
*
* @internal
*/
export const FrameEmittedEvents = {
FrameNavigated: Symbol('Frame.FrameNavigated'),
FrameSwapped: Symbol('Frame.FrameSwapped'),
LifecycleEvent: Symbol('Frame.LifecycleEvent'),
FrameNavigatedWithinDocument: Symbol('Frame.FrameNavigatedWithinDocument'),
FrameDetached: Symbol('Frame.FrameDetached'),
};

/**
* @internal
*/
Expand Down Expand Up @@ -106,7 +120,7 @@ export class Frame extends BaseFrame {

let ensureNewDocumentNavigation = false;
const watcher = new LifecycleWatcher(
this._frameManager,
this._frameManager.networkManager,
this,
waitUntil,
timeout
Expand Down Expand Up @@ -180,7 +194,7 @@ export class Frame extends BaseFrame {
timeout = this._frameManager.timeoutSettings.navigationTimeout(),
} = options;
const watcher = new LifecycleWatcher(
this._frameManager,
this._frameManager.networkManager,
this,
waitUntil,
timeout
Expand Down
28 changes: 21 additions & 7 deletions packages/puppeteer-core/src/common/FrameManager.ts
Expand Up @@ -20,12 +20,15 @@ import {Page} from '../api/Page.js';
import {assert} from '../util/assert.js';
import {isErrorLike} from '../util/ErrorLike.js';

import {CDPSession, isTargetClosedError} from './Connection.js';
import {
CDPSession,
CDPSessionEmittedEvents,
isTargetClosedError,
} from './Connection.js';
import {DeviceRequestPromptManager} from './DeviceRequestPrompt.js';
import {EventEmitter} from './EventEmitter.js';
import {ExecutionContext} from './ExecutionContext.js';
import {Frame} from './Frame.js';
import {Frame as CDPFrame} from './Frame.js';
import {Frame, FrameEmittedEvents} from './Frame.js';
import {FrameTree} from './FrameTree.js';
import {IsolatedWorld} from './IsolatedWorld.js';
import {MAIN_WORLD, PUPPETEER_WORLD} from './IsolatedWorlds.js';
Expand Down Expand Up @@ -54,8 +57,6 @@ export const FrameManagerEmittedEvents = {
FrameNavigatedWithinDocument: Symbol(
'FrameManager.FrameNavigatedWithinDocument'
),
ExecutionContextCreated: Symbol('FrameManager.ExecutionContextCreated'),
ExecutionContextDestroyed: Symbol('FrameManager.ExecutionContextDestroyed'),
};

/**
Expand Down Expand Up @@ -111,6 +112,12 @@ export class FrameManager extends EventEmitter {
this.#networkManager = new NetworkManager(client, ignoreHTTPSErrors, this);
this.#timeoutSettings = timeoutSettings;
this.setupEventListeners(this.#client);
client.once(CDPSessionEmittedEvents.Disconnected, () => {
const mainFrame = this._frameTree.getMainFrame();
if (mainFrame) {
this.#removeFramesRecursively(mainFrame);
}
});
}

private setupEventListeners(session: CDPSession) {
Expand Down Expand Up @@ -248,6 +255,7 @@ export class FrameManager extends EventEmitter {
}
frame._onLifecycleEvent(event.loaderId, event.name);
this.emit(FrameManagerEmittedEvents.LifecycleEvent, frame);
frame.emit(FrameEmittedEvents.LifecycleEvent);
}

#onFrameStartedLoading(frameId: string): void {
Expand All @@ -265,6 +273,7 @@ export class FrameManager extends EventEmitter {
}
frame._onLoadingStopped();
this.emit(FrameManagerEmittedEvents.LifecycleEvent, frame);
frame.emit(FrameEmittedEvents.LifecycleEvent);
}

#handleFrameTree(
Expand Down Expand Up @@ -309,7 +318,7 @@ export class FrameManager extends EventEmitter {
return;
}

frame = new CDPFrame(this, frameId, parentFrameId, session);
frame = new Frame(this, frameId, parentFrameId, session);
this._frameTree.addFrame(frame);
this.emit(FrameManagerEmittedEvents.FrameAttached, frame);
}
Expand All @@ -335,14 +344,15 @@ export class FrameManager extends EventEmitter {
frame._id = frameId;
} else {
// Initial main frame navigation.
frame = new CDPFrame(this, frameId, undefined, this.#client);
frame = new Frame(this, frameId, undefined, this.#client);
}
this._frameTree.addFrame(frame);
}

frame = await this._frameTree.waitForFrame(frameId);
frame._navigated(framePayload);
this.emit(FrameManagerEmittedEvents.FrameNavigated, frame);
frame.emit(FrameEmittedEvents.FrameNavigated);
}

async #createIsolatedWorld(session: CDPSession, name: string): Promise<void> {
Expand Down Expand Up @@ -385,7 +395,9 @@ export class FrameManager extends EventEmitter {
}
frame._navigatedWithinDocument(url);
this.emit(FrameManagerEmittedEvents.FrameNavigatedWithinDocument, frame);
frame.emit(FrameEmittedEvents.FrameNavigatedWithinDocument);
this.emit(FrameManagerEmittedEvents.FrameNavigated, frame);
frame.emit(FrameEmittedEvents.FrameNavigated);
}

#onFrameDetached(
Expand All @@ -402,6 +414,7 @@ export class FrameManager extends EventEmitter {
}
} else if (reason === 'swap') {
this.emit(FrameManagerEmittedEvents.FrameSwapped, frame);
frame?.emit(FrameEmittedEvents.FrameSwapped);
}
}

Expand Down Expand Up @@ -478,5 +491,6 @@ export class FrameManager extends EventEmitter {
frame._detach();
this._frameTree.removeFrame(frame);
this.emit(FrameManagerEmittedEvents.FrameDetached, frame);
frame.emit(FrameEmittedEvents.FrameDetached, frame);
}
}
2 changes: 1 addition & 1 deletion packages/puppeteer-core/src/common/IsolatedWorld.ts
Expand Up @@ -293,7 +293,7 @@ export class IsolatedWorld implements Realm {
await setPageContent(this, html);

const watcher = new LifecycleWatcher(
this.#frameManager,
this.#frameManager.networkManager,
this.#frame,
waitUntil,
timeout
Expand Down
63 changes: 19 additions & 44 deletions packages/puppeteer-core/src/common/LifecycleWatcher.ts
Expand Up @@ -18,12 +18,10 @@ import {HTTPResponse} from '../api/HTTPResponse.js';
import {assert} from '../util/assert.js';
import {Deferred} from '../util/Deferred.js';

import {CDPSessionEmittedEvents} from './Connection.js';
import {TimeoutError} from './Errors.js';
import {Frame} from './Frame.js';
import {FrameManager, FrameManagerEmittedEvents} from './FrameManager.js';
import {Frame, FrameEmittedEvents} from './Frame.js';
import {HTTPRequest} from './HTTPRequest.js';
import {NetworkManagerEmittedEvents} from './NetworkManager.js';
import {NetworkManager, NetworkManagerEmittedEvents} from './NetworkManager.js';
import {
addEventListener,
PuppeteerEventListener,
Expand Down Expand Up @@ -62,7 +60,6 @@ const puppeteerToProtocolLifecycle = new Map<
*/
export class LifecycleWatcher {
#expectedLifecycle: ProtocolLifeCycleEvent[];
#frameManager: FrameManager;
#frame: Frame;
#timeout: number;
#navigationRequest: HTTPRequest | null = null;
Expand All @@ -80,7 +77,7 @@ export class LifecycleWatcher {
#navigationResponseReceived?: Deferred<void>;

constructor(
frameManager: FrameManager,
networkManager: NetworkManager,
frame: Frame,
waitUntil: PuppeteerLifeCycleEvent | PuppeteerLifeCycleEvent[],
timeout: number
Expand All @@ -97,55 +94,46 @@ export class LifecycleWatcher {
return protocolEvent as ProtocolLifeCycleEvent;
});

this.#frameManager = frameManager;
this.#frame = frame;
this.#timeout = timeout;
this.#eventListeners = [
addEventListener(
frameManager.client,
CDPSessionEmittedEvents.Disconnected,
this.#terminate.bind(
this,
new Error('Navigation failed because browser has disconnected!')
)
),
addEventListener(
this.#frameManager,
FrameManagerEmittedEvents.LifecycleEvent,
frame,
FrameEmittedEvents.LifecycleEvent,
this.#checkLifecycleComplete.bind(this)
),
addEventListener(
this.#frameManager,
FrameManagerEmittedEvents.FrameNavigatedWithinDocument,
frame,
FrameEmittedEvents.FrameNavigatedWithinDocument,
this.#navigatedWithinDocument.bind(this)
),
addEventListener(
this.#frameManager,
FrameManagerEmittedEvents.FrameNavigated,
frame,
FrameEmittedEvents.FrameNavigated,
this.#navigated.bind(this)
),
addEventListener(
this.#frameManager,
FrameManagerEmittedEvents.FrameSwapped,
frame,
FrameEmittedEvents.FrameSwapped,
this.#frameSwapped.bind(this)
),
addEventListener(
this.#frameManager,
FrameManagerEmittedEvents.FrameDetached,
frame,
FrameEmittedEvents.FrameDetached,
this.#onFrameDetached.bind(this)
),
addEventListener(
this.#frameManager.networkManager,
networkManager,
NetworkManagerEmittedEvents.Request,
this.#onRequest.bind(this)
),
addEventListener(
this.#frameManager.networkManager,
networkManager,
NetworkManagerEmittedEvents.Response,
this.#onResponse.bind(this)
),
addEventListener(
this.#frameManager.networkManager,
networkManager,
NetworkManagerEmittedEvents.RequestFailed,
this.#onRequestFailed.bind(this)
),
Expand Down Expand Up @@ -204,10 +192,6 @@ export class LifecycleWatcher {
return this.#navigationRequest ? this.#navigationRequest.response() : null;
}

#terminate(error: Error): void {
this.#terminationDeferred.resolve(error);
}

sameDocumentNavigationPromise(): Promise<Error | undefined> {
return this.#sameDocumentNavigationDeferred.valueOrThrow();
}
Expand All @@ -224,25 +208,16 @@ export class LifecycleWatcher {
return this.#terminationDeferred.valueOrThrow();
}

#navigatedWithinDocument(frame: Frame): void {
if (frame !== this.#frame) {
return;
}
#navigatedWithinDocument(): void {
this.#hasSameDocumentNavigation = true;
this.#checkLifecycleComplete();
}

#navigated(frame: Frame): void {
if (frame !== this.#frame) {
return;
}
#navigated(): void {
this.#checkLifecycleComplete();
}

#frameSwapped(frame: Frame): void {
if (frame !== this.#frame) {
return;
}
#frameSwapped(): void {
this.#swapped = true;
this.#checkLifecycleComplete();
}
Expand Down
4 changes: 2 additions & 2 deletions test/src/launcher.spec.ts
Expand Up @@ -59,7 +59,7 @@ describe('Launcher specs', function () {
const error = await navigationPromise;
expect(
[
'Navigation failed because browser has disconnected!',
'Navigating frame was detached',
'Protocol error (Page.navigate): Target closed.',
].includes(error.message)
).toBeTruthy();
Expand All @@ -82,7 +82,7 @@ describe('Launcher specs', function () {
});
remote.disconnect();
const error = await watchdog;
expect(error.message).toContain('Protocol error');
expect(error.message).toContain('frame got detached');
} finally {
await close();
}
Expand Down

0 comments on commit 56930ae

Please sign in to comment.