Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: implement improved waitForNavigation #11929

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: implement improved waitForNavigation #11929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
183 changes: 107 additions & 76 deletions packages/puppeteer-core/src/bidi/Frame.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,18 @@ import type {Observable} from '../../third_party/rxjs/rxjs.js';
import {
combineLatest,
defer,
delayWhen,
filter,
first,
firstValueFrom,
from,
map,
merge,
mergeMap,
of,
raceWith,
switchMap,
startWith,
take,
zip,
} from '../../third_party/rxjs/rxjs.js';
import type {CDPSession} from '../api/CDPSession.js';
import type {ElementHandle} from '../api/ElementHandle.js';
Expand Down Expand Up @@ -121,10 +125,8 @@ export class BidiFrame extends Frame {
});
});

this.browsingContext.on('navigation', ({navigation}) => {
navigation.once('fragment', () => {
this.page().trustedEmitter.emit(PageEvent.FrameNavigated, this);
});
this.browsingContext.on('fragment', () => {
this.page().trustedEmitter.emit(PageEvent.FrameNavigated, this);
});
this.browsingContext.on('load', () => {
this.page().trustedEmitter.emit(PageEvent.Load, undefined);
Expand Down Expand Up @@ -292,23 +294,26 @@ export class BidiFrame extends Frame {
url: string,
options: GoToOptions = {}
): Promise<BidiHTTPResponse | null> {
const [response] = await Promise.all([
this.waitForNavigation(options),
// Some implementations currently only report errors when the
// readiness=interactive.
//
// Related: https://bugzilla.mozilla.org/show_bug.cgi?id=1846601
this.browsingContext.navigate(
url,
Bidi.BrowsingContext.ReadinessState.Interactive
),
]).catch(
return await firstValueFrom(
this.#waitForNavigation$({
...options,
// Some implementations currently only report errors when the
// readiness=interactive.
//
// Related: https://bugzilla.mozilla.org/show_bug.cgi?id=1846601
completion$: from(
this.browsingContext.navigate(
url,
Bidi.BrowsingContext.ReadinessState.Interactive
)
),
})
).catch(
rewriteNavigationError(
url,
options.timeout ?? this.timeoutSettings.navigationTimeout()
)
);
return response;
}

@throwIfDetached
Expand All @@ -331,64 +336,7 @@ export class BidiFrame extends Frame {
override async waitForNavigation(
options: WaitForOptions = {}
): Promise<BidiHTTPResponse | null> {
const {timeout: ms = this.timeoutSettings.navigationTimeout()} = options;

const frames = this.childFrames().map(frame => {
return frame.#detached$();
});
return await firstValueFrom(
combineLatest([
fromEmitterEvent(this.browsingContext, 'navigation').pipe(
switchMap(({navigation}) => {
return this.#waitForLoad$(options).pipe(
delayWhen(() => {
if (frames.length === 0) {
return of(undefined);
}
return combineLatest(frames);
}),
raceWith(
fromEmitterEvent(navigation, 'fragment'),
fromEmitterEvent(navigation, 'failed').pipe(
map(({url}) => {
throw new Error(`Navigation failed: ${url}`);
})
),
fromEmitterEvent(navigation, 'aborted').pipe(
map(({url}) => {
throw new Error(`Navigation aborted: ${url}`);
})
)
),
map(() => {
return navigation;
})
);
})
),
this.#waitForNetworkIdle$(options),
]).pipe(
map(([navigation]) => {
const request = navigation.request;
if (!request) {
return null;
}
const httpRequest = requests.get(request)!;
const lastRedirect = httpRequest.redirectChain().at(-1);
return (
lastRedirect !== undefined ? lastRedirect : httpRequest
).response();
}),
raceWith(
timeout(ms),
this.#detached$().pipe(
map(() => {
throw new TargetCloseError('Frame detached.');
})
)
)
)
);
return await firstValueFrom(this.#waitForNavigation$(options));
}

override waitForDevicePrompt(): never {
Expand Down Expand Up @@ -446,6 +394,89 @@ export class BidiFrame extends Frame {
return new BidiCdpSession(this, sessionId);
}

@throwIfDetached
#waitForNavigation$(
options: WaitForOptions & {
/**
* If defined, waitForNavigation$ will wait for this condition and a
* navigation.
*/
completion$?: Observable<unknown>;
} = {}
): Observable<BidiHTTPResponse | null> {
const {
timeout: ms = this.timeoutSettings.navigationTimeout(),
completion$,
} = options;

const enum State {
WaitingForRequest,
WaitingForResponse,
}

// Wait for the first navigation of any type.
let navigations$: Observable<unknown> = merge(
fromEmitterEvent(this.browsingContext, 'navigation'),
fromEmitterEvent(this.browsingContext, 'fragment')
).pipe(first());

// Wait for the completion condition to complete if defined.
if (completion$ !== undefined) {
navigations$ = zip(navigations$, completion$);
}

// An observable that returns the response to the first navigation request,
// if any.
const frames = this.childFrames();
const response$ = fromEmitterEvent(this.browsingContext, 'navigation').pipe(
take(1),
mergeMap(({navigation}) => {
// Wait for conditions before returning the response.
return combineLatest([
this.#waitForNetworkIdle$(options),
this.#waitForLoad$(options),
...frames.map(frame => {
return frame.#detached$();
}),
]).pipe(
map(() => {
if (!navigation.request) {
return null;
}
const httpRequest = requests.get(navigation.request)!;
const redirect = httpRequest.redirectChain().at(-1);
return (redirect !== undefined ? redirect : httpRequest).response();
}),
startWith(State.WaitingForResponse)
);
}),
startWith(State.WaitingForRequest)
);

return combineLatest([response$, navigations$]).pipe(
mergeMap(([stateOrResponse]) => {
switch (stateOrResponse) {
// If we are waiting for the navigation request, give up.
case State.WaitingForRequest:
return of(null);
// We shall also wait for the response.
case State.WaitingForResponse:
return of();
default:
return of(stateOrResponse);
}
}),
raceWith(
timeout(ms),
this.#detached$().pipe(
map(() => {
throw new TargetCloseError('Frame detached.');
})
)
)
);
}

@throwIfDetached
#waitForLoad$(options: WaitForOptions = {}): Observable<void> {
let {waitUntil = 'load'} = options;
Expand Down
35 changes: 24 additions & 11 deletions packages/puppeteer-core/src/bidi/core/BrowsingContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ export class BrowsingContext extends EventEmitter<{
/** The realm for the new dedicated worker */
realm: DedicatedWorkerRealm;
};
/** Emitted whenever the browsing context fragment navigates */
fragment: {
/** The new url */
url: string;
/** The timestamp of the navigation */
timestamp: Date;
};
}> {
static from(
userContext: UserContext,
Expand Down Expand Up @@ -214,35 +221,41 @@ export class BrowsingContext extends EventEmitter<{
if (info.context !== this.id) {
return;
}
this.#url = info.url;

for (const [id, request] of this.#requests) {
if (request.disposed) {
this.#requests.delete(id);
}
}
// If the navigation hasn't finished, then this is nested navigation. The
// current navigation will handle this.
if (this.#navigation !== undefined && !this.#navigation.disposed) {
return;

if (this.#navigation !== undefined) {
this.#navigation.dispose();
}

// Note the navigation ID is null for this event.
this.#navigation = Navigation.from(this);
this.#navigation = Navigation.from(this, info.url);

const navigationEmitter = this.#disposables.use(
new EventEmitter(this.#navigation)
);
for (const eventName of ['fragment', 'failed', 'aborted'] as const) {
navigationEmitter.once(eventName, ({url}) => {
for (const eventName of ['failed', 'aborted'] as const) {
navigationEmitter.once(eventName, () => {
navigationEmitter[disposeSymbol]();

this.#url = url;
});
}

this.emit('navigation', {navigation: this.#navigation});
});
sessionEmitter.on('browsingContext.fragmentNavigated', info => {
if (info.context !== this.id) {
return;
}
this.#url = info.url;

this.emit('fragment', {
url: info.url,
timestamp: new Date(info.timestamp),
});
});
sessionEmitter.on('network.beforeRequestSent', event => {
if (event.context !== this.id) {
return;
Expand Down
48 changes: 8 additions & 40 deletions packages/puppeteer-core/src/bidi/core/Navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,31 +26,30 @@ export interface NavigationInfo {
export class Navigation extends EventEmitter<{
/** Emitted when navigation has a request associated with it. */
request: Request;
/** Emitted when fragment navigation occurred. */
fragment: NavigationInfo;
/** Emitted when navigation failed. */
failed: NavigationInfo;
/** Emitted when navigation was aborted. */
aborted: NavigationInfo;
}> {
static from(context: BrowsingContext): Navigation {
const navigation = new Navigation(context);
static from(context: BrowsingContext, url: string): Navigation {
const navigation = new Navigation(context, url);
navigation.#initialize();
return navigation;
}

// keep-sorted start
#request: Request | undefined;
#navigation: Navigation | undefined;
readonly #browsingContext: BrowsingContext;
readonly #disposables = new DisposableStack();
readonly #id = new Deferred<string | null>();
readonly #url: string;
// keep-sorted end

private constructor(context: BrowsingContext) {
private constructor(context: BrowsingContext, url: string) {
super();
// keep-sorted start
this.#browsingContext = context;
this.#url = url;
// keep-sorted end
}

Expand Down Expand Up @@ -84,35 +83,7 @@ export class Navigation extends EventEmitter<{
const sessionEmitter = this.#disposables.use(
new EventEmitter(this.#session)
);
sessionEmitter.on('browsingContext.navigationStarted', info => {
if (
info.context !== this.#browsingContext.id ||
this.#navigation !== undefined
) {
return;
}
this.#navigation = Navigation.from(this.#browsingContext);
});

for (const eventName of [
'browsingContext.domContentLoaded',
'browsingContext.load',
] as const) {
sessionEmitter.on(eventName, info => {
if (
info.context !== this.#browsingContext.id ||
info.navigation === null ||
!this.#matches(info.navigation)
) {
return;
}

this.dispose();
});
}

for (const [eventName, event] of [
['browsingContext.fragmentNavigated', 'fragment'],
['browsingContext.navigationFailed', 'failed'],
['browsingContext.navigationAborted', 'aborted'],
] as const) {
Expand All @@ -136,9 +107,6 @@ export class Navigation extends EventEmitter<{
}

#matches(navigation: string | null): boolean {
if (this.#navigation !== undefined && !this.#navigation.disposed) {
return false;
}
if (!this.#id.resolved()) {
this.#id.resolve(navigation);
return true;
Expand All @@ -156,13 +124,13 @@ export class Navigation extends EventEmitter<{
get request(): Request | undefined {
return this.#request;
}
get navigation(): Navigation | undefined {
return this.#navigation;
get url(): string {
return this.#url;
}
// keep-sorted end

@inertIfDisposed
private dispose(): void {
dispose(): void {
this[disposeSymbol]();
}

Expand Down