From c75b91d96c91535c730d4be0ac4e34837500ac93 Mon Sep 17 00:00:00 2001
From: dpapad <[email protected]>
Date: Tue, 16 Jun 2026 12:20:26 -0700
Subject: [PATCH] WebUI: Fix leaking cr-dialog 'popstate' listener when disconnected.

The leaking listener is the root cause for various flaky tests, such as
the one that caused the revert at crrev.com/c/7944686.

After fixing the leaking listener, some workarounds in
chrome/test/data/webui/settings/people_page_test.ts can be removed,
where individual test cases were waiting for 'popstate' to fire before
declaring the test done, to not interfere with subsequent tests, which
is fragile and shouldn't be the responsibility of a test case to begin
with.

Bug: 522412676
Change-Id: Id72486ea8f4dd84be95839aaee6e28e3dd692dce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7947646
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Reviewed-by: John Lee <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1647795}
---

diff --git a/chrome/test/data/webui/cr_elements/cr_dialog_test.ts b/chrome/test/data/webui/cr_elements/cr_dialog_test.ts
index df7d46eed..cdfbe4e0 100644
--- a/chrome/test/data/webui/cr_elements/cr_dialog_test.ts
+++ b/chrome/test/data/webui/cr_elements/cr_dialog_test.ts
@@ -616,4 +616,22 @@
     window.dispatchEvent(new CustomEvent('popstate'));
     assertTrue(dialog.open);
   });
+
+  test('popstate listener removed on disconnect', function() {
+    document.body.innerHTML = getTrustedHTML`
+      <cr-dialog>
+        <div slot="title">title</div>
+      </cr-dialog>`;
+    const dialog = document.body.querySelector('cr-dialog')!;
+    dialog.showModal();
+
+    let cancelFired = false;
+    dialog.addEventListener('cancel', () => {
+      cancelFired = true;
+    });
+
+    dialog.remove();
+    window.dispatchEvent(new CustomEvent('popstate'));
+    assertFalse(cancelFired);
+  });
 });
diff --git a/chrome/test/data/webui/settings/people_page_test.ts b/chrome/test/data/webui/settings/people_page_test.ts
index 506bd691..2f551ac0 100644
--- a/chrome/test/data/webui/settings/people_page_test.ts
+++ b/chrome/test/data/webui/settings/people_page_test.ts
@@ -380,12 +380,6 @@
         loadTimeData.getStringF(
             'deleteProfileWarningWithCountsPlural', 2, 'fakeUsername'),
         warningMessage.textContent.trim());
-
-    // Close the disconnect dialog.
-    signoutDialog.$.disconnectConfirm.click();
-    await new Promise(function(resolve) {
-      listenOnce(window, 'popstate', resolve);
-    });
   });
 
   test('NavigateDirectlyToSignOutURL', async function() {
@@ -401,13 +395,6 @@
     // handler if the user navigates directly to
     // chrome://settings/signOut. if so, it should not cause a crash.
     new ProfileInfoBrowserProxyImpl().getProfileStatsCount();
-
-    // Close the disconnect dialog.
-    peoplePage.shadowRoot!.querySelector('settings-signout-dialog')!.$
-        .disconnectConfirm.click();
-    await new Promise(function(resolve) {
-      listenOnce(window, 'popstate', resolve);
-    });
   });
 
   test('Signout dialog suppressed when not signed in', async function() {
diff --git a/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.ts b/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.ts
index 2fb3122f..91f8a25 100644
--- a/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.ts
+++ b/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.ts
@@ -22,6 +22,7 @@
 import '../cr_icon_button/cr_icon_button.js';
 
 import {assert} from '//resources/js/assert.js';
+import {EventTracker} from '//resources/js/event_tracker.js';
 import {CrLitElement} from '//resources/lit/v3_0/lit.rollup.js';
 
 import type {CrInputElement} from '../cr_input/cr_input.js';
@@ -108,6 +109,7 @@
 
   private mutationObserver_: MutationObserver|null = null;
   private boundKeydown_: ((e: KeyboardEvent) => void)|null = null;
+  private tracker_: EventTracker = new EventTracker();
 
   override connectedCallback() {
     super.connectedCallback();
@@ -131,6 +133,12 @@
     if (this.showOnAttach) {
       this.showModal();
     }
+
+    this.tracker_.add(window, 'popstate', () => {
+      if (!this.ignorePopstate && this.$.dialog.open) {
+        this.cancel();
+      }
+    });
   }
 
   override disconnectedCallback() {
@@ -140,17 +148,10 @@
       this.mutationObserver_.disconnect();
       this.mutationObserver_ = null;
     }
+    this.tracker_.removeAll();
   }
 
   override firstUpdated() {
-    // If the active history entry changes (i.e. user clicks back button),
-    // all open dialogs should be cancelled.
-    window.addEventListener('popstate', () => {
-      if (!this.ignorePopstate && this.$.dialog.open) {
-        this.cancel();
-      }
-    });
-
     if (!this.ignoreEnterKey) {
       this.addEventListener('keypress', this.onKeypress_.bind(this));
     }

diff --git a/chrome/test/data/webui/cr_elements/cr_dialog_test.ts b/chrome/test/data/webui/cr_elements/cr_dialog_test.ts
index df7d46eed..cdfbe4e0 100644
--- a/chrome/test/data/webui/cr_elements/cr_dialog_test.ts
+++ b/chrome/test/data/webui/cr_elements/cr_dialog_test.ts
@@ -616,4 +616,22 @@
     window.dispatchEvent(new CustomEvent('popstate'));
     assertTrue(dialog.open);
   });
+
+  test('popstate listener removed on disconnect', function() {
+    document.body.innerHTML = getTrustedHTML`
+      <cr-dialog>
+        <div slot="title">title</div>
+      </cr-dialog>`;
+    const dialog = document.body.querySelector('cr-dialog')!;
+    dialog.showModal();
+
+    let cancelFired = false;
+    dialog.addEventListener('cancel', () => {
+      cancelFired = true;
+    });
+
+    dialog.remove();
+    window.dispatchEvent(new CustomEvent('popstate'));
+    assertFalse(cancelFired);
+  });
 });
diff --git a/chrome/test/data/webui/settings/people_page_test.ts b/chrome/test/data/webui/settings/people_page_test.ts
index 506bd691..2f551ac0 100644
--- a/chrome/test/data/webui/settings/people_page_test.ts
+++ b/chrome/test/data/webui/settings/people_page_test.ts
@@ -380,12 +380,6 @@
         loadTimeData.getStringF(
             'deleteProfileWarningWithCountsPlural', 2, 'fakeUsername'),
         warningMessage.textContent.trim());
-
-    // Close the disconnect dialog.
-    signoutDialog.$.disconnectConfirm.click();
-    await new Promise(function(resolve) {
-      listenOnce(window, 'popstate', resolve);
-    });
   });
 
   test('NavigateDirectlyToSignOutURL', async function() {
@@ -401,13 +395,6 @@
     // handler if the user navigates directly to
     // chrome://settings/signOut. if so, it should not cause a crash.
     new ProfileInfoBrowserProxyImpl().getProfileStatsCount();
-
-    // Close the disconnect dialog.
-    peoplePage.shadowRoot!.querySelector('settings-signout-dialog')!.$
-        .disconnectConfirm.click();
-    await new Promise(function(resolve) {
-      listenOnce(window, 'popstate', resolve);
-    });
   });
 
   test('Signout dialog suppressed when not signed in', async function() {