Bug 1331604 - Minor style and idiom changes (r?mgoodwin) draft
authorMathieu Leplatre <mathieu@mozilla.com>
Tue, 17 Jan 2017 15:04:43 +0100
changeset 463092 6c681a3e018188cd3613ffb29cbbbee30d027347
parent 463091 1579c47aea46600d16e9316e05bcbfefc7d0b4ea
child 542569 ba6fe7833c2a119fde2f84e9add04010ed126761
push id41950
push usermleplatre@mozilla.com
push dateWed, 18 Jan 2017 14:00:38 +0000
reviewersmgoodwin
bugs1331604
milestone53.0a1
Bug 1331604 - Minor style and idiom changes (r?mgoodwin) MozReview-Commit-ID: IdO8EP3hg0P
services/common/blocklist-clients.js
services/common/blocklist-updater.js
--- a/services/common/blocklist-clients.js
+++ b/services/common/blocklist-clients.js
@@ -47,16 +47,17 @@ const INVALID_SIGNATURE = "Invalid conte
 // FirefoxAdapter, so for backwards compatibility we maintain this
 // filename, even though it isn't descriptive of who is using it.
 this.KINTO_STORAGE_PATH    = "kinto.sqlite";
 
 this.FILENAME_ADDONS_JSON  = "blocklist-addons.json";
 this.FILENAME_GFX_JSON     = "blocklist-gfx.json";
 this.FILENAME_PLUGINS_JSON = "blocklist-plugins.json";
 
