Bug 1379560 - Part 2 - Add support for custom default permissions in SitePermissions.jsm. r=Paolo

Part 1 added support for changing default permissions via pref. This
patch adds support in the frontend code, which is required to actually
make it work for most permission prompts.

This patch introduces the concept of SitePermissions.PROMPT (which
already exists in the permission manager) to distinguish between
the default UNKNOWN state and the explicit PROMPT state. They both
have the same effect (always asking the user for confirmation).

MozReview-Commit-ID: 2Gg9uwigter

--HG--
extra : rebase_source : 2c8da24f849cee53e17be8897c0b320ca9e39e7e

browser/base/content/pageinfo/permissions.js

@@ -76,14 +76,14 @@ function initRow(aPartId) {
   var checkbox = document.getElementById(aPartId + "Def");
   var command  = document.getElementById("cmd_" + aPartId + "Toggle");
   var {state} = SitePermissions.get(gPermURI, aPartId);
+  let defaultState = SitePermissions.getDefault(aPartId);
 
-  if (state != SitePermissions.UNKNOWN) {
+  if (state != defaultState) {
     checkbox.checked = false;
     command.removeAttribute("disabled");
   } else {
     checkbox.checked = true;
     command.setAttribute("disabled", "true");
-    state = SitePermissions.getDefault(aPartId);
   }
   setRadioState(aPartId, state);
 
@@ -169,7 +169,7 @@ function onPluginRadioClick(aEvent) {
 function onRadioClick(aPartId) {
   var radioGroup = document.getElementById(aPartId + "RadioGroup");
   var id = radioGroup.selectedItem.id;
-  var permission = id.split("#")[1];
+  var permission = parseInt(id.split("#")[1]);
   SitePermissions.set(gPermURI, aPartId, permission);
 }
 

browser/components/preferences/sitePermissions.js

@@ -160,8 +160,15 @@ var gSitePermissionsManager = {
     menulist.appendChild(menupopup);
     let states = SitePermissions.getAvailableStates(permission.type);
     for (let state of states) {
-      if (state == SitePermissions.UNKNOWN)
+      // Work around the (rare) edge case when a user has changed their
+      // default permission type back to UNKNOWN while still having a
+      // PROMPT permission set for an origin.
+      if (state == SitePermissions.UNKNOWN &&
+          permission.capability == SitePermissions.PROMPT) {
+        state = SitePermissions.PROMPT;
+      } else if (state == SitePermissions.UNKNOWN) {
         continue;
+      }
       let m = document.createElement("menuitem");
       m.setAttribute("label", this._getCapabilityString(state));
       m.setAttribute("value", state);

browser/locales/en-US/chrome/browser/sitePermissions.properties

@@ -14,6 +14,7 @@ state.current.allowedForSession = Allowed for Session
 state.current.allowedTemporarily = Allowed Temporarily
 state.current.blockedTemporarily = Blocked Temporarily
 state.current.blocked = Blocked
+state.current.prompt = Always Ask
 
 # LOCALIZATION NOTE (state.multichoice.alwaysAsk,
 #                    state.multichoice.allow,

browser/modules/SitePermissions.jsm

@@ -141,6 +141,7 @@ this.SitePermissions = {
   UNKNOWN: Services.perms.UNKNOWN_ACTION,
   ALLOW: Services.perms.ALLOW_ACTION,
   BLOCK: Services.perms.DENY_ACTION,
+  PROMPT: Services.perms.PROMPT_ACTION,
   ALLOW_COOKIES_FOR_SESSION: Components.interfaces.nsICookiePermission.ACCESS_SESSION,
 
   // Permission scopes.
@@ -149,6 +150,8 @@ this.SitePermissions = {
   SCOPE_SESSION: "{SitePermissions.SCOPE_SESSION}",
   SCOPE_PERSISTENT: "{SitePermissions.SCOPE_PERSISTENT}",
 
+  _defaultPrefBranch: Services.prefs.getBranch("permissions.default."),
+
   /**
    * Gets all custom permissions for a given URI.
    * Install addon permission is excluded, check bug 1303108.
@@ -278,10 +281,13 @@ this.SitePermissions = {
         gPermissionObject[permissionID].states)
       return gPermissionObject[permissionID].states;
 
+    /* Since the permissions we are dealing with have adopted the convention
+     * of treating UNKNOWN == PROMPT, we only include one of either UNKNOWN
+     * or PROMPT in this list, to avoid duplicating states. */
     if (this.getDefault(permissionID) == this.UNKNOWN)
       return [ SitePermissions.UNKNOWN, SitePermissions.ALLOW, SitePermissions.BLOCK ];
 
-    return [ SitePermissions.ALLOW, SitePermissions.BLOCK ];
+    return [ SitePermissions.PROMPT, SitePermissions.ALLOW, SitePermissions.BLOCK ];
   },
 
   /**
@@ -293,11 +299,14 @@ this.SitePermissions = {
    * @return {SitePermissions.state} the default state.
    */
   getDefault(permissionID) {
+    // If the permission has custom logic for getting its default value,
+    // try that first.
     if (permissionID in gPermissionObject &&
         gPermissionObject[permissionID].getDefault)
       return gPermissionObject[permissionID].getDefault();
 
-    return this.UNKNOWN;
+    // Otherwise try to get the default preference for that permission.
+    return this._defaultPrefBranch.getIntPref(permissionID, this.UNKNOWN);
   },
 
   /**
@@ -320,7 +329,8 @@ this.SitePermissions = {
    *             (e.g. SitePermissions.SCOPE_PERSISTENT)
    */
   get(uri, permissionID, browser) {
-    let result = { state: this.UNKNOWN, scope: this.SCOPE_PERSISTENT };
+    let defaultState = this.getDefault(permissionID);
+    let result = { state: defaultState, scope: this.SCOPE_PERSISTENT };
     if (this.isSupportedURI(uri)) {
       let permission = null;
       if (permissionID in gPermissionObject &&
@@ -338,7 +348,7 @@ this.SitePermissions = {
       }
     }
 
-    if (!result.state) {
+    if (result.state == defaultState) {
       // If there's no persistent permission saved, check if we have something
       // set temporarily.
       let value = TemporaryBlockedPermissions.get(browser, permissionID);
@@ -371,7 +381,7 @@ this.SitePermissions = {
    *        This needs to be provided if the scope is SCOPE_TEMPORARY!
    */
   set(uri, permissionID, state, scope = this.SCOPE_PERSISTENT, browser = null) {
-    if (state == this.UNKNOWN) {
+    if (state == this.UNKNOWN || state == this.getDefault(permissionID)) {
       this.remove(uri, permissionID, browser);
       return;
     }
@@ -488,6 +498,7 @@ this.SitePermissions = {
   getMultichoiceStateLabel(state) {
     switch (state) {
       case this.UNKNOWN:
+      case this.PROMPT:
         return gStringBundle.GetStringFromName("state.multichoice.alwaysAsk");
       case this.ALLOW:
         return gStringBundle.GetStringFromName("state.multichoice.allow");
@@ -513,6 +524,8 @@ this.SitePermissions = {
    */
   getCurrentStateLabel(state, scope = null) {
     switch (state) {
+      case this.PROMPT:
+        return gStringBundle.GetStringFromName("state.current.prompt");
       case this.ALLOW:
         if (scope && scope != this.SCOPE_PERSISTENT)
           return gStringBundle.GetStringFromName("state.current.allowedTemporarily");
@@ -553,10 +566,7 @@ var gPermissionObject = {
    */
 
   "image": {
-    getDefault() {
-      return Services.prefs.getIntPref("permissions.default.image") == 2 ?
-               SitePermissions.BLOCK : SitePermissions.ALLOW;
-    }
+    states: [ SitePermissions.ALLOW, SitePermissions.BLOCK ],
   },
 
   "cookie": {
@@ -594,14 +604,16 @@ var gPermissionObject = {
     getDefault() {
       return Services.prefs.getBoolPref("dom.disable_open_during_load") ?
                SitePermissions.BLOCK : SitePermissions.ALLOW;
-    }
+    },
+    states: [ SitePermissions.ALLOW, SitePermissions.BLOCK ],
   },
 
   "install": {
     getDefault() {
       return Services.prefs.getBoolPref("xpinstall.whitelist.required") ?
                SitePermissions.BLOCK : SitePermissions.ALLOW;
-    }
+    },
+    states: [ SitePermissions.ALLOW, SitePermissions.BLOCK ],
   },
 
   "geo": {

browser/modules/test/unit/test_SitePermissions.js

@@ -66,6 +66,14 @@ add_task(async function testGetAvailableStates() {
                      SitePermissions.ALLOW,
                      SitePermissions.BLOCK ]);
 
+  // Test available states with a default permission set.
+  Services.prefs.setIntPref("permissions.default.camera", SitePermissions.ALLOW);
+  Assert.deepEqual(SitePermissions.getAvailableStates("camera"),
+                   [ SitePermissions.PROMPT,
+                     SitePermissions.ALLOW,
+                     SitePermissions.BLOCK ]);
+  Services.prefs.clearUserPref("permissions.default.camera");
+
   Assert.deepEqual(SitePermissions.getAvailableStates("cookie"),
                    [ SitePermissions.ALLOW,
                      SitePermissions.ALLOW_COOKIES_FOR_SESSION,
@@ -96,21 +104,75 @@ add_task(async function testExactHostMatch() {
 
     if (exactHostMatched.includes(permission)) {
       // Check that the sub-origin does not inherit the permission from its parent.
-      Assert.equal(SitePermissions.get(subUri, permission).state, SitePermissions.UNKNOWN);
+      Assert.equal(SitePermissions.get(subUri, permission).state, SitePermissions.UNKNOWN,
+        `${permission} should exact-host match`);
     } else if (nonExactHostMatched.includes(permission)) {
       // Check that the sub-origin does inherit the permission from its parent.
-      Assert.equal(SitePermissions.get(subUri, permission).state, SitePermissions.ALLOW);
+      Assert.equal(SitePermissions.get(subUri, permission).state, SitePermissions.ALLOW,
+        `${permission} should not exact-host match`);
     } else {
       Assert.ok(false, `Found an unknown permission ${permission} in exact host match test.` +
                        "Please add new permissions from SitePermissions.jsm to this test.");
     }
 
     // Check that the permission can be made specific to the sub-origin.
-    SitePermissions.set(subUri, permission, SitePermissions.BLOCK);
-    Assert.equal(SitePermissions.get(subUri, permission).state, SitePermissions.BLOCK);
+    SitePermissions.set(subUri, permission, SitePermissions.PROMPT);
+    Assert.equal(SitePermissions.get(subUri, permission).state, SitePermissions.PROMPT);
     Assert.equal(SitePermissions.get(uri, permission).state, SitePermissions.ALLOW);
 
     SitePermissions.remove(subUri, permission);
     SitePermissions.remove(uri, permission);
   }
 });
+
+add_task(function* testDefaultPrefs() {
+  let uri = Services.io.newURI("https://example.com")
+
+  // Check that without a pref the default return value is UNKNOWN.
+  Assert.deepEqual(SitePermissions.get(uri, "camera"), {
+    state: SitePermissions.UNKNOWN,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+
+  // Check that the default return value changed after setting the pref.
+  Services.prefs.setIntPref("permissions.default.camera", SitePermissions.BLOCK);
+  Assert.deepEqual(SitePermissions.get(uri, "camera"), {
+    state: SitePermissions.BLOCK,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+
+  // Check that other permissions still return UNKNOWN.
+  Assert.deepEqual(SitePermissions.get(uri, "microphone"), {
+    state: SitePermissions.UNKNOWN,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+
+  // Check that the default return value changed after changing the pref.
+  Services.prefs.setIntPref("permissions.default.camera", SitePermissions.ALLOW);
+  Assert.deepEqual(SitePermissions.get(uri, "camera"), {
+    state: SitePermissions.ALLOW,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+
+  // Check that the preference is ignored if there is a value.
+  SitePermissions.set(uri, "camera", SitePermissions.BLOCK);
+  Assert.deepEqual(SitePermissions.get(uri, "camera"), {
+    state: SitePermissions.BLOCK,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+
+  // The preference should be honored again, after resetting the permissions.
+  SitePermissions.remove(uri, "camera");
+  Assert.deepEqual(SitePermissions.get(uri, "camera"), {
+    state: SitePermissions.ALLOW,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+
+  // Should be UNKNOWN after clearing the pref.
+  Services.prefs.clearUserPref("permissions.default.camera");
+  Assert.deepEqual(SitePermissions.get(uri, "camera"), {
+    state: SitePermissions.UNKNOWN,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+});
+