tor-browser

commit 2d38cd5bd7d7958137dd5b4f97aef2f7185a0eb1
parent 67a43911bcf923cc909c683b685a3b36615d56a6
Author: Jens Stutte <jstutte@mozilla.com>
Date:   Wed,  8 Oct 2025 16:31:41 +0000

Bug 1991853 - Make InfoBar's "domwindowopened" handling more robust. r=omc-reviewers,mviar

Differential Revision: https://phabricator.services.mozilla.com/D267556

Diffstat:
M
browser/components/asrouter/modules/InfoBar.sys.mjs
 | 
21
++++++++++++++-------
M
browser/components/asrouter/tests/browser/browser_asrouter_universal_infobar.js
 | 
9
++++++++-

2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/browser/components/asrouter/modules/InfoBar.sys.mjs b/browser/components/asrouter/modules/InfoBar.sys.mjs
@@ -464,13 +464,9 @@ class InfoBarNotification {
    */
   removeUniversalInfobars() {
     // Remove the new window observer
-    try {
+    if (InfoBar._observingWindowOpened) {
+      InfoBar._observingWindowOpened = false;
       Services.obs.removeObserver(InfoBar, "domwindowopened");
-    } catch (error) {
-      console.error(
-        "Error removing domwindowopened observer on InfoBar: ",
-        error
-      );
     }
     // Remove the universal infobar
     InfoBar._universalInfobars.forEach(({ box, notification }) => {
@@ -504,6 +500,7 @@ class InfoBarNotification {
 export const InfoBar = {
   _activeInfobar: null,
   _universalInfobars: [],
+  _observingWindowOpened: false,
 
   maybeLoadCustomElement(win) {
     if (!win.customElements.get("remote-text")) {
@@ -624,7 +621,17 @@ export const InfoBar = {
 
     if (isFirstUniversal) {
       await this.showNotificationAllWindows(notification);
-      Services.obs.addObserver(this, "domwindowopened");
+      if (!this._observingWindowOpened) {
+        this._observingWindowOpened = true;
+        Services.obs.addObserver(this, "domwindowopened");
+      } else {
+        // TODO: At least during testing it seems that we can get here more
+        // than once without passing through removeUniversalInfobars(). Is
+        // this expected?
+        console.warn(
+          "InfoBar: Already observing new windows for universal infobar."
+        );
+      }
     } else {
       await notification.showNotification(browser);
     }
diff --git a/browser/components/asrouter/tests/browser/browser_asrouter_universal_infobar.js b/browser/components/asrouter/tests/browser/browser_asrouter_universal_infobar.js
@@ -28,9 +28,15 @@ const UNIVERSAL_MESSAGE = {
   },
 };
 
+// TODO: It might be cleaner to have a testing version of
+// removeUniversalInfobars defined on InfoBar that does this cleanup?
 const cleanupInfobars = () => {
   InfoBar._universalInfobars = [];
   InfoBar._activeInfobar = null;
+  if (InfoBar._observingWindowOpened) {
+    InfoBar._observingWindowOpened = false;
+    Services.obs.removeObserver(InfoBar, "domwindowopened");
+  }
 };
 
 const makeFakeWin = ({
@@ -303,8 +309,9 @@ add_task(async function removeObserver_on_removeUniversalInfobars() {
     "removeObserver was invoked for domwindowopened"
   );
 
-  // Cleanup
+  // Cleanup. Make sure we remove the observer that removeSpy left behind.
   Services.obs = origObs;
+  InfoBar._observingWindowOpened = true;
   cleanupInfobars();
   sandbox.restore();
 });