Bug 1637059 fix persistent listeners when updating permissions r=rpl
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 11 Dec 2020 21:30:05 +0000
changeset 560394 92a8af8f6853143612cd0c18bee7c2694a2198f2
parent 560393 5f82b68a19d5f55f003f6106e4a42db25c51ceee
child 560395 6deeaf8a7a2eba95e0164517d382a79f13910131
push id132627
push userscaraveo@mozilla.com
push dateFri, 11 Dec 2020 21:33:10 +0000
treeherderautoland@92a8af8f6853 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl
bugs1637059
milestone85.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 1637059 fix persistent listeners when updating permissions r=rpl Differential Revision: https://phabricator.services.mozilla.com/D85790
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/test/xpcshell/test_ext_webRequest_startup.js
toolkit/mozapps/extensions/internal/XPIDatabase.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -2279,17 +2279,19 @@ class EventManager {
   // about all primed listeners in the extension's persistentListeners Map.
   static primeListeners(extension) {
     if (!EventManager._initPersistentListeners(extension)) {
       return;
     }
 
     for (let [module, moduleEntry] of extension.persistentListeners) {
       let api = extension.apiManager.getAPI(module, extension, "addon_parent");
-      if (!api.primeListener) {
+      // If an extension is upgraded and a permission, such as webRequest, is
+      // removed, we will have been called but the API is no longer available.
+      if (!api?.primeListener) {
         // The runtime module no longer implements primed listeners, drop them.
         extension.persistentListeners.delete(module);
         EventManager._writePersistentListeners(extension);
         continue;
       }
       for (let [event, eventEntry] of moduleEntry) {
         for (let listener of eventEntry.values()) {
           let primed = { pendingEvents: [] };
--- a/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_startup.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_startup.js
@@ -1,58 +1,98 @@
 "use strict";
 
+// Delay loading until createAppInfo is called and setup.
+ChromeUtils.defineModuleGetter(
+  this,
+  "AddonManager",
+  "resource://gre/modules/AddonManager.jsm"
+);
+
 AddonTestUtils.init(this);
 AddonTestUtils.overrideCertDB();
+
+// The app and platform version here should be >= of the version set in the extensions.webExtensionsMinPlatformVersion preference,
+// otherwise test_persistent_listener_after_staged_update will fail because no compatible updates will be found.
 AddonTestUtils.createAppInfo(
   "xpcshell@tests.mozilla.org",
   "XPCShell",
-  "1",
-  "43"
+  "42",
+  "42"
 );
 
 let {
   promiseRestartManager,
   promiseShutdownManager,
   promiseStartupManager,
 } = AddonTestUtils;
 
 const server = createHttpServer({ hosts: ["example.com"] });
 server.registerDirectory("/data/", do_get_file("data"));
 
+let scopes = AddonManager.SCOPE_PROFILE | AddonManager.SCOPE_APPLICATION;
+Services.prefs.setIntPref("extensions.enabledScopes", scopes);
+
 Services.prefs.setBoolPref(
   "extensions.webextensions.background-delayed-startup",
   true
 );
 
 function trackEvents(wrapper) {
   let events = new Map();
   for (let event of ["background-page-event", "start-background-page"]) {
     events.set(event, false);
     wrapper.extension.once(event, () => events.set(event, true));
   }
   return events;
 }
 
+async function testPersistentRequestStartup(extension, events, expect) {
+  equal(
+    events.get("background-page-event"),
+    expect.background,
+    "Should have gotten a background page event"
+  );
+  equal(
+    events.get("start-background-page"),
+    false,
+    "Background page should not be started"
+  );
+
+  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
+  await ExtensionParent.browserPaintedPromise;
+
+  equal(
+    events.get("start-background-page"),
+    expect.delayedStart,
+    "Should have gotten start-background-page event"
+  );
+
+  if (expect.request) {
+    await extension.awaitMessage("got-request");
+    ok(true, "Background page loaded and received webRequest event");
+  }
+}
+
 // Test that a non-blocking listener during startup does not immediately
 // start the background page, but the event is queued until the background
 // page is started.
 add_task(async function test_1() {
   await promiseStartupManager();
 
   let extension = ExtensionTestUtils.loadExtension({
     useAddonManager: "permanent",
     manifest: {
       permissions: ["webRequest", "http://example.com/"],
     },
 
     background() {
       browser.webRequest.onBeforeRequest.addListener(
         details => {
-          browser.test.sendMessage("saw-request");
+          browser.test.sendMessage("got-request");
         },
         { urls: ["http://example.com/data/file_sample.html"] }
       );
     },
   });
 
   await extension.startup();
 
@@ -61,38 +101,21 @@ add_task(async function test_1() {
 
   let events = trackEvents(extension);
 
   await ExtensionTestUtils.fetch(
     "http://example.com/",
     "http://example.com/data/file_sample.html"
   );
 
-  equal(
-    events.get("background-page-event"),
-    true,
-    "Should have gotten a background page event"
-  );
-  equal(
-    events.get("start-background-page"),
-    false,
-    "Background page should not be started"
-  );
-
-  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
-  await new Promise(executeSoon);
-
-  equal(
-    events.get("start-background-page"),
-    true,
-    "Should have gotten start-background-page event"
-  );
-
-  await extension.awaitMessage("saw-request");
-  ok(true, "Background page loaded and received webRequest event");
+  await testPersistentRequestStartup(extension, events, {
+    background: true,
+    delayedStart: true,
+    request: true,
+  });
 
   await extension.unload();
 
   await promiseShutdownManager();
 });
 
 // Tests that filters are handled properly: if we have a blocking listener
 // with a filter, a request that does not match the filter does not get
@@ -131,30 +154,21 @@ add_task(async function test_2() {
 
   let events = trackEvents(extension);
 
   await ExtensionTestUtils.fetch(
     "http://example.com/",
     "http://example.com/data/file_sample.html"
   );
 
-  equal(
-    events.get("background-page-event"),
-    false,
-    "Should not have gotten a background page event"
-  );
-
-  Services.obs.notifyObservers(null, "browser-delayed-startup-finished");
-  await new Promise(executeSoon);
-
-  equal(
-    events.get("start-background-page"),
-    false,
-    "Should not have tried to start background page yet"
-  );
+  await testPersistentRequestStartup(extension, events, {
+    background: false,
+    delayedStart: false,
+    request: false,
+  });
 
   Services.obs.notifyObservers(null, "sessionstore-windows-restored");
   await extension.awaitMessage("ready");
 
   await extension.unload();
   await promiseShutdownManager();
 });
 
@@ -212,8 +226,438 @@ add_task(async function test_3() {
     data,
     DATA,
     "Stream filter was properly installed for a load during startup"
   );
 
   await extension.unload();
   await promiseShutdownManager();
 });
+
+// Tests that moving permission to optional retains permission and that the
+// persistent listeners are used as expected.
+add_task(async function test_persistent_listener_after_sideload_upgrade() {
+  let id = "permission-sideload-upgrade@test";
+  let extensionData = {
+    useAddonManager: "permanent",
+    manifest: {
+      version: "1.0",
+      applications: { gecko: { id } },
+      permissions: ["webRequest", "http://example.com/"],
+    },
+
+    background() {
+      browser.webRequest.onBeforeRequest.addListener(
+        details => {
+          browser.test.sendMessage("got-request");
+        },
+        { urls: ["http://example.com/data/file_sample.html"] }
+      );
+    },
+  };
+  let xpi = AddonTestUtils.createTempWebExtensionFile(extensionData);
+
+  let extension = ExtensionTestUtils.expectExtension(id);
+  await AddonTestUtils.manuallyInstall(xpi);
+  await promiseStartupManager();
+  await extension.awaitStartup();
+
+  await ExtensionTestUtils.fetch(
+    "http://example.com/",
+    "http://example.com/data/file_sample.html"
+  );
+  await extension.awaitMessage("got-request");
+
+  await promiseShutdownManager();
+
+  // Prepare a sideload update for the extension.
+  extensionData.manifest.version = "2.0";
+  extensionData.manifest.permissions = ["http://example.com/"];
+  extensionData.manifest.optional_permissions = ["webRequest"];
+  xpi = AddonTestUtils.createTempWebExtensionFile(extensionData);
+  await AddonTestUtils.manuallyInstall(xpi);
+
+  ExtensionParent._resetStartupPromises();
+  await promiseStartupManager();
+  await extension.awaitStartup();
+  let events = trackEvents(extension);
+
+  // Verify webRequest permission.
+  let policy = WebExtensionPolicy.getByID(id);
+  ok(policy.hasPermission("webRequest"), "addon webRequest permission added");
+
+  await ExtensionTestUtils.fetch(
+    "http://example.com/",
+    "http://example.com/data/file_sample.html"
+  );
+
+  await testPersistentRequestStartup(extension, events, {
+    background: true,
+    delayedStart: true,
+    request: true,
+  });
+
+  await extension.unload();
+  await promiseShutdownManager();
+});
+
+// Utility to install builtin addon
+async function installBuiltinExtension(extensionData) {
+  let xpi = await AddonTestUtils.createTempWebExtensionFile(extensionData);
+
+  // The built-in location requires a resource: URL that maps to a
+  // jar: or file: URL.  This would typically be something bundled
+  // into omni.ja but for testing we just use a temp file.
+  let base = Services.io.newURI(`jar:file:${xpi.path}!/`);
+  let resProto = Services.io
+    .getProtocolHandler("resource")
+    .QueryInterface(Ci.nsIResProtocolHandler);
+  resProto.setSubstitution("ext-test", base);
+  return AddonManager.installBuiltinAddon("resource://ext-test/");
+}
+
+function promisePostponeInstall(install) {
+  return new Promise((resolve, reject) => {
+    let listener = {
+      onInstallFailed: () => {
+        install.removeListener(listener);
+        reject(new Error("extension installation should not have failed"));
+      },
+      onInstallEnded: () => {
+        install.removeListener(listener);
+        reject(
+          new Error(
+            `extension installation should not have ended for ${install.addon.id}`
+          )
+        );
+      },
+      onInstallPostponed: () => {
+        install.removeListener(listener);
+        resolve();
+      },
+    };
+
+    install.addListener(listener);
+    install.install();
+  });
+}
+
+// Tests that moving permission to optional retains permission and that the
+// persistent listeners are used as expected.
+add_task(
+  async function test_persistent_listener_after_builtin_location_upgrade() {
+    let id = "permission-builtin-upgrade@test";
+    let extensionData = {
+      useAddonManager: "permanent",
+      manifest: {
+        version: "1.0",
+        applications: { gecko: { id } },
+        permissions: ["webRequest", "http://example.com/"],
+      },
+
+      async background() {
+        browser.runtime.onUpdateAvailable.addListener(() => {
+          browser.test.sendMessage("postponed");
+        });
+
+        browser.webRequest.onBeforeRequest.addListener(
+          details => {
+            browser.test.sendMessage("got-request");
+          },
+          { urls: ["http://example.com/data/file_sample.html"] }
+        );
+      },
+    };
+    await promiseStartupManager();
+    // If we use an extension wrapper via ExtensionTestUtils.expectExtension
+    // it will continue to handle messages even after the update, resulting
+    // in errors when it receives additional messages without any awaitMessage.
+    let promiseExtension = AddonTestUtils.promiseWebExtensionStartup(id);
+    await installBuiltinExtension(extensionData);
+    let extv1 = await promiseExtension;
+
+    // Prepare an update for the extension.
+    extensionData.manifest.version = "2.0";
+    let xpi = AddonTestUtils.createTempWebExtensionFile(extensionData);
+    let install = await AddonManager.getInstallForFile(xpi);
+
+    // Install the update and wait for the onUpdateAvailable event to complete.
+    let promiseUpdate = new Promise(resolve =>
+      extv1.once("test-message", (kind, msg) => {
+        if (msg == "postponed") {
+          resolve();
+        }
+      })
+    );
+    await Promise.all([promisePostponeInstall(install), promiseUpdate]);
+    await promiseShutdownManager();
+
+    // restarting allows upgrade to proceed
+    ExtensionParent._resetStartupPromises();
+    let extension = ExtensionTestUtils.expectExtension(id);
+    await promiseStartupManager();
+    await extension.awaitStartup();
+    let events = trackEvents(extension);
+
+    await ExtensionTestUtils.fetch(
+      "http://example.com/",
+      "http://example.com/data/file_sample.html"
+    );
+
+    await testPersistentRequestStartup(extension, events, {
+      background: true,
+      delayedStart: true,
+      request: true,
+    });
+
+    await extension.unload();
+
+    // remove the builtin addon which will have restarted now.
+    let addon = await AddonManager.getAddonByID(id);
+    await addon.uninstall();
+
+    await promiseShutdownManager();
+  }
+);
+
+// Tests that moving permission to optional during a staged upgrade retains permission
+// and that the persistent listeners are used as expected.
+add_task(async function test_persistent_listener_after_staged_upgrade() {
+  AddonManager.checkUpdateSecurity = false;
+  let id = "persistent-staged-upgrade@test";
+
+  // register an update file.
+  AddonTestUtils.registerJSON(server, "/test_update.json", {
+    addons: {
+      "persistent-staged-upgrade@test": {
+        updates: [
+          {
+            version: "2.0",
+            update_link:
+              "http://example.com/addons/test_settings_staged_restart.xpi",
+          },
+        ],
+      },
+    },
+  });
+
+  let extensionData = {
+    useAddonManager: "permanent",
+    manifest: {
+      version: "2.0",
+      applications: {
+        gecko: { id, update_url: `http://example.com/test_update.json` },
+      },
+      permissions: ["http://example.com/"],
+      optional_permissions: ["webRequest"],
+    },
+
+    background() {
+      browser.webRequest.onBeforeRequest.addListener(
+        details => {
+          browser.test.sendMessage("got-request");
+        },
+        { urls: ["http://example.com/data/file_sample.html"] }
+      );
+      // Force a staged updated.
+      browser.runtime.onUpdateAvailable.addListener(async details => {
+        if (details && details.version) {
+          // This should be the version of the pending update.
+          browser.test.assertEq("2.0", details.version, "correct version");
+          browser.test.sendMessage("delay");
+        }
+      });
+    },
+  };
+
+  // Prepare the update first.
+  server.registerFile(
+    `/addons/test_settings_staged_restart.xpi`,
+    AddonTestUtils.createTempWebExtensionFile(extensionData)
+  );
+
+  // Prepare the extension that will be updated.
+  extensionData.manifest.version = "1.0";
+  extensionData.manifest.permissions = ["webRequest", "http://example.com/"];
+  delete extensionData.manifest.optional_permissions;
+
+  await promiseStartupManager();
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  await extension.startup();
+
+  await ExtensionTestUtils.fetch(
+    "http://example.com/",
+    "http://example.com/data/file_sample.html"
+  );
+  await extension.awaitMessage("got-request");
+  ok(true, "Initial version received webRequest event");
+
+  let addon = await AddonManager.getAddonByID(id);
+  Assert.equal(addon.version, "1.0", "1.0 is loaded");
+
+  let update = await AddonTestUtils.promiseFindAddonUpdates(addon);
+  let install = update.updateAvailable;
+  Assert.ok(install, `install is available ${update.error}`);
+
+  await AddonTestUtils.promiseCompleteAllInstalls([install]);
+
+  Assert.equal(
+    install.state,
+    AddonManager.STATE_POSTPONED,
+    "update is staged for install"
+  );
+  await extension.awaitMessage("delay");
+
+  await promiseShutdownManager();
+
+  // restarting allows upgrade to proceed
+  ExtensionParent._resetStartupPromises();
+  await promiseStartupManager();
+  await extension.awaitStartup();
+  let events = trackEvents(extension);
+
+  // Verify webRequest permission.
+  let policy = WebExtensionPolicy.getByID(id);
+  ok(policy.hasPermission("webRequest"), "addon webRequest permission added");
+
+  await ExtensionTestUtils.fetch(
+    "http://example.com/",
+    "http://example.com/data/file_sample.html"
+  );
+
+  await testPersistentRequestStartup(extension, events, {
+    background: true,
+    delayedStart: true,
+    request: true,
+  });
+
+  await extension.unload();
+  await promiseShutdownManager();
+  AddonManager.checkUpdateSecurity = true;
+});
+
+// Tests that removing the permission releases the persistent listener.
+add_task(async function test_persistent_listener_after_permission_removal() {
+  let id = "persistent-staged-remove@test";
+
+  // register an update file.
+  AddonTestUtils.registerJSON(server, "/test_remove.json", {
+    addons: {
+      "persistent-staged-remove@test": {
+        updates: [
+          {
+            version: "2.0",
+            update_link:
+              "http://example.com/addons/test_settings_staged_remove.xpi",
+          },
+        ],
+      },
+    },
+  });
+
+  let extensionData = {
+    useAddonManager: "permanent",
+    manifest: {
+      version: "2.0",
+      applications: {
+        gecko: { id, update_url: `http://example.com/test_remove.json` },
+      },
+      permissions: ["tabs", "http://example.com/"],
+    },
+
+    background() {
+      browser.test.sendMessage("loaded");
+    },
+  };
+
+  // Prepare the update first.
+  server.registerFile(
+    `/addons/test_settings_staged_remove.xpi`,
+    AddonTestUtils.createTempWebExtensionFile(extensionData)
+  );
+
+  await promiseStartupManager();
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "permanent",
+    manifest: {
+      version: "1.0",
+      applications: {
+        gecko: { id, update_url: `http://example.com/test_remove.json` },
+      },
+      permissions: ["webRequest", "http://example.com/"],
+    },
+
+    background() {
+      browser.webRequest.onBeforeRequest.addListener(
+        details => {
+          browser.test.sendMessage("got-request");
+        },
+        { urls: ["http://example.com/data/file_sample.html"] }
+      );
+      // Force a staged updated.
+      browser.runtime.onUpdateAvailable.addListener(async details => {
+        if (details && details.version) {
+          // This should be the version of the pending update.
+          browser.test.assertEq("2.0", details.version, "correct version");
+          browser.test.sendMessage("delay");
+        }
+      });
+    },
+  });
+
+  await extension.startup();
+
+  await ExtensionTestUtils.fetch(
+    "http://example.com/",
+    "http://example.com/data/file_sample.html"
+  );
+  await extension.awaitMessage("got-request");
+  ok(true, "Initial version received webRequest event");
+
+  let addon = await AddonManager.getAddonByID(id);
+  Assert.equal(addon.version, "1.0", "1.0 is loaded");
+
+  let update = await AddonTestUtils.promiseFindAddonUpdates(addon);
+  let install = update.updateAvailable;
+  Assert.ok(install, `install is available ${update.error}`);
+
+  await AddonTestUtils.promiseCompleteAllInstalls([install]);
+
+  Assert.equal(
+    install.state,
+    AddonManager.STATE_POSTPONED,
+    "update is staged for install"
+  );
+  await extension.awaitMessage("delay");
+
+  await promiseShutdownManager();
+
+  // restarting allows upgrade to proceed
+  await promiseStartupManager();
+  let events = trackEvents(extension);
+  await extension.awaitStartup();
+
+  // Verify webRequest permission.
+  let policy = WebExtensionPolicy.getByID(id);
+  ok(
+    !policy.hasPermission("webRequest"),
+    "addon webRequest permission removed"
+  );
+
+  await ExtensionTestUtils.fetch(
+    "http://example.com/",
+    "http://example.com/data/file_sample.html"
+  );
+
+  await testPersistentRequestStartup(extension, events, {
+    background: false,
+    delayedStart: false,
+    request: false,
+  });
+
+  Services.obs.notifyObservers(null, "sessionstore-windows-restored");
+
+  await extension.awaitMessage("loaded");
+  ok(true, "Background page loaded");
+
+  await extension.unload();
+  await promiseShutdownManager();
+});
--- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -2923,19 +2923,18 @@ this.XPIDatabaseReconcile = {
    *        The install location containing the add-on
    * @param {AddonInternal} aOldAddon
    *        The AddonInternal as it appeared the last time the application
    *        ran
    * @param {XPIState} aAddonState
    *        The new state of the add-on
    * @param {AddonInternal?} [aNewAddon]
    *        The manifest for the new add-on if it has already been loaded
-   * @returns {boolean?}
-   *        A boolean indicating if flushing caches is required to complete
-   *        changing this add-on
+   * @returns {AddonInternal}
+   *        The AddonInternal that was added to the database
    */
   updateMetadata(aLocation, aOldAddon, aAddonState, aNewAddon) {
     logger.debug(`Add-on ${aOldAddon.id} modified in ${aLocation.name}`);
 
     try {
       // If there isn't an updated install manifest for this add-on then load it.
       if (!aNewAddon) {
         aNewAddon = XPIInstall.syncLoadManifest(
@@ -2969,16 +2968,18 @@ this.XPIDatabaseReconcile = {
       }
 
       return null;
     }
 
     // Set the additional properties on the new AddonInternal
     aNewAddon.updateDate = aAddonState.mtime;
 
+    XPIProvider.persistStartupData(aNewAddon, aAddonState);
+
     // Update the database
     return XPIDatabase.updateAddonMetadata(
       aOldAddon,
       aNewAddon,
       aAddonState.path
     );
   },
 
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -774,16 +774,18 @@ class XPIStateLocation extends Map {
    *        The DBAddon to add.
    */
   addAddon(addon) {
     logger.debug(
       "XPIStates adding add-on ${id} in ${location}: ${path}",
       addon
     );
 
+    XPIProvider.persistStartupData(addon);
+
     let xpiState = this._addState(addon.id, { file: addon._sourceBundle });
     xpiState.syncWithDB(addon, true);
 
     XPIProvider.addTelemetry(addon.id, { location: this.name });
   }
 
   /**
    * Remove the XPIState for an add-on and save the new state.
@@ -3104,16 +3106,35 @@ var XPIProvider = {
    *        The data to store.  Must be JSON serializable.
    */
   setStartupData(aID, aData) {
     let state = XPIStates.findAddon(aID);
     state.startupData = aData;
     XPIStates.save();
   },
 
+  /**
+   * Persists some startupData into an addon if it is available in the current
+   * XPIState for the addon id.
+   *
+   * @param {AddonInternal} addon An addon to receive the startup data, typically an update that is occuring.
+   * @param {XPIState} state optional
+   */
+  persistStartupData(addon, state) {
+    if (!addon.startupData) {
+      state = state || XPIStates.findAddon(addon.id);
+      if (state?.enabled) {
+        // Save persistent listener data if available.  It will be
+        // removed later if necessary.
+        let persistentListeners = state.startupData?.persistentListeners;
+        addon.startupData = { persistentListeners };
+      }
+    }
+  },
+
   getAddonIDByInstanceID(aInstanceID) {
     if (!aInstanceID || typeof aInstanceID != "symbol") {
       throw Components.Exception(
         "aInstanceID must be a Symbol()",
         Cr.NS_ERROR_INVALID_ARG
       );
     }