Bug 1648134 - Permission Manager does not save changes when scope is changed., r=mayhemer

Differential Revision: https://phabricator.services.mozilla.com/D81292

extensions/permissions/PermissionManager.cpp

@@ -1779,6 +1779,8 @@ nsresult PermissionManager::AddInternal(
         break;
       }
 
+      PermissionEntry oldPermissionEntry = entry->GetPermissions()[index];
+
       // If the new expireType is EXPIRE_SESSION, then we have to keep a
       // copy of the previous permission/expireType values. This cached value
       // will be used when restoring the permissions of an app.
@@ -1802,13 +1804,29 @@ nsresult PermissionManager::AddInternal(
       entry->GetPermissions()[index].mExpireTime = aExpireTime;
       entry->GetPermissions()[index].mModificationTime = aModificationTime;
 
-      if (aDBOperation == eWriteToDB &&
-          IsPersistentExpire(aExpireType, aType)) {
-        // We care only about the id, the permission and
-        // expireType/expireTime/modificationTime here. We pass dummy values for
-        // all other parameters.
-        UpdateDB(op, id, EmptyCString(), EmptyCString(), aPermission,
-                 aExpireType, aExpireTime, aModificationTime);
+      if (aDBOperation == eWriteToDB) {
+        bool newIsPersistentExpire = IsPersistentExpire(aExpireType, aType);
+        bool oldIsPersistentExpire =
+            IsPersistentExpire(oldPermissionEntry.mExpireType, aType);
+
+        if (!newIsPersistentExpire && oldIsPersistentExpire) {
+          // Maybe we have to remove the previous permission if that was
+          // persistent.
+          UpdateDB(eOperationRemoving, id, EmptyCString(), EmptyCString(), 0,
+                   nsIPermissionManager::EXPIRE_NEVER, 0, 0);
+        } else if (newIsPersistentExpire && !oldIsPersistentExpire) {
+          // It could also be that the previous permission was session-only but
+          // this needs to be written into the DB. In this case, we have to run
+          // an Adding operation.
+          UpdateDB(eOperationAdding, id, origin, aType, aPermission,
+                   aExpireType, aExpireTime, aModificationTime);
+        } else if (newIsPersistentExpire) {
+          // This is the a simple update.  We care only about the id, the
+          // permission and expireType/expireTime/modificationTime here. We pass
+          // dummy values for all other parameters.
+          UpdateDB(op, id, EmptyCString(), EmptyCString(), aPermission,
+                   aExpireType, aExpireTime, aModificationTime);
+        }
       }
 
       if (aNotifyOperation == eNotify) {

extensions/permissions/test/unit/test_permmanager_remove_add_update.js

@@ -0,0 +1,83 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function check_enumerator(principal, permissions) {
+  let perms = Services.perms.getAllForPrincipal(principal);
+  for (let [type, capability, expireType] of permissions) {
+    let perm = perms.shift();
+    Assert.ok(perm != null);
+    Assert.equal(perm.type, type);
+    Assert.equal(perm.capability, capability);
+    Assert.equal(perm.expireType, expireType);
+  }
+  Assert.ok(!perms.length);
+}
+
+add_task(async function test() {
+  Services.prefs.setCharPref("permissions.manager.defaultsUrl", "");
+
+  // setup a profile directory
+  do_get_profile();
+
+  // We need to execute a pm method to be sure that the DB is fully
+  // initialized.
+  var pm = Services.perms;
+  Assert.ok(pm.all.length === 0);
+
+  let principal = Services.scriptSecurityManager.createContentPrincipalFromOrigin(
+    "http://example.com"
+  );
+
+  info("From session to persistent");
+  pm.addFromPrincipal(
+    principal,
+    "test/foo",
+    pm.ALLOW_ACTION,
+    pm.EXPIRE_SESSION
+  );
+
+  check_enumerator(principal, [
+    ["test/foo", pm.ALLOW_ACTION, pm.EXPIRE_SESSION],
+  ]);
+
+  pm.addFromPrincipal(principal, "test/foo", pm.ALLOW_ACTION, pm.EXPIRE_NEVER);
+
+  check_enumerator(principal, [["test/foo", pm.ALLOW_ACTION, pm.EXPIRE_NEVER]]);
+
+  // Let's reload the DB.
+  Services.obs.notifyObservers(null, "testonly-reload-permissions-from-disk");
+
+  Assert.ok(pm.all.length === 1);
+  check_enumerator(principal, [["test/foo", pm.ALLOW_ACTION, pm.EXPIRE_NEVER]]);
+
+  info("From persistent to session");
+  pm.addFromPrincipal(
+    principal,
+    "test/foo",
+    pm.ALLOW_ACTION,
+    pm.EXPIRE_SESSION
+  );
+
+  check_enumerator(principal, [
+    ["test/foo", pm.ALLOW_ACTION, pm.EXPIRE_SESSION],
+  ]);
+
+  // Let's reload the DB.
+  Services.obs.notifyObservers(null, "testonly-reload-permissions-from-disk");
+  Assert.ok(pm.all.length === 0);
+
+  info("From persistent to persistent");
+  pm.addFromPrincipal(principal, "test/foo", pm.ALLOW_ACTION, pm.EXPIRE_NEVER);
+  pm.addFromPrincipal(principal, "test/foo", pm.DENY_ACTION, pm.EXPIRE_NEVER);
+
+  check_enumerator(principal, [["test/foo", pm.DENY_ACTION, pm.EXPIRE_NEVER]]);
+
+  // Let's reload the DB.
+  Services.obs.notifyObservers(null, "testonly-reload-permissions-from-disk");
+  Assert.ok(pm.all.length === 1);
+  check_enumerator(principal, [["test/foo", pm.DENY_ACTION, pm.EXPIRE_NEVER]]);
+
+  info("Cleanup");
+  pm.removeAll();
+  check_enumerator(principal, []);
+});

extensions/permissions/test/unit/xpcshell.ini

@@ -32,3 +32,4 @@ skip-if = debug == true
 [test_permmanager_migrate_9-10.js]
 [test_permmanager_migrate_10-11.js]
 [test_permmanager_oa_strip.js]
+[test_permmanager_remove_add_update.js]