Bug 1531974 revert automatic extension permission in permanent private browsing r=rpl
authorShane Caraveo <scaraveo@mozilla.com>
Mon, 04 Mar 2019 19:00:21 +0000
changeset 520157 7df695610f7f17d3ef2c3da8aae44344320395ec
parent 520156 02e06636f9d5e6603017e5187ddebec5bc41b2fb
child 520158 b58080db50e6d16aa2e9c8e83188435353a2ecdc
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl
bugs1531974
milestone67.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 1531974 revert automatic extension permission in permanent private browsing r=rpl Differential Revision: https://phabricator.services.mozilla.com/D21937
browser/modules/ExtensionsUI.jsm
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/parent/ext-backgroundPage.js
toolkit/components/extensions/test/xpcshell/test_ext_background_private_browsing.js
--- a/browser/modules/ExtensionsUI.jsm
+++ b/browser/modules/ExtensionsUI.jsm
@@ -438,18 +438,17 @@ var ExtensionsUI = {
     let bundle = window.gNavigatorBundle;
 
     let message = bundle.getFormattedString("addonPostInstall.message1",
                                             ["<>", appName]);
     return new Promise(resolve => {
       // Show or hide private permission ui based on the pref.
       let checkbox = window.document.getElementById("addon-incognito-checkbox");
       checkbox.checked = false;
-      checkbox.hidden = allowPrivateBrowsingByDefault ||
-                        PrivateBrowsingUtils.permanentPrivateBrowsing;
+      checkbox.hidden = allowPrivateBrowsingByDefault;
 
       async function actionResolve() {
         if (checkbox.checked) {
           let perms = {permissions: ["internal:privateBrowsingAllowed"], origins: []};
           await ExtensionPermissions.add(addon.id, perms);
           // Reload the extension if it is already enabled.  This ensures any change
           // on the private browsing permission is properly handled.
           if (addon.isActive) {
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -1908,18 +1908,17 @@ class Extension extends ExtensionData {
 
       // We automatically add permissions to some extensions:
       // 1. system/built-in extensions
       // 2. all extensions when in permanent private browsing
       //
       // Extensions expliticy stating not_allowed will never get permission.
       if (!allowPrivateBrowsingByDefault && this.manifest.incognito !== "not_allowed" &&
           !this.permissions.has(PRIVATE_ALLOWED_PERMISSION)) {
-        if ((this.isPrivileged && !this.addonData.temporarilyInstalled) ||
-            (PrivateBrowsingUtils.permanentPrivateBrowsing && this.startupReason == "ADDON_INSTALL")) {
+        if (this.isPrivileged && !this.addonData.temporarilyInstalled) {
           // Add to EP so it is preserved after ADDON_INSTALL.  We don't wait on the add here
           // since we are pushing the value into this.permissions.  EP will eventually save.
           ExtensionPermissions.add(this.id, {permissions: [PRIVATE_ALLOWED_PERMISSION], origins: []});
           this.permissions.add(PRIVATE_ALLOWED_PERMISSION);
         }
       }
 
       GlobalManager.init(this);
@@ -1940,16 +1939,27 @@ class Extension extends ExtensionData {
         } else if (ExtensionStorageIDB.isMigratedExtension(this)) {
           this.setSharedData("storageIDBBackend", true);
           this.setSharedData("storageIDBPrincipal", ExtensionStorageIDB.getStoragePrincipal(this));
         }
       }
 
       resolveReadyPromise(this.policy);
 
+      // When in PPB skip any startup and disable the policy if the extension
+      // does not have permission.
+      if (PrivateBrowsingUtils.permanentPrivateBrowsing && !this.privateBrowsingAllowed) {
+        this.state = "Startup: Cancelled: (not running in permenant private browsing mode)";
+
+        this.policy.active = false;
+
+        this.cleanupGeneratedFile();
+        return;
+      }
+
       // The "startup" Management event sent on the extension instance itself
       // is emitted just before the Management "startup" event,
       // and it is used to run code that needs to be executed before
       // any of the "startup" listeners.
       this.emit("startup", this);
 
       let state = new Set(["Emit startup", "Run manifest"]);
       this.state = `Startup: ${Array.from(state)}`;
@@ -1962,17 +1972,16 @@ class Extension extends ExtensionData {
           state.delete("Run manifest");
           this.state = `Startup: ${Array.from(state)}`;
         }),
       ]);
       this.state = "Startup: Ran manifest";
 
       Management.emit("ready", this);
       this.emit("ready");
-      ExtensionTelemetry.extensionStartup.stopwatchFinish(this);
 
       this.state = "Startup: Complete";
     } catch (errors) {
       this.state = `Startup: Error: ${errors}`;
 
       for (let e of [].concat(errors)) {
         dump(`Extension error: ${e.message || e} ${e.filename || e.fileName}:${e.lineNumber} :: ${e.stack || new Error().stack}\n`);
         Cu.reportError(e);
@@ -1980,16 +1989,18 @@ class Extension extends ExtensionData {
 
       if (this.policy) {
         this.policy.active = false;
       }
 
       this.cleanupGeneratedFile();
 
       throw errors;
+    } finally {
+      ExtensionTelemetry.extensionStartup.stopwatchFinish(this);
     }
   }
 
   cleanupGeneratedFile() {
     if (!this.cleanupFile) {
       return;
     }
 
--- a/toolkit/components/extensions/parent/ext-backgroundPage.js
+++ b/toolkit/components/extensions/parent/ext-backgroundPage.js
@@ -4,16 +4,18 @@ var {ExtensionParent} = ChromeUtils.impo
 var {
   HiddenExtensionPage,
   promiseExtensionViewLoaded,
 } = ExtensionParent;
 
 
 ChromeUtils.defineModuleGetter(this, "ExtensionTelemetry",
                                "resource://gre/modules/ExtensionTelemetry.jsm");
+ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
+                               "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 XPCOMUtils.defineLazyPreferenceGetter(this, "DELAYED_STARTUP",
                                       "extensions.webextensions.background-delayed-startup");
 
 // Responsible for the background_page section of the manifest.
 class BackgroundPage extends HiddenExtensionPage {
   constructor(extension, options) {
     super(extension, "background");
@@ -86,16 +88,23 @@ this.backgroundPage = class extends Exte
     return this.bgPage.build();
   }
 
   onManifestEntry(entryName) {
     let {extension} = this;
 
     this.bgPage = null;
 
+    // When in PPB background pages all run in a private context.  This check
+    // simply avoids an extraneous error in the console since the BaseContext
+    // will throw.
+    if (PrivateBrowsingUtils.permanentPrivateBrowsing && !extension.privateBrowsingAllowed) {
+      return;
+    }
+
     if (extension.startupReason !== "APP_STARTUP" || !DELAYED_STARTUP) {
       return this.build();
     }
 
     EventManager.primeListeners(extension);
 
     extension.once("start-background-page", async () => {
       if (!this.extension) {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_background_private_browsing.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_background_private_browsing.js
@@ -1,81 +1,59 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
-const {AddonManager} = ChromeUtils.import("resource://gre/modules/AddonManager.jsm");
-const {ExtensionPermissions} = ChromeUtils.import("resource://gre/modules/ExtensionPermissions.jsm");
-
-AddonTestUtils.init(this);
-AddonTestUtils.overrideCertDB();
-AddonTestUtils.createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1");
-AddonTestUtils.usePrivilegedSignatures = false;
-
 add_task(async function test_background_incognito() {
   info("Test background page incognito value with permanent private browsing enabled");
-  await AddonTestUtils.promiseStartupManager();
 
   Services.prefs.setBoolPref("extensions.allowPrivateBrowsingByDefault", false);
   Services.prefs.setBoolPref("browser.privatebrowsing.autostart", true);
   registerCleanupFunction(() => {
     Services.prefs.clearUserPref("browser.privatebrowsing.autostart");
     Services.prefs.clearUserPref("extensions.allowPrivateBrowsingByDefault");
   });
 
-  let extensionId = "@permTest";
-  // We do not need to override incognito here, an extension installed during
-  // permanent private browsing gets the permission during install.
   let extension = ExtensionTestUtils.loadExtension({
-    useAddonManager: "permanent",
-    manifest: {
-      applications: {
-        gecko: {id: extensionId},
-      },
-    },
+    incognitoOverride: "spanning",
     async background() {
       browser.test.assertEq(window, browser.extension.getBackgroundPage(),
                             "Caller should be able to access itself as a background page");
       browser.test.assertEq(window, await browser.runtime.getBackgroundPage(),
                             "Caller should be able to access itself as a background page");
 
       browser.test.assertEq(browser.extension.inIncognitoContext, true,
                             "inIncognitoContext is true for permanent private browsing");
 
-      browser.test.sendMessage("incognito");
+      browser.test.notifyPass("incognito");
     },
   });
 
-  // Startup reason is ADDON_INSTALL
   await extension.startup();
 
-  await extension.awaitMessage("incognito");
-
-  let addon = await AddonManager.getAddonByID(extensionId);
-  await addon.disable();
+  await extension.awaitFinish("incognito");
 
-  // Permission remains when an extension is disabled.
-  let perms = await ExtensionPermissions.get(extensionId);
-  equal(perms.permissions.length, 1, "one permission");
-  equal(perms.permissions[0], "internal:privateBrowsingAllowed", "internal permission present");
+  await extension.unload();
+});
+
+add_task(async function test_background_PPB_not_allowed() {
+  info("Test background page incognito value with permanent private browsing enabled");
 
-  // Startup reason is ADDON_ENABLE, the permission is
-  // not granted in this case, but the extension should
-  // still have permission.
-  await addon.enable();
-  await Promise.all([
-    extension.awaitStartup(),
-    extension.awaitMessage("incognito"),
-  ]);
+  Services.prefs.setBoolPref("extensions.allowPrivateBrowsingByDefault", false);
+  Services.prefs.setBoolPref("browser.privatebrowsing.autostart", true);
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("browser.privatebrowsing.autostart");
+    Services.prefs.clearUserPref("extensions.allowPrivateBrowsingByDefault");
+  });
 
-  // ExtensionPermissions should still have it.
-  perms = await ExtensionPermissions.get(extensionId);
-  equal(perms.permissions.length, 1, "one permission");
-  equal(perms.permissions[0], "internal:privateBrowsingAllowed", "internal permission present");
+  let extension = ExtensionTestUtils.loadExtension({
+    async background() {
+      browser.test.notifyFail("incognito");
+    },
+  });
 
-  // This is the same as uninstall, no permissions after.
+  await extension.startup();
+
+  let state = extension.extension.state;
+  ok(state.startsWith("Startup: Cancelled"), `extension startup state should be cancelled "${state}"`);
+
   await extension.unload();
-
-  perms = await ExtensionPermissions.get(extensionId);
-  equal(perms.permissions.length, 0, "no permission");
-
-  await AddonTestUtils.promiseShutdownManager();
 });