Bug 1440932 - Add some documentation and other small improvements to the policy engine. r=bytesized

MozReview-Commit-ID: 5QeOmcx076r

--HG--
extra : rebase_source : ebaa788562f3d3dfc9c34b93fe3d8ba5f28e8db4

browser/components/enterprisepolicies/EnterprisePolicies.js

@@ -8,7 +8,6 @@ ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   WindowsGPOParser: "resource:///modules/policies/WindowsGPOParser.jsm",
-  NetUtil: "resource://gre/modules/NetUtil.jsm",
   Policies: "resource:///modules/policies/Policies.jsm",
   PoliciesValidator: "resource:///modules/policies/PoliciesValidator.jsm",
 });
@@ -140,11 +139,11 @@ EnterprisePoliciesManager.prototype = {
       let policyImpl = Policies[policyName];
 
       for (let timing of Object.keys(this._callbacks)) {
-        let policyCallback = policyImpl["on" + timing];
+        let policyCallback = policyImpl[timing];
         if (policyCallback) {
           this._schedulePolicyCallback(
             timing,
-            policyCallback.bind(null,
+            policyCallback.bind(policyImpl,
                                 this, /* the EnterprisePoliciesManager */
                                 parsedParameters));
         }
@@ -153,10 +152,24 @@ EnterprisePoliciesManager.prototype = {
   },
 
   _callbacks: {
-    BeforeAddons: [],
-    ProfileAfterChange: [],
-    BeforeUIStartup: [],
-    AllWindowsRestored: [],
+    // The earlist that a policy callback can run. This will
+    // happen right after the Policy Engine itself has started,
+    // and before the Add-ons Manager has started.
+    onBeforeAddons: [],
+
+    // This happens after all the initialization related to
+    // the profile has finished (prefs, places database, etc.).
+    onProfileAfterChange: [],
+
+    // Just before the first browser window gets created.
+    onBeforeUIStartup: [],
+
+    // Called after all windows from the last session have been
+    // restored (or the default window and homepage tab, if the
+    // session is not being restored).
+    // The content of the tabs themselves have not necessarily
+    // finished loading.
+    onAllWindowsRestored: [],
   },
 
   _schedulePolicyCallback(timing, callback) {
@@ -216,22 +229,23 @@ EnterprisePoliciesManager.prototype = {
   observe: function BG_observe(subject, topic, data) {
     switch (topic) {
       case "policies-startup":
+        // Before the first set of policy callbacks runs, we must
+        // initialize the service.
         this._initialize();
-        this._runPoliciesCallbacks("BeforeAddons");
+
+        this._runPoliciesCallbacks("onBeforeAddons");
         break;
 
       case "profile-after-change":
-        // Before the first set of policy callbacks runs, we must
-        // initialize the service.
-        this._runPoliciesCallbacks("ProfileAfterChange");
+        this._runPoliciesCallbacks("onProfileAfterChange");
         break;
 
       case "final-ui-startup":
-        this._runPoliciesCallbacks("BeforeUIStartup");
+        this._runPoliciesCallbacks("onBeforeUIStartup");
         break;
 
       case "sessionstore-windows-restored":
-        this._runPoliciesCallbacks("AllWindowsRestored");
+        this._runPoliciesCallbacks("onAllWindowsRestored");
 
         // After the last set of policy callbacks ran, notify the test observer.
         Services.obs.notifyObservers(null,

browser/components/enterprisepolicies/Policies.jsm

@@ -32,6 +32,32 @@ XPCOMUtils.defineLazyGetter(this, "log", () => {
 
 var EXPORTED_SYMBOLS = ["Policies"];
 
+/*
+ * ============================
+ * = POLICIES IMPLEMENTATIONS =
+ * ============================
+ *
+ * The Policies object below is where the implementation for each policy
+ * happens. An object for each policy should be defined, containing
+ * callback functions that will be called by the engine.
+ *
+ * See the _callbacks object in EnterprisePolicies.js for the list of
+ * possible callbacks and an explanation of each.
+ *
+ * Each callback will be called with two parameters:
+ * - manager
+ *   This is the EnterprisePoliciesManager singleton object from
+ *   EnterprisePolicies.js
+ *
+ * - param
+ *   The parameter defined for this policy in policies-schema.json.
+ *   It will be different for each policy. It could be a boolean,
+ *   a string, an array or a complex object. All parameters have
+ *   been validated according to the schema, and no unknown
+ *   properties will be present on them.
+ *
+ * The callbacks will be bound to their parent policy object.
+ */
 var Policies = {
   "BlockAboutAddons": {
     onBeforeUIStartup(manager, param) {
@@ -206,6 +232,19 @@ var Policies = {
  * The functions below are helpers to be used by several policies.
  */
 
+/**
+ * setAndLockPref
+ *
+ * Sets the _default_ value of a pref, and locks it (meaning that
+ * the default value will always be returned, independent from what
+ * is stored as the user value).
+ * The value is only changed in memory, and not stored to disk.
+ *
+ * @param {string} prefName
+ *        The pref to be changed
+ * @param {boolean,number,string} prefValue
+ *        The value to set and lock
+ */
 function setAndLockPref(prefName, prefValue) {
   if (Services.prefs.prefIsLocked(prefName)) {
     Services.prefs.unlockPref(prefName);
@@ -234,6 +273,19 @@ function setAndLockPref(prefName, prefValue) {
   Services.prefs.lockPref(prefName);
 }
 
+/**
+ * addAllowDenyPermissions
+ *
+ * Helper function to call the permissions manager (Services.perms.add)
+ * for two arrays of URLs.
+ *
+ * @param {string} permissionName
+ *        The name of the permission to change
+ * @param {array} allowList
+ *        The list of URLs to be set as ALLOW_ACTION for the chosen permission.
+ * @param {array} blockList
+ *        The list of URLs to be set as DENY_ACTION for the chosen permission.
+ */
 function addAllowDenyPermissions(permissionName, allowList, blockList) {
   allowList = allowList || [];
   blockList = blockList || [];