[Lighthouse] Dismiss dialogs during navigation mode

adamraine · Devtools-frontend LUCI CQ · commit 52561448e121 · 2023-01-10T08:46:03.000Z
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>
diff --git a/front_end/core/sdk/ResourceTreeModel.ts b/front_end/core/sdk/ResourceTreeModel.ts
@@ -655,6 +655,7 @@ export enum Events {
   BackForwardCacheDetailsUpdated = 'BackForwardCacheDetailsUpdated',
   PrerenderingStatusUpdated = 'PrerenderingStatusUpdated',
   PrerenderAttemptCompleted = 'PrerenderAttemptCompleted',
+  JavaScriptDialogOpening = 'JavaScriptDialogOpening',
 }
 
 export type EventTypes = {
@@ -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 {
@@ -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});
     }
   }
diff --git a/front_end/panels/lighthouse/LighthouseProtocolService.ts b/front_end/panels/lighthouse/LighthouseProtocolService.ts
@@ -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> {
@@ -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;
@@ -163,6 +181,7 @@ export class ProtocolService {
       await oldParallelConnection.disconnect();
     }
     await SDK.TargetManager.TargetManager.instance().resumeAllTargets();
+    this.removeDialogHandler?.();
   }
 
   registerStatusCallback(callback: (arg0: string) => void): void {

Original file line number	Diff line number	Diff line change
`@@ -655,6 +655,7 @@ export enum Events {`
`655`	`655`	`BackForwardCacheDetailsUpdated = 'BackForwardCacheDetailsUpdated',`
`656`	`656`	`PrerenderingStatusUpdated = 'PrerenderingStatusUpdated',`
`657`	`657`	`PrerenderAttemptCompleted = 'PrerenderAttemptCompleted',`
	`658`	`+ JavaScriptDialogOpening = 'JavaScriptDialogOpening',`
`658`	`659`	`}`
`659`	`660`
`660`	`661`	`export type EventTypes = {`
`@@ -677,6 +678,7 @@ export type EventTypes = {`
`677`	`678`	`[Events.BackForwardCacheDetailsUpdated]: ResourceTreeFrame,`
`678`	`679`	`[Events.PrerenderingStatusUpdated]: ResourceTreeFrame,`
`679`	`680`	`[Events.PrerenderAttemptCompleted]: Protocol.Page.PrerenderAttemptCompletedEvent,`
	`681`	`+ [Events.JavaScriptDialogOpening]: Protocol.Page.JavascriptDialogOpeningEvent,`
`680`	`682`	`};`
`681`	`683`
`682`	`684`	`export class ResourceTreeFrame {`
`@@ -1154,8 +1156,9 @@ export class PageDispatcher implements ProtocolProxyApi.PageDispatcher {`
`1154`	`1156`	`this.#resourceTreeModel.dispatchEventToListeners(Events.FrameResized);`
`1155`	`1157`	`}`
`1156`	`1158`
`1157`		`- javascriptDialogOpening({hasBrowserHandler}: Protocol.Page.JavascriptDialogOpeningEvent): void {`
`1158`		`- if (!hasBrowserHandler) {`
	`1159`	`+ javascriptDialogOpening(event: Protocol.Page.JavascriptDialogOpeningEvent): void {`
	`1160`	`+ this.#resourceTreeModel.dispatchEventToListeners(Events.JavaScriptDialogOpening, event);`
	`1161`	`+ if (!event.hasBrowserHandler) {`
`1159`	`1162`	`void this.#resourceTreeModel.agent.invoke_handleJavaScriptDialog({accept: false});`
`1160`	`1163`	`}`
`1161`	`1164`	`}`