Bug 697314 - Defer importing xpinstall permissions until necessary. r=Mossop
authorBlair McBride <bmcbride@mozilla.com>
Fri, 25 Oct 2013 14:22:32 +1300
changeset 166988 3f41909b9433907b0178660e959105e40c3a2694
parent 166987 65d339e6816388edc6cfeae087925b8982d92025
child 166989 10f395671f719637a2bbe270612012e2d3fef041
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMossop
bugs697314
milestone27.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 697314 - Defer importing xpinstall permissions until necessary. r=Mossop
browser/components/preferences/permissions.js
toolkit/mozapps/extensions/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_bug578467.js
toolkit/mozapps/extensions/test/xpcshell/test_permissions_prefs.js
toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
--- a/browser/components/preferences/permissions.js
+++ b/browser/components/preferences/permissions.js
@@ -1,16 +1,18 @@
 /* -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const nsIPermissionManager = Components.interfaces.nsIPermissionManager;
 const nsICookiePermission = Components.interfaces.nsICookiePermission;
 
+const NOTIFICATION_FLUSH_PERMISSIONS = "flush-pending-permissions";
+
 function Permission(host, rawHost, type, capability, perm) 
 {
   this.host = host;
   this.rawHost = rawHost;
   this.type = type;
   this.capability = capability;
   this.perm = perm;
 }
@@ -178,16 +180,17 @@ var gPermissionManager = {
 
     this.onHostInput(urlField);
 
     var urlLabel = document.getElementById("urlLabel");
     urlLabel.hidden = !urlFieldVisible;
 
     var os = Components.classes["@mozilla.org/observer-service;1"]
                        .getService(Components.interfaces.nsIObserverService);
+    os.notifyObservers(null, NOTIFICATION_FLUSH_PERMISSIONS, this._type);
     os.addObserver(this, "perm-changed", false);
 
     this._loadPermissions();
     
     urlField.focus();
   },
   
   uninit: function ()
--- a/toolkit/mozapps/extensions/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
@@ -97,16 +97,17 @@ const KEY_TEMPDIR                     = 
 const KEY_APP_DISTRIBUTION            = "XREAppDist";
 
 const KEY_APP_PROFILE                 = "app-profile";
 const KEY_APP_GLOBAL                  = "app-global";
 const KEY_APP_SYSTEM_LOCAL            = "app-system-local";
 const KEY_APP_SYSTEM_SHARE            = "app-system-share";
 const KEY_APP_SYSTEM_USER             = "app-system-user";
 
+const NOTIFICATION_FLUSH_PERMISSIONS  = "flush-pending-permissions";
 const XPI_PERMISSION                  = "install";
 
 const RDFURI_INSTALL_MANIFEST_ROOT    = "urn:mozilla:install-manifest";
 const PREFIX_NS_EM                    = "http://www.mozilla.org/2004/em-rdf#";
 
 const TOOLKIT_ID                      = "toolkit@mozilla.org";
 
 // The maximum amount of file data to buffer at a time during file extraction
@@ -1947,16 +1948,17 @@ var XPIProvider = {
     this.minCompatibleAppVersion = Prefs.getCharPref(PREF_EM_MIN_COMPAT_APP_VERSION,
                                                      null);
     this.minCompatiblePlatformVersion = Prefs.getCharPref(PREF_EM_MIN_COMPAT_PLATFORM_VERSION,
                                                           null);
     this.enabledAddons = "";
 
     Services.prefs.addObserver(PREF_EM_MIN_COMPAT_APP_VERSION, this, false);
     Services.prefs.addObserver(PREF_EM_MIN_COMPAT_PLATFORM_VERSION, this, false);
+    Services.obs.addObserver(this, NOTIFICATION_FLUSH_PERMISSIONS, false);
 
     let flushCaches = this.checkForChanges(aAppChanged, aOldAppVersion,
                                            aOldPlatformVersion);
 
     // Changes to installed extensions may have changed which theme is selected
     this.applyThemeChange();
 
     // If the application has been upgraded and there are add-ons outside the
@@ -3317,20 +3319,16 @@ var XPIProvider = {
    *         The version of the platform last run with this profile or null
    *         if it is a new profile or the version is unknown
    * @return true if a change requiring a restart was detected
    */
   checkForChanges: function XPI_checkForChanges(aAppChanged, aOldAppVersion,
                                                 aOldPlatformVersion) {
     LOG("checkForChanges");
 
-    // Import the website installation permissions if the application has changed
-    if (aAppChanged !== false)
-      this.importPermissions();
-
     // Keep track of whether and why we need to open and update the database at
     // startup time.
     let updateReasons = [];
     if (aAppChanged) {
       updateReasons.push("appChanged");
     }
 
     // Load the list of bootstrapped add-ons first so processFileChanges can
@@ -3516,16 +3514,17 @@ var XPIProvider = {
 
     if (!aUri)
       return true;
 
     // file: and chrome: don't need whitelisted hosts
     if (aUri.schemeIs("chrome") || aUri.schemeIs("file"))
       return true;
 
+    this.importPermissions();
 
     let permission = Services.perms.testPermission(aUri, XPI_PERMISSION);
     if (permission == Ci.nsIPermissionManager.DENY_ACTION)
       return false;
 
     let requireWhitelist = Prefs.getBoolPref(PREF_XPI_WHITELIST_REQUIRED, true);
     if (requireWhitelist && (permission != Ci.nsIPermissionManager.ALLOW_ACTION))
       return false;
@@ -3844,25 +3843,34 @@ var XPIProvider = {
   },
 
   /**
    * Notified when a preference we're interested in has changed.
    *
    * @see nsIObserver
    */
   observe: function XPI_observe(aSubject, aTopic, aData) {
-    switch (aData) {
-    case PREF_EM_MIN_COMPAT_APP_VERSION:
-    case PREF_EM_MIN_COMPAT_PLATFORM_VERSION:
-      this.minCompatibleAppVersion = Prefs.getCharPref(PREF_EM_MIN_COMPAT_APP_VERSION,
-                                                       null);
-      this.minCompatiblePlatformVersion = Prefs.getCharPref(PREF_EM_MIN_COMPAT_PLATFORM_VERSION,
-                                                            null);
-      this.updateAddonAppDisabledStates();
-      break;
+    if (aTopic == NOTIFICATION_FLUSH_PERMISSIONS) {
+      if (!aData || aData == XPI_PERMISSION) {
+        this.importPermissions();
+      }
+      return;
+    }
+
+    if (aTopic == "nsPref:changed") {
+      switch (aData) {
+      case PREF_EM_MIN_COMPAT_APP_VERSION:
+      case PREF_EM_MIN_COMPAT_PLATFORM_VERSION:
+        this.minCompatibleAppVersion = Prefs.getCharPref(PREF_EM_MIN_COMPAT_APP_VERSION,
+                                                         null);
+        this.minCompatiblePlatformVersion = Prefs.getCharPref(PREF_EM_MIN_COMPAT_PLATFORM_VERSION,
+                                                              null);
+        this.updateAddonAppDisabledStates();
+        break;
+      }
     }
   },
 
   /**
    * Tests whether enabling an add-on will require a restart.
    *
    * @param  aAddon
    *         The add-on to test
rename from toolkit/mozapps/extensions/test/xpcshell/test_bug578467.js
rename to toolkit/mozapps/extensions/test/xpcshell/test_permissions_prefs.js
--- a/toolkit/mozapps/extensions/test/xpcshell/test_bug578467.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_permissions_prefs.js
@@ -1,37 +1,71 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
 // Tests that xpinstall.[whitelist|blacklist].add preferences are emptied when
-// converted into permissions on startup with new profile
+// converted into permissions.
 
 const PREF_XPI_WHITELIST_PERMISSIONS  = "xpinstall.whitelist.add";
 const PREF_XPI_BLACKLIST_PERMISSIONS  = "xpinstall.blacklist.add";
 
+function do_check_permission_prefs(preferences) {
+  // Check preferences were emptied
+  for (let pref of preferences) {
+    try {
+      do_check_eq(Services.prefs.getCharPref(pref), "");
+    }
+    catch (e) {
+      // Successfully emptied
+    }
+  }
+}
+
+function clear_imported_preferences_cache() {
+  let scope = Components.utils.import("resource://gre/modules/PermissionsUtils.jsm", {});
+  scope.gImportedPrefBranches.clear();
+}
+
 function run_test() {
   createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9");
 
   // Create own preferences to test
   Services.prefs.setCharPref("xpinstall.whitelist.add.EMPTY", "");
   Services.prefs.setCharPref("xpinstall.whitelist.add.TEST", "whitelist.example.com");
   Services.prefs.setCharPref("xpinstall.blacklist.add.EMPTY", "");
   Services.prefs.setCharPref("xpinstall.blacklist.add.TEST", "blacklist.example.com");
 
   // Get list of preferences to check
   var whitelistPreferences = Services.prefs.getChildList(PREF_XPI_WHITELIST_PERMISSIONS, {});
   var blacklistPreferences = Services.prefs.getChildList(PREF_XPI_BLACKLIST_PERMISSIONS, {});
   var preferences = whitelistPreferences.concat(blacklistPreferences);
 
   startupManager();
 
-  // Check preferences were emptied
-  preferences.forEach(function(aPreference) {
-    try {
-      do_check_eq(Services.prefs.getCharPref(aPreference), "");
-    }
-    catch (e) {
-      // Successfully emptied
-    }
-  });
+  // Permissions are imported lazily - act as thought we're checking an install,
+  // to trigger on-deman importing of the permissions.
+  let url = Services.io.newURI("http://example.com/file.xpi", null, null);
+  AddonManager.isInstallAllowed("application/x-xpinstall", url);
+  do_check_permission_prefs(preferences);
+
+
+  // Import can also be triggerred by an observer notification by any other area
+  // of code, such as a permissions management UI.
+
+  // First, request to flush all permissions
+  clear_imported_preferences_cache();
+  Services.prefs.setCharPref("xpinstall.whitelist.add.TEST2", "whitelist2.example.com");
+  Services.obs.notifyObservers(null, "flush-pending-permissions", "install");
+  do_check_permission_prefs(preferences);
+
+  // Then, request to flush just install permissions
+  clear_imported_preferences_cache();
+  Services.prefs.setCharPref("xpinstall.whitelist.add.TEST3", "whitelist3.example.com");
+  Services.obs.notifyObservers(null, "flush-pending-permissions", "");
+  do_check_permission_prefs(preferences);
+
+  // And a request to flush some other permissions sholdn't flush install permissions
+  clear_imported_preferences_cache();
+  Services.prefs.setCharPref("xpinstall.whitelist.add.TEST4", "whitelist4.example.com");
+  Services.obs.notifyObservers(null, "flush-pending-permissions", "lolcats");
+  do_check_eq(Services.prefs.getCharPref("xpinstall.whitelist.add.TEST4"), "whitelist4.example.com");
 }
-
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
@@ -117,17 +117,16 @@ run-sequentially = Uses hardcoded ports 
 # Bug 676992: test consistently fails on Android
 fail-if = os == "android"
 [test_bug564030.js]
 [test_bug566626.js]
 [test_bug567184.js]
 [test_bug569138.js]
 [test_bug570173.js]
 [test_bug576735.js]
-[test_bug578467.js]
 [test_bug587088.js]
 [test_bug594058.js]
 [test_bug595081.js]
 [test_bug595573.js]
 [test_bug596343.js]
 [test_bug596607.js]
 [test_bug616841.js]
 # Bug 676992: test consistently fails on Android
@@ -200,16 +199,17 @@ skip-if = os == "android"
 [test_migrate2.js]
 [test_migrate3.js]
 [test_migrate4.js]
 [test_migrate5.js]
 [test_migrateAddonRepository.js]
 [test_migrate_max_version.js]
 [test_onPropertyChanged_appDisabled.js]
 [test_permissions.js]
+[test_permissions_prefs.js]
 [test_plugins.js]
 [test_pluginchange.js]
 [test_pluginBlocklistCtp.js]
 # Bug 676992: test consistently fails on Android
 fail-if = os == "android"
 [test_pref_properties.js]
 [test_registry.js]
 [test_safemode.js]