+
 function mergeChanges(collection, localRecords, changes) {
   const records = {};
   // Local records by id.
   localRecords.forEach((record) => records[record.id] = collection.cleanLocalFields(record));
   // All existing records are replaced by the version from the server.
   changes.forEach((record) => records[record.id] = record);
 
   return Object.values(records)
@@ -83,20 +84,20 @@ function fetchRemoteCollection(collectio
 }
 
 /**
  * Helper to instantiate a Kinto client based on preferences for remote server
  * URL and bucket name. It uses the `FirefoxAdapter` which relies on SQLite to
  * persist the local DB.
  */
 function kintoClient(connection, bucket) {
-  let base = Services.prefs.getCharPref(PREF_SETTINGS_SERVER);
+  const remote = Services.prefs.getCharPref(PREF_SETTINGS_SERVER);
 
-  let config = {
-    remote: base,
+  const config = {
+    remote,
     bucket,
     adapter: FirefoxAdapter,
     adapterOptions: {sqliteHandle: connection},
   };
 
   return new Kinto(config);
 }
 
@@ -122,17 +123,17 @@ class BlocklistClient {
 
       let toSerialize;
       if (ignoreLocal) {
         toSerialize = {
           last_modified: `${payload.last_modified}`,
           data: payload.data
         };
       } else {
-        const localRecords = (yield collection.list()).data;
+        const {data: localRecords} = yield collection.list();
         const records = mergeChanges(collection, localRecords, payload.changes);
         toSerialize = {
           last_modified: `${payload.lastModified}`,
           data: records
         };
       }
 
       const serialized = CanonicalJSON.stringify(toSerialize);
@@ -151,101 +152,101 @@ class BlocklistClient {
    * Synchronize from Kinto server, if necessary.
    *
    * @param {int}  lastModified the lastModified date (on the server) for
                                 the remote collection.
    * @param {Date} serverTime   the current date return by the server.
    * @return {Promise}          which rejects on sync or process failure.
    */
   maybeSync(lastModified, serverTime) {
-    let opts = {};
-    let enforceCollectionSigning =
+    const opts = {};
+    const enforceCollectionSigning =
       Services.prefs.getBoolPref(PREF_BLOCKLIST_ENFORCE_SIGNING);
 
     // if there is a signerName and collection signing is enforced, add a
     // hook for incoming changes that validates the signature
     if (this.signerName && enforceCollectionSigning) {
       opts.hooks = {
         "incoming-changes": [this.validateCollectionSignature.bind(this)]
       }
     }
 
 
     return Task.spawn((function* syncCollection() {
       let connection;
       try {
         connection = yield FirefoxAdapter.openConnection({path: KINTO_STORAGE_PATH});
-        let db = kintoClient(connection, this.bucketName);
-        let collection = db.collection(this.collectionName, opts);
+        const db = kintoClient(connection, this.bucketName);
+        const collection = db.collection(this.collectionName, opts);
 
-        let collectionLastModified = yield collection.db.getLastModified();
+        const collectionLastModified = yield collection.db.getLastModified();
         // If the data is up to date, there's no need to sync. We still need
         // to record the fact that a check happened.
         if (lastModified <= collectionLastModified) {
           this.updateLastCheck(serverTime);
           return;
         }
         // Fetch changes from server.
         try {
-          let syncResult = yield collection.sync();
-          if (!syncResult.ok) {
+          const {ok} = yield collection.sync();
+          if (!ok) {
             throw new Error("Sync failed");
           }
         } catch (e) {
           if (e.message == INVALID_SIGNATURE) {
             // if sync fails with a signature error, it's likely that our
             // local data has been modified in some way.
             // We will attempt to fix this by retrieving the whole
             // remote collection.
-            let payload = yield fetchRemoteCollection(collection);
+            const payload = yield fetchRemoteCollection(collection);
             yield this.validateCollectionSignature(payload, collection, true);
             // if the signature is good (we haven't thrown), and the remote
             // last_modified is newer than the local last_modified, replace the
             // local data
             const localLastModified = yield collection.db.getLastModified();
             if (payload.last_modified >= localLastModified) {
               yield collection.clear();
               yield collection.loadDump(payload.data);
             }
           } else {
             throw e;
           }
         }
         // Read local collection of records.
-        let list = yield collection.list();
+        const {data} = yield collection.list();
 
-        yield this.processCallback(list.data);
+        yield this.processCallback(data);
 
         // Track last update.
         this.updateLastCheck(serverTime);
       } finally {
         yield connection.close();
       }
     }).bind(this));
   }
 
   /**
    * Save last time server was checked in users prefs.
    *
    * @param {Date} serverTime   the current date return by server.
    */
   updateLastCheck(serverTime) {
-    let checkedServerTimeInSeconds = Math.round(serverTime / 1000);
+    const checkedServerTimeInSeconds = Math.round(serverTime / 1000);
     Services.prefs.setIntPref(this.lastCheckTimePref, checkedServerTimeInSeconds);
   }
 }
 
 /**
  * Revoke the appropriate certificates based on the records from the blocklist.
  *
  * @param {Object} records   current records in the local db.
  */
 function* updateCertBlocklist(records) {
-  let certList = Cc["@mozilla.org/security/certblocklist;1"]
-                   .getService(Ci.nsICertBlocklist);
+  const certList = Cc["@mozilla.org/security/certblocklist;1"]
+                     .getService(Ci.nsICertBlocklist);
   for (let item of records) {
     try {
       if (item.issuerName && item.serialNumber) {
         certList.revokeCertByIssuerAndSerial(item.issuerName,
                                             item.serialNumber);
       } else if (item.subject && item.pubKeyHash) {
         certList.revokeCertBySubjectAndPubKey(item.subject,
                                               item.pubKeyHash);
@@ -262,49 +263,50 @@ function* updateCertBlocklist(records) {
 
 /**
  * Modify the appropriate security pins based on records from the remote
  * collection.
  *
  * @param {Object} records   current records in the local db.
  */
 function* updatePinningList(records) {
-  if (Services.prefs.getBoolPref(PREF_BLOCKLIST_PINNING_ENABLED)) {
-    const appInfo = Cc["@mozilla.org/xre/app-info;1"]
-        .getService(Ci.nsIXULAppInfo);
+  if (!Services.prefs.getBoolPref(PREF_BLOCKLIST_PINNING_ENABLED)) {
+    return;
+  }
+  const appInfo = Cc["@mozilla.org/xre/app-info;1"]
+      .getService(Ci.nsIXULAppInfo);
 
-    const siteSecurityService = Cc["@mozilla.org/ssservice;1"]
-        .getService(Ci.nsISiteSecurityService);
+  const siteSecurityService = Cc["@mozilla.org/ssservice;1"]
+      .getService(Ci.nsISiteSecurityService);
 
-    // clear the current preload list
-    siteSecurityService.clearPreloads();
+  // clear the current preload list
+  siteSecurityService.clearPreloads();
 
-    // write each KeyPin entry to the preload list
-    for (let item of records) {
-      try {
-        const {pinType, pins = [], versions} = item;
-        if (versions.indexOf(appInfo.version) != -1) {
-          if (pinType == "KeyPin" && pins.length) {
-            siteSecurityService.setKeyPins(item.hostName,
-                item.includeSubdomains,
-                item.expires,
-                pins.length,
-                pins, true);
-          }
-          if (pinType == "STSPin") {
-            siteSecurityService.setHSTSPreload(item.hostName,
-                                               item.includeSubdomains,
-                                               item.expires);
-          }
+  // write each KeyPin entry to the preload list
+  for (let item of records) {
+    try {
+      const {pinType, pins = [], versions} = item;
+      if (versions.indexOf(appInfo.version) != -1) {
+        if (pinType == "KeyPin" && pins.length) {
+          siteSecurityService.setKeyPins(item.hostName,
+              item.includeSubdomains,
+              item.expires,
+              pins.length,
+              pins, true);
         }
-      } catch (e) {
-        // prevent errors relating to individual preload entries from causing
-        // sync to fail. We will accumulate telemetry for such failures in bug
-        // 1254099.
+        if (pinType == "STSPin") {
+          siteSecurityService.setHSTSPreload(item.hostName,
+                                             item.includeSubdomains,
+                                             item.expires);
+        }
       }
+    } catch (e) {
+      // prevent errors relating to individual preload entries from causing
+      // sync to fail. We will accumulate telemetry for such failures in bug
+      // 1254099.
     }
   }
 }
 
 /**
  * Write list of records into JSON file, and notify nsBlocklistService.
  *
  * @param {String} filename  path relative to profile dir.
--- a/services/common/blocklist-updater.js
+++ b/services/common/blocklist-updater.js
@@ -37,64 +37,58 @@ this.checkVersions = function() {
     // {"data":[
     //   {
     //     "host":"kinto-ota.dev.mozaws.net",
     //     "last_modified":1450717104423,
     //     "bucket":"blocklists",
     //     "collection":"certificates"
     //    }]}
     // Right now, we only use the collection name and the last modified info
-    let kintoBase = Services.prefs.getCharPref(PREF_SETTINGS_SERVER);
-    let changesEndpoint = kintoBase + Services.prefs.getCharPref(PREF_BLOCKLIST_CHANGES_PATH);
+    const kintoBase = Services.prefs.getCharPref(PREF_SETTINGS_SERVER);
+    const changesEndpoint = kintoBase + Services.prefs.getCharPref(PREF_BLOCKLIST_CHANGES_PATH);
 
     // Use ETag to obtain a `304 Not modified` when no change occurred.
     const headers = {};
     if (Services.prefs.prefHasUserValue(PREF_BLOCKLIST_LAST_ETAG)) {
       const lastEtag = Services.prefs.getCharPref(PREF_BLOCKLIST_LAST_ETAG);
       if (lastEtag) {
         headers["If-None-Match"] = lastEtag;
       }
     }
 
-    let response = yield fetch(changesEndpoint, {headers});
+    const response = yield fetch(changesEndpoint, {headers});
 
     let versionInfo;
     // No changes since last time. Go on with empty list of changes.
     if (response.status == 304) {
       versionInfo = {data: []};
     } else {
       versionInfo = yield response.json();
     }
 
     // If the server is failing, the JSON response might not contain the
     // expected data (e.g. error response - Bug 1259145)
     if (!versionInfo.hasOwnProperty("data")) {
       throw new Error("Polling for changes failed.");
     }
 
     // Record new update time and the difference between local and server time
-    let serverTimeMillis = Date.parse(response.headers.get("Date"));
+    const serverTimeMillis = Date.parse(response.headers.get("Date"));
 
     // negative clockDifference means local time is behind server time
     // by the absolute of that value in seconds (positive means it's ahead)
-    let clockDifference = Math.floor((Date.now() - serverTimeMillis) / 1000);
+    const clockDifference = Math.floor((Date.now() - serverTimeMillis) / 1000);
     Services.prefs.setIntPref(PREF_BLOCKLIST_CLOCK_SKEW_SECONDS, clockDifference);
     Services.prefs.setIntPref(PREF_BLOCKLIST_LAST_UPDATE, serverTimeMillis / 1000);
 
     let firstError;
     for (let collectionInfo of versionInfo.data) {
-      let collection = collectionInfo.collection;
-      let client = gBlocklistClients[collection];
-      if (client &&
-          client.bucketName == collectionInfo.bucket &&
-          client.maybeSync) {
-        let lastModified = 0;
-        if (collectionInfo.last_modified) {
-          lastModified = collectionInfo.last_modified;
-        }
+      const {bucket, collection, last_modified: lastModified} = collectionInfo;
+      const client = gBlocklistClients[collection];
+      if (client && client.bucketName == bucket) {
         try {
           yield client.maybeSync(lastModified, serverTimeMillis);
         } catch (e) {
           if (!firstError) {
             firstError = e;
           }
         }
       }