Skip to content

Commit

Permalink
[Lighthouse] Dismiss dialogs during navigation mode
Browse files Browse the repository at this point in the history
Required for bfcache testing in Lighthouse to work:
GoogleChrome/lighthouse#14465

Bug: None
Change-Id: I4faf54e5dee9acdf8ffde4ee8762c21ed7fb50b5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4117673
Commit-Queue: Adam Raine <asraine@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Paul Irish <paulirish@chromium.org>
  • Loading branch information
adamraine authored and Devtools-frontend LUCI CQ committed Jan 10, 2023
1 parent b6b4321 commit 5256144
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
7 changes: 5 additions & 2 deletions front_end/core/sdk/ResourceTreeModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ export enum Events {
BackForwardCacheDetailsUpdated = 'BackForwardCacheDetailsUpdated',
PrerenderingStatusUpdated = 'PrerenderingStatusUpdated',
PrerenderAttemptCompleted = 'PrerenderAttemptCompleted',
JavaScriptDialogOpening = 'JavaScriptDialogOpening',
}

export type EventTypes = {
Expand All @@ -677,6 +678,7 @@ export type EventTypes = {
[Events.BackForwardCacheDetailsUpdated]: ResourceTreeFrame,
[Events.PrerenderingStatusUpdated]: ResourceTreeFrame,
[Events.PrerenderAttemptCompleted]: Protocol.Page.PrerenderAttemptCompletedEvent,
[Events.JavaScriptDialogOpening]: Protocol.Page.JavascriptDialogOpeningEvent,
};

export class ResourceTreeFrame {
Expand Down Expand Up @@ -1154,8 +1156,9 @@ export class PageDispatcher implements ProtocolProxyApi.PageDispatcher {
this.#resourceTreeModel.dispatchEventToListeners(Events.FrameResized);
}

javascriptDialogOpening({hasBrowserHandler}: Protocol.Page.JavascriptDialogOpeningEvent): void {
if (!hasBrowserHandler) {
javascriptDialogOpening(event: Protocol.Page.JavascriptDialogOpeningEvent): void {
this.#resourceTreeModel.dispatchEventToListeners(Events.JavaScriptDialogOpening, event);
if (!event.hasBrowserHandler) {
void this.#resourceTreeModel.agent.invoke_handleJavaScriptDialog({accept: false});
}
}
Expand Down
19 changes: 19 additions & 0 deletions front_end/panels/lighthouse/LighthouseProtocolService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export class ProtocolService {
private parallelConnection?: ProtocolClient.InspectorBackend.Connection;
private lighthouseWorkerPromise?: Promise<Worker>;
private lighthouseMessageUpdateCallback?: ((arg0: string) => void);
private removeDialogHandler?: () => void;
private configForTesting?: Object;

async attach(): Promise<void> {
Expand Down Expand Up @@ -92,6 +93,23 @@ export class ProtocolService {
this.dispatchProtocolMessage(message);
});

// Lighthouse implements its own dialog handler like this, however its lifecycle ends when
// the internal Lighthouse session is disposed.
//
// If the page is reloaded near the end of the run (e.g. bfcache testing), the Lighthouse
// internal session can be disposed before a dialog message appears. This allows the dialog
// to block important Lighthouse teardown operations in LighthouseProtocolService.
//
// To ensure the teardown operations can proceed, we need a dialog handler which lasts until
// the LighthouseProtocolService detaches.
const dialogHandler = (): void => {
void mainTarget.pageAgent().invoke_handleJavaScriptDialog({accept: true});
};

resourceTreeModel.addEventListener(SDK.ResourceTreeModel.Events.JavaScriptDialogOpening, dialogHandler);
this.removeDialogHandler = (): void =>
resourceTreeModel.removeEventListener(SDK.ResourceTreeModel.Events.JavaScriptDialogOpening, dialogHandler);

this.parallelConnection = connection;
this.targetInfos = childTargetManager.targetInfos();
this.mainFrameId = mainFrame.id;
Expand Down Expand Up @@ -163,6 +181,7 @@ export class ProtocolService {
await oldParallelConnection.disconnect();
}
await SDK.TargetManager.TargetManager.instance().resumeAllTargets();
this.removeDialogHandler?.();
}

registerStatusCallback(callback: (arg0: string) => void): void {
Expand Down

0 comments on commit 5256144

Please sign in to comment.