Bug 1531838 - Set up fxmonitor PanelUI only when actually showing a notification. r=johannh
authorNihanth Subramanya <nhnt11@gmail.com>
Tue, 16 Apr 2019 12:06:48 +0000
changeset 469656 ce0b0350b646
parent 469655 2058918f63f0
child 469657 735492db6be1
push id35878
push userapavel@mozilla.com
push dateTue, 16 Apr 2019 15:43:40 +0000
treeherdermozilla-central@258af4e91151 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1531838
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1531838 - Set up fxmonitor PanelUI only when actually showing a notification. r=johannh This avoids an AssertionError when loading a stylesheet in a nested event loop. See comment 15 in the bug. Differential Revision: https://phabricator.services.mozilla.com/D26971
browser/extensions/fxmonitor/privileged/FirefoxMonitor.jsm
browser/extensions/fxmonitor/privileged/subscripts/EveryWindow.jsm
--- a/browser/extensions/fxmonitor/privileged/FirefoxMonitor.jsm
+++ b/browser/extensions/fxmonitor/privileged/FirefoxMonitor.jsm
@@ -91,16 +91,18 @@ this.FirefoxMonitor = {
   // Used to enforce idempotency of delayedInit. delayedInit is
   // called in startObserving() to ensure we load our strings, etc.
   _delayedInited: false,
   async delayedInit() {
     if (this._delayedInited) {
       return;
     }
 
+    this._delayedInited = true;
+
     /* globals Preferences, RemoteSettings, fetch, btoa, XUL_NS */
     Services.scriptloader.loadSubScript(
       this.getURL("privileged/subscripts/Globals.jsm"));
 
     /* globals EveryWindow */
     Services.scriptloader.loadSubScript(
       this.getURL("privileged/subscripts/EveryWindow.jsm"));
 
@@ -142,18 +144,16 @@ this.FirefoxMonitor = {
     XPCOMUtils.defineLazyPreferenceGetter(this, "FirefoxMonitorURL",
       this.kFirefoxMonitorURLPref, this.kDefaultFirefoxMonitorURL);
 
     XPCOMUtils.defineLazyPreferenceGetter(this, "firstAlertShown",
       this.kFirstAlertShownPref, false);
 
     await this.loadStrings();
     await this.loadBreaches();
-
-    this._delayedInited = true;
   },
 
   loadStrings() {
     let l10nManifest;
     if (this.extension.rootURI instanceof Ci.nsIJARURI) {
       l10nManifest = this.extension.rootURI.JARFile
                             .QueryInterface(Ci.nsIFileURL).file;
     } else if (this.extension.rootURI instanceof Ci.nsIFileURL) {
@@ -231,72 +231,36 @@ this.FirefoxMonitor = {
   notificationsByWindow: new WeakMap(),
   panelUIsByWindow: new WeakMap(),
 
   async startObserving() {
     if (this.observerAdded) {
       return;
     }
 
-    await this.delayedInit();
-
     EveryWindow.registerCallback(
       this.kNotificationID,
       (win) => {
         if (this.notificationsByWindow.has(win)) {
           // We've already set up this window.
           return;
         }
 
         this.notificationsByWindow.set(win, new Set());
 
-        // Inject our stylesheet.
-        let DOMWindowUtils = win.windowUtils;
-        DOMWindowUtils.loadSheetUsingURIString(this.getURL("privileged/FirefoxMonitor.css"),
-                                               DOMWindowUtils.AUTHOR_SHEET);
-
-        // Setup the popup notification stuff. First, the URL bar icon:
-        let doc = win.document;
-        let notificationBox = doc.getElementById("notification-popup-box");
-        // We create a box to use as the anchor, and put an icon image
-        // inside it. This way, when we animate the icon, its scale change
-        // does not cause the popup notification to bounce due to the anchor
-        // point moving.
-        let anchorBox = doc.createElementNS(XUL_NS, "box");
-        anchorBox.setAttribute("id", `${this.kNotificationID}-notification-anchor`);
-        anchorBox.classList.add("notification-anchor-icon");
-        let img = doc.createElementNS(XUL_NS, "image");
-        img.setAttribute("role", "button");
-        img.classList.add(`${this.kNotificationID}-icon`);
-        img.style.listStyleImage = `url(${this.getURL("assets/monitor32.svg")})`;
-        anchorBox.appendChild(img);
-        notificationBox.appendChild(anchorBox);
-        img.setAttribute("tooltiptext",
-          this.getFormattedString("fxmonitor.anchorIcon.tooltiptext",
-                                  [this.getString("fxmonitor.brandName")]));
-
-        // Now, the popupnotificationcontent:
-        let parentElt = doc.defaultView.PopupNotifications.panel.parentNode;
-        let pn = doc.createElementNS(XUL_NS, "popupnotification");
-        let pnContent = doc.createElementNS(XUL_NS, "popupnotificationcontent");
-        let panelUI = new PanelUI(doc);
-        pnContent.appendChild(panelUI.box);
-        pn.appendChild(pnContent);
-        pn.setAttribute("id", `${this.kNotificationID}-notification`);
-        pn.setAttribute("hidden", "true");
-        parentElt.appendChild(pn);
-        this.panelUIsByWindow.set(win, panelUI);
-
-        // Start listening across all tabs!
-        win.gBrowser.addTabsProgressListener(this);
+        // Start listening across all tabs! The UI will
+        // be set up lazily when we actually need to show
+        // a notification.
+        this.delayedInit().then(() => {
+          win.gBrowser.addTabsProgressListener(this);
+        });
       },
-      (win) => {
-        // If the window is being destroyed and gBrowser no longer exists,
-        // don't bother doing anything.
-        if (!win.gBrowser) {
+      (win, closing) => {
+        // If the window is going away, don't bother doing anything.
+        if (closing) {
           return;
         }
 
         let DOMWindowUtils = win.windowUtils;
         DOMWindowUtils.removeSheetUsingURIString(this.getURL("privileged/FirefoxMonitor.css"),
                                                  DOMWindowUtils.AUTHOR_SHEET);
 
         this.notificationsByWindow.get(win).forEach(n => {
@@ -311,16 +275,56 @@ this.FirefoxMonitor = {
 
         win.gBrowser.removeTabsProgressListener(this);
       },
     );
 
     this.observerAdded = true;
   },
 
+  setupPanelUI(win) {
+    // Inject our stylesheet.
+    let DOMWindowUtils = win.windowUtils;
+    DOMWindowUtils.loadSheetUsingURIString(this.getURL("privileged/FirefoxMonitor.css"),
+                                           DOMWindowUtils.AUTHOR_SHEET);
+
+    // Setup the popup notification stuff. First, the URL bar icon:
+    let doc = win.document;
+    let notificationBox = doc.getElementById("notification-popup-box");
+    // We create a box to use as the anchor, and put an icon image
+    // inside it. This way, when we animate the icon, its scale change
+    // does not cause the popup notification to bounce due to the anchor
+    // point moving.
+    let anchorBox = doc.createElementNS(XUL_NS, "box");
+    anchorBox.setAttribute("id", `${this.kNotificationID}-notification-anchor`);
+    anchorBox.classList.add("notification-anchor-icon");
+    let img = doc.createElementNS(XUL_NS, "image");
+    img.setAttribute("role", "button");
+    img.classList.add(`${this.kNotificationID}-icon`);
+    img.style.listStyleImage = `url(${this.getURL("assets/monitor32.svg")})`;
+    anchorBox.appendChild(img);
+    notificationBox.appendChild(anchorBox);
+    img.setAttribute("tooltiptext",
+      this.getFormattedString("fxmonitor.anchorIcon.tooltiptext",
+                              [this.getString("fxmonitor.brandName")]));
+
+    // Now, the popupnotificationcontent:
+    let parentElt = doc.defaultView.PopupNotifications.panel.parentNode;
+    let pn = doc.createElementNS(XUL_NS, "popupnotification");
+    let pnContent = doc.createElementNS(XUL_NS, "popupnotificationcontent");
+    let panelUI = new PanelUI(doc);
+    pnContent.appendChild(panelUI.box);
+    pn.appendChild(pnContent);
+    pn.setAttribute("id", `${this.kNotificationID}-notification`);
+    pn.setAttribute("hidden", "true");
+    parentElt.appendChild(pn);
+    this.panelUIsByWindow.set(win, panelUI);
+    return panelUI;
+  },
+
   stopObserving() {
     if (!this.observerAdded) {
       return;
     }
 
     EveryWindow.unregisterCallback(this.kNotificationID);
 
     this.observerAdded = false;
@@ -350,16 +354,19 @@ this.FirefoxMonitor = {
     }
 
     this.warnedHostsSet.add(host);
     Preferences.set(this.kWarnedHostsPref, JSON.stringify([...this.warnedHostsSet]));
 
     let doc = browser.ownerDocument;
     let win = doc.defaultView;
     let panelUI = this.panelUIsByWindow.get(win);
+    if (!panelUI) {
+      panelUI = this.setupPanelUI(win);
+    }
 
     let animatedOnce = false;
     let populatePanel = (event) => {
       switch (event) {
         case "showing":
           panelUI.refresh(site);
           if (animatedOnce) {
             // If we've already animated once for this site, don't animate again.
--- a/browser/extensions/fxmonitor/privileged/subscripts/EveryWindow.jsm
+++ b/browser/extensions/fxmonitor/privileged/subscripts/EveryWindow.jsm
@@ -51,12 +51,12 @@ this.EveryWindow = {
     aWindow.addEventListener("unload",
                              this._onWindowClosing.bind(this),
                              { once: true });
   },
 
   _onWindowClosing(aEvent) {
     let win = aEvent.target;
     for (let c of this._callbacks.values()) {
-      c.uninit(win);
+      c.uninit(win, true);
     }
   },
 };