Bug 1803255 - Fix progress notifications for WebExtensions. r=aleca

This changes browser_ext_tabs_events.js to stop expecting page load events when switching tabs. It also fixes the case where about:message loads after `tabmail.openTab` returns, or where it's in a standalone message window. Differential Revision: https://phabricator.services.mozilla.com/D164170 --HG-- extra : rebase_source : cb16c3a1915ec763130d84a9f6cf1d06fbcb3979 extra : absorb_source : fad5000f79cb3d38466c6d3362d83663f1683ca2
2022-12-05 12:59:47 +13:00 · 2022-12-05 12:59:47 +13:00 · 5489dfbd9e
--- a/mail/base/content/aboutMessage.js
+++ b/mail/base/content/aboutMessage.js
@ -82,6 +82,10 @@ window.addEventListener("DOMContentLoaded", event => {
  }

  preferenceObserver.init();
+
+  window.dispatchEvent(
+    new CustomEvent("aboutMessageLoaded", { bubbles: true })
+  );
 });

 window.addEventListener("unload", () => {
@ -147,12 +151,6 @@ function displayMessage(uri, viewWrapper) {
  MailE10SUtils.changeRemoteness(content, null);
  content.docShell.allowAuth = false;
  content.docShell.allowDNSPrefetch = false;
-  content.docShell
-    ?.QueryInterface(Ci.nsIWebProgress)
-    .addProgressListener(
-      top.msgWindow.statusFeedback,
-      Ci.nsIWebProgress.NOTIFY_ALL
-    );

  messageService.DisplayMessage(uri, content.docShell, null, null, null, {});

--- a/mail/base/content/mailWindow.js
+++ b/mail/base/content/mailWindow.js
@ -948,3 +948,107 @@ function switchToTabHavingURI(aURI, aOpenNew, aOpenParams = {}) {

  return false;
 }
+
+/**
+ * Combines all nsIWebProgress notifications from all content browsers in this
+ * window and reports them to the registered listeners.
+ *
+ * @see WindowTracker (ext-mail.js)
+ * @see StatusListener, WindowTrackerBase (ext-tabs-base.js)
+ */
+var contentProgress = {
+  _listeners: new Set(),
+
+  addListener(listener) {
+    this._listeners.add(listener);
+  },
+
+  removeListener(listener) {
+    this._listeners.delete(listener);
+  },
+
+  callListeners(method, args) {
+    for (let listener of this._listeners.values()) {
+      if (method in listener) {
+        try {
+          listener[method](...args);
+        } catch (e) {
+          console.error(e);
+        }
+      }
+    }
+  },
+
+  /**
+   * Ensure that `browser` has a ProgressListener attached to it.
+   *
+   * @param {Browser} browser
+   */
+  addProgressListenerToBrowser(browser) {
+    if (browser?.webProgress && !browser._progressListener) {
+      browser._progressListener = new contentProgress.ProgressListener(browser);
+      browser.webProgress.addProgressListener(
+        browser._progressListener,
+        Ci.nsIWebProgress.NOTIFY_ALL
+      );
+    }
+  },
+
+  // @implements {nsIWebProgressListener}
+  // @implements {nsIWebProgressListener2}
+  ProgressListener: class {
+    QueryInterface = ChromeUtils.generateQI([
+      "nsIWebProgressListener",
+      "nsIWebProgressListener2",
+      "nsISupportsWeakReference",
+    ]);
+
+    constructor(browser) {
+      this.browser = browser;
+    }
+
+    callListeners(method, args) {
+      args.unshift(this.browser);
+      contentProgress.callListeners(method, args);
+    }
+
+    onProgressChange(...args) {
+      this.callListeners("onProgressChange", args);
+    }
+
+    onProgressChange64(...args) {
+      this.callListeners("onProgressChange64", args);
+    }
+
+    onLocationChange(...args) {
+      this.callListeners("onLocationChange", args);
+    }
+
+    onStateChange(...args) {
+      this.callListeners("onStateChange", args);
+    }
+
+    onStatusChange(...args) {
+      this.callListeners("onStatusChange", args);
+    }
+
+    onSecurityChange(...args) {
+      this.callListeners("onSecurityChange", args);
+    }
+
+    onContentBlockingEvent(...args) {
+      this.callListeners("onContentBlockingEvent", args);
+    }
+
+    onRefreshAttempted(...args) {
+      return this.callListeners("onRefreshAttempted", args);
+    }
+  },
+};
+
+// Add a progress listener to any about:message content browser that comes
+// along. This often happens after the tab is opened so the usual mechanism
+// doesn't work. It also works for standalone message windows.
+window.addEventListener("aboutMessageLoaded", event =>
+  contentProgress.addProgressListenerToBrowser(event.target.content)
+);
--- a/mail/base/content/tabmail.js
+++ b/mail/base/content/tabmail.js
@ -482,7 +482,6 @@ var { UIFontSize } = ChromeUtils.import("resource:///modules/UIFontSize.jsm");
      this.tabMonitors = [];
      this.recentlyClosedTabs = [];
      this.mLastTabOpener = null;
-      this.mTabsProgressListeners = new Set();
      this.unrestoredTabs = [];

      // @implements {nsIController}
@ -898,9 +897,6 @@ var { UIFontSize } = ChromeUtils.import("resource:///modules/UIFontSize.jsm");
            tab.linkedBrowser = browser;
          }
        }
-        if (tab.browser && !background) {
-          tab.browser._activeTabId = tab.tabId;
-        }

        let restoreState = this._restoringTabState;
        for (let tabMonitor of this.tabMonitors) {
@ -948,17 +944,7 @@ var { UIFontSize } = ChromeUtils.import("resource:///modules/UIFontSize.jsm");
        t.dispatchEvent(evt);
        delete tab.beforeTabOpen;

-        // Register browser progress listeners
-        if (browser && browser.webProgress && !browser._progressListener) {
-          // It would probably be better to have the tabs register this listener, since the
-          // browser can change. This wasn't trivial to do while implementing basic WebExtension
-          // support, so let's assume one browser only for now.
-          browser._progressListener = new TabProgressListener(browser, this);
-          browser.webProgress.addProgressListener(
-            browser._progressListener,
-            Ci.nsIWebProgress.NOTIFY_ALL
-          );
-        }
+        contentProgress.addProgressListenerToBrowser(browser);

        return tab;
      } catch (e) {
@ -1543,13 +1529,8 @@ var { UIFontSize } = ChromeUtils.import("resource:///modules/UIFontSize.jsm");
    }

    getTabForBrowser(aBrowser) {
-      // Tabs from the "mail" type share the same browser. Return the active
-      // one, if possible.
-      if (
-        aBrowser &&
-        aBrowser.id == "messagepane" &&
-        this.selectedTab.mode.tabType.name == "mail"
-      ) {
+      // Check the selected browser first, since that's the most likely.
+      if (this.getBrowserForSelectedTab() == aBrowser) {
        return this.currentTabInfo;
      }
      for (let tabInfo of this.tabInfo) {
@ -1618,9 +1599,6 @@ var { UIFontSize } = ChromeUtils.import("resource:///modules/UIFontSize.jsm");
            tab.linkedBrowser = browser;
          }
        }
-        if (tab.browser) {
-          tab.browser._activeTabId = tab.tabId;
-        }

        for (let tabMonitor of this.tabMonitors) {
          try {
@ -1846,30 +1824,6 @@ var { UIFontSize } = ChromeUtils.import("resource:///modules/UIFontSize.jsm");
      document.title = docTitle;
    }

-    addTabsProgressListener(aListener) {
-      this.mTabsProgressListeners.add(aListener);
-    }
-
-    removeTabsProgressListener(aListener) {
-      this.mTabsProgressListeners.delete(aListener);
-    }
-
-    _callTabListeners(aMethod, aArgs) {
-      let rv = true;
-      for (let listener of this.mTabsProgressListeners.values()) {
-        if (aMethod in listener) {
-          try {
-            if (!listener[aMethod](...aArgs)) {
-              rv = false;
-            }
-          } catch (e) {
-            console.error(e);
-          }
-        }
-      }
-      return rv;
-    }
-
    // Called by <browser>, unused by tabmail.
    finishBrowserRemotenessChange(browser, loadSwitchId) {}

@ -1894,56 +1848,6 @@ var { UIFontSize } = ChromeUtils.import("resource:///modules/UIFontSize.jsm");
  }

  customElements.define("tabmail", MozTabmail);
-
-  // @implements {nsIWebProgressListener}
-  class TabProgressListener {
-    constructor(browser, tabmail) {
-      this.browser = browser;
-      this.tabmail = tabmail;
-    }
-
-    _callTabListeners(method, args) {
-      args.unshift(this.browser);
-      this.tabmail._callTabListeners(method, args);
-    }
-
-    onProgressChange(...args) {
-      this._callTabListeners("onProgressChange", args);
-    }
-
-    onProgressChange64(...args) {
-      this._callTabListeners("onProgressChange64", args);
-    }
-
-    onLocationChange(...args) {
-      this._callTabListeners("onLocationChange", args);
-    }
-
-    onStateChange(...args) {
-      this._callTabListeners("onStateChange", args);
-    }
-
-    onStatusChange(...args) {
-      this._callTabListeners("onStatusChange", args);
-    }
-
-    onSecurityChange(...args) {
-      this._callTabListeners("onSecurityChange", args);
-    }
-
-    onContentBlockingEvent(...args) {
-      this._callTabListeners("onContentBlockingEvent", args);
-    }
-
-    onRefreshAttempted(...args) {
-      return this._callTabListeners("onRefreshAttempted", args);
-    }
-  }
-  TabProgressListener.prototype.QueryInterface = ChromeUtils.generateQI([
-    "nsIWebProgressListener",
-    "nsIWebProgressListener2",
-    "nsISupportsWeakReference",
-  ]);
 }

 // Set up the tabContextMenu, which is used as the context menu for all tabmail
--- a/mail/components/extensions/parent/ext-mail.js
+++ b/mail/components/extensions/parent/ext-mail.js
@ -386,9 +386,8 @@ class WindowTracker extends WindowTrackerBase {
   * @param {object} listener - The listener to add
   */
  addProgressListener(window, listener) {
-    let tabmail = window.document.getElementById("tabmail");
-    if (tabmail) {
-      tabmail.addTabsProgressListener(listener);
+    if (window.contentProgress) {
+      window.contentProgress.addListener(listener);
    }
  }

@ -399,9 +398,8 @@ class WindowTracker extends WindowTrackerBase {
   * @param {object} listener - The listener to remove
   */
  removeProgressListener(window, listener) {
-    let tabmail = window.document.getElementById("tabmail");
-    if (tabmail) {
-      tabmail.removeTabsProgressListener(listener);
+    if (window.contentProgress) {
+      window.contentProgress.removeListener(listener);
    }
  }

--- a/mail/components/extensions/parent/ext-tabs.js
+++ b/mail/components/extensions/parent/ext-tabs.js
@ -349,15 +349,13 @@ this.tabs = class extends ExtensionAPIPersistent {
      let statusListener = ({ browser, status, url }) => {
        let { extension } = this;
        let { tabManager } = extension;
-        let tabmail = browser.ownerDocument.getElementById("tabmail");
-        let nativeTabInfo = tabmail.getTabForBrowser(browser);
-        if (nativeTabInfo) {
+        let tabId = tabTracker.getBrowserTabId(browser);
+        if (tabId != -1) {
          let changed = { status };
          if (url) {
            changed.url = url;
          }
-
-          fireForTab(tabManager.getWrapper(nativeTabInfo), changed);
+          fireForTab(tabManager.get(tabId), changed);
        }
      };

--- a/mail/components/extensions/test/browser/browser_ext_tabs_events.js
+++ b/mail/components/extensions/test/browser/browser_ext_tabs_events.js
@ -84,7 +84,7 @@ add_task(async () => {
            }
            return actualArgs;
          },
-          async pageLoad(tab) {
+          async pageLoad(tab, active = true) {
            while (true) {
              // Read the first event without consuming it.
              let [
@ -114,7 +114,7 @@ add_task(async () => {
              {
                id: tab,
                windowId: initialWindow,
-                active: true,
+                active,
                mailTab: false,
              }
            );
@ -322,14 +322,6 @@ add_task(async () => {
          )
        );

-        // In some circumstances this onUpdated event and the onCreated event
-        // happen out of order. We're not interested in the onUpdated event
-        // so just throw it away.
-        let unwantedEvent = await listener.nextEvent();
-        if (unwantedEvent[0] == "onUpdated") {
-          listener.events.shift();
-        }
-
        let [{ id: messageTab1 }] = await listener.checkEvent("onCreated", {
          index: 6,
          windowId: initialWindow,
@ -385,6 +377,7 @@ add_task(async () => {
            messageTab1,
          ].includes(messageTab2)
        );
+        await listener.pageLoad(messageTab2, false);

        browser.test.log(
          "Activate each of the tabs in a somewhat random order to test the onActivated event."
@ -406,26 +399,10 @@ add_task(async () => {
              browser.tabs.update(tab, { active: true })
            )
          );
-          if ([messageTab1, messageTab2].includes(tab)) {
-            await listener.checkEvent(
-              "onUpdated",
-              tab,
-              { status: "loading" },
-              {
-                id: tab,
-                windowId: initialWindow,
-                active: true,
-                mailTab: false,
-              }
-            );
-          }
          await listener.checkEvent("onActivated", {
            tabId: tab,
            windowId: initialWindow,
          });
-          if ([messageTab1, messageTab2].includes(tab)) {
-            await listener.pageLoad(tab);
-          }
        }

        browser.test.log(
--- a/mail/modules/MailE10SUtils.jsm
+++ b/mail/modules/MailE10SUtils.jsm
@ -69,6 +69,10 @@ var MailE10SUtils = {
   * @param {string} remoteType
   */
  changeRemoteness(browser, remoteType) {
+    if (browser.remoteType == remoteType) {
+      return;
+    }
+
    browser.destroy();

    if (remoteType) {
--- a/mail/test/browser/content-policy/browser_generalContentPolicy.js
+++ b/mail/test/browser/content-policy/browser_generalContentPolicy.js
@ -226,20 +226,15 @@ function addMsgToFolderAndCheckContent(folder, test) {
  assert_selected_and_displayed(gMsgNo);

  // Now check that the content hasn't been loaded
+  let messageDocument = get_about_message().content.contentDocument;
  if (test.checkDenied) {
-    if (
-      test.checkForAllowed(
-        mc.window.content.document.getElementById("testelement")
-      )
-    ) {
+    if (test.checkForAllowed(messageDocument.getElementById("testelement"))) {
      throw new Error(
        test.type + " has not been blocked in message content as expected."
      );
    }
  } else if (
-    !test.checkForAllowed(
-      mc.window.content.document.getElementById("testelement")
-    )
+    !test.checkForAllowed(messageDocument.getElementById("testelement"))
  ) {
    throw new Error(
      test.type + " has been unexpectedly blocked in message content."
@ -431,11 +426,8 @@ function checkAllowFeedMsg(test) {
  assert_selected_and_displayed(gMsgNo);

  // Now check that the content hasn't been blocked
-  if (
-    !test.checkForAllowed(
-      mc.window.content.document.getElementById("testelement")
-    )
-  ) {
+  let messageDocument = get_about_message().content.contentDocument;
+  if (!test.checkForAllowed(messageDocument.getElementById("testelement"))) {
    throw new Error(
      test.type + " has been unexpectedly blocked in feed message content."
    );
@ -470,11 +462,8 @@ function checkAllowForSenderWithPerms(test) {
  assert_selected_and_displayed(gMsgNo);

  // Now check that the content hasn't been blocked
-  if (
-    !test.checkForAllowed(
-      mc.window.content.document.getElementById("testelement")
-    )
-  ) {
+  let messageDocument = get_about_message().content.contentDocument;
+  if (!test.checkForAllowed(messageDocument.getElementById("testelement"))) {
    throw new Error(
      `${test.type} has been unexpectedly blocked for sender=${authorEmailAddress}`
    );
@ -502,7 +491,9 @@ function checkAllowForHostsWithPerms(test) {
  Assert.equal(msgDbHdr, msgHdr);
  assert_selected_and_displayed(gMsgNo);

-  let src = mc.window.content.document.getElementById("testelement").src;
+  let aboutMessage = get_about_message();
+  let messageDocument = aboutMessage.content.contentDocument;
+  let src = messageDocument.getElementById("testelement").src;

  if (!src.startsWith("http")) {
    // Just test http in this test.
@ -521,11 +512,8 @@ function checkAllowForHostsWithPerms(test) {
  assert_selected_and_displayed(gMsgNo);

  // Now check that the content hasn't been blocked.
-  if (
-    !test.checkForAllowed(
-      mc.window.content.document.getElementById("testelement")
-    )
-  ) {
+  messageDocument = aboutMessage.content.contentDocument;
+  if (!test.checkForAllowed(messageDocument.getElementById("testelement"))) {
    throw new Error(
      test.type + " has been unexpectedly blocked for url=" + uri.spec
    );
@ -560,7 +548,8 @@ add_task(async function test_generalContentPolicy() {
        // We do the first test which is the one with the image.

        // Add the site to the whitelist.
-        let src = mc.window.content.document.getElementById("testelement").src;
+        let messageDocument = get_about_message().content.contentDocument;
+        let src = messageDocument.getElementById("testelement").src;

        let uri = Services.io.newURI(src);
        addPermission(uri, Services.perms.ALLOW_ACTION);