Bug 1469525 - Remove blocklist clients files from profile r=Gijs draft
authorMathieu Leplatre <mathieu@mozilla.com>
Mon, 06 Aug 2018 17:34:28 +0200
changeset 827144 4cf1de6b60399ab5092bf01ab4c64b255fdfb068
parent 826918 0dbeb72b579816a450db3430f35174655a975c53
push id118484
push usermleplatre@mozilla.com
push dateTue, 07 Aug 2018 10:56:07 +0000
reviewersGijs
bugs1469525
milestone63.0a1
Bug 1469525 - Remove blocklist clients files from profile r=Gijs MozReview-Commit-ID: Fm5Pegz5LcU
browser/components/nsBrowserGlue.js
services/common/blocklist-clients.js
services/common/tests/unit/test_blocklist_clients.js
services/settings/remote-settings.js
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -1797,17 +1797,17 @@ BrowserGlue.prototype = {
       }
     }
   },
 
   // eslint-disable-next-line complexity
   _migrateUI: function BG__migrateUI() {
     // Use an increasing number to keep track of the current migration state.
     // Completely unrelated to the current Firefox release number.
-    const UI_VERSION = 72;
+    const UI_VERSION = 73;
     const BROWSER_DOCURL = AppConstants.BROWSER_CHROME_URL;
 
     let currentUIVersion;
     if (Services.prefs.prefHasUserValue("browser.migration.version")) {
       currentUIVersion = Services.prefs.getIntPref("browser.migration.version");
     } else {
       // This is a new profile, nothing to migrate.
       Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
@@ -2134,16 +2134,29 @@ BrowserGlue.prototype = {
     }
 
     if (currentUIVersion < 72) {
       // Migrate performance tool's recording interval value from msec to usec.
       let pref = "devtools.performance.recording.interval";
       Services.prefs.setIntPref(pref, Services.prefs.getIntPref(pref, 1) * 1000);
     }
 
+    if (currentUIVersion < 73) {
+      // Remove blocklist JSON local dumps in profile.
+      OS.File.removeDir(OS.Path.join(OS.Constants.Path.profileDir, "blocklists"),
+                        { ignoreAbsent: true });
+      OS.File.removeDir(OS.Path.join(OS.Constants.Path.profileDir, "blocklists-preview"),
+                        { ignoreAbsent: true });
+      for (const filename of ["addons.json", "plugins.json", "gfx.json"]) {
+        // Some old versions used to dump without subfolders. Clean them while we are at it.
+        const path = OS.Path.join(OS.Constants.Path.profileDir, `blocklists-${filename}`);
+        OS.File.remove(path, { ignoreAbsent: true });
+      }
+    }
+
     // Update the migration version.
     Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
   },
 
   _checkForDefaultBrowser() {
     // Perform default browser checking.
     if (!ShellService) {
       return;
--- a/services/common/blocklist-clients.js
+++ b/services/common/blocklist-clients.js
@@ -99,41 +99,16 @@ async function updatePinningList({ data:
       // 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 {Object} client   RemoteSettingsClient instance
- * @param {Object} data      Current records in the local db.
- */
-async function updateJSONBlocklist(client, { data: { current: records } }) {
-  // Write JSON dump for synchronous load at startup.
-  const path = OS.Path.join(OS.Constants.Path.profileDir, client.filename);
-  const blocklistFolder = OS.Path.dirname(path);
-
-  await OS.File.makeDir(blocklistFolder, {from: OS.Constants.Path.profileDir});
-
-  const serialized = JSON.stringify({data: records}, null, 2);
-  try {
-    await OS.File.writeAtomic(path, serialized, {tmpPath: path + ".tmp"});
-    // Notify change to `nsBlocklistService`
-    const eventData = {filename: client.filename};
-    Services.cpmm.sendAsyncMessage("Blocklist:reload-from-disk", eventData);
-  } catch (e) {
-    Cu.reportError(e);
-  }
-}
-
-
-/**
  * This custom filter function is used to limit the entries returned
  * by `RemoteSettings("...").get()` depending on the target app information
  * defined on entries.
  */
 async function targetAppFilter(entry, environment) {
   // If the entry has a JEXL filter expression, it should prevail.
   // The legacy target app mechanism will be kept in place for old entries.
   // See https://bugzilla.mozilla.org/show_bug.cgi?id=1463377
@@ -206,33 +181,30 @@ function initialize() {
   OneCRLBlocklistClient.on("sync", updateCertBlocklist);
 
   AddonBlocklistClient = RemoteSettings(Services.prefs.getCharPref(PREF_BLOCKLIST_ADDONS_COLLECTION), {
     bucketName: Services.prefs.getCharPref(PREF_BLOCKLIST_BUCKET),
     lastCheckTimePref: PREF_BLOCKLIST_ADDONS_CHECKED_SECONDS,
     signerName: Services.prefs.getCharPref(PREF_BLOCKLIST_ADDONS_SIGNER),
     filterFunc: targetAppFilter,
   });
-  AddonBlocklistClient.on("sync", updateJSONBlocklist.bind(null, AddonBlocklistClient));
 
   PluginBlocklistClient = RemoteSettings(Services.prefs.getCharPref(PREF_BLOCKLIST_PLUGINS_COLLECTION), {
     bucketName: Services.prefs.getCharPref(PREF_BLOCKLIST_BUCKET),
     lastCheckTimePref: PREF_BLOCKLIST_PLUGINS_CHECKED_SECONDS,
     signerName: Services.prefs.getCharPref(PREF_BLOCKLIST_PLUGINS_SIGNER),
     filterFunc: targetAppFilter,
   });
-  PluginBlocklistClient.on("sync", updateJSONBlocklist.bind(null, PluginBlocklistClient));
 
   GfxBlocklistClient = RemoteSettings(Services.prefs.getCharPref(PREF_BLOCKLIST_GFX_COLLECTION), {
     bucketName: Services.prefs.getCharPref(PREF_BLOCKLIST_BUCKET),
     lastCheckTimePref: PREF_BLOCKLIST_GFX_CHECKED_SECONDS,
     signerName: Services.prefs.getCharPref(PREF_BLOCKLIST_GFX_SIGNER),
     filterFunc: targetAppFilter,
   });
-  GfxBlocklistClient.on("sync", updateJSONBlocklist.bind(null, GfxBlocklistClient));
 
   PinningBlocklistClient = RemoteSettings(Services.prefs.getCharPref(PREF_BLOCKLIST_PINNING_COLLECTION), {
     bucketName: Services.prefs.getCharPref(PREF_BLOCKLIST_PINNING_BUCKET),
     lastCheckTimePref: PREF_BLOCKLIST_PINNING_CHECKED_SECONDS,
     signerName: Services.prefs.getCharPref(PREF_BLOCKLIST_PINNING_SIGNER),
   });
   PinningBlocklistClient.on("sync", updatePinningList);
 
--- a/services/common/tests/unit/test_blocklist_clients.js
+++ b/services/common/tests/unit/test_blocklist_clients.js
@@ -1,47 +1,35 @@
 const { Constructor: CC } = Components;
 
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://testing-common/httpd.js");
 const { FileUtils } = ChromeUtils.import("resource://gre/modules/FileUtils.jsm", {});
-const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm", {});
 
 const { RemoteSettings } = ChromeUtils.import("resource://services-settings/remote-settings.js", {});
 const BlocklistClients = ChromeUtils.import("resource://services-common/blocklist-clients.js", {});
 
 const BinaryInputStream = CC("@mozilla.org/binaryinputstream;1",
   "nsIBinaryInputStream", "setInputStream");
 
 const IS_ANDROID = AppConstants.platform == "android";
 
 
 let gBlocklistClients;
 let server;
 
-async function readJSON(filepath) {
-  const binaryData = await OS.File.read(filepath);
-  const textData = (new TextDecoder()).decode(binaryData);
-  return Promise.resolve(JSON.parse(textData));
-}
-
 async function clear_state() {
   for (let {client} of gBlocklistClients) {
     // Remove last server times.
     Services.prefs.clearUserPref(client.lastCheckTimePref);
 
     // Clear local DB.
     const collection = await client.openCollection();
     await collection.clear();
-
-    // Remove JSON dumps folders in profile dir.
-    const dumpFile = OS.Path.join(OS.Constants.Path.profileDir, client.filename);
-    const folder = OS.Path.dirname(dumpFile);
-    await OS.File.removeDir(folder, { ignoreAbsent: true });
   }
 }
 
 
 function run_test() {
   // Set up an HTTP Server
   server = new HttpServer();
   server.start(-1);
@@ -141,80 +129,29 @@ add_task(async function test_initial_dum
 
     // Calling .get() will load the dump.
     const afterLoaded = await client.get();
     ok(afterLoaded.length > 0, `Loaded dump of ${client.collectionName} has ${afterLoaded.length} records`);
   }
 });
 add_task(clear_state);
 
-add_task(async function test_list_is_written_to_file_in_profile() {
-  for (let {client, testData} of gBlocklistClients) {
-    const filePath = OS.Path.join(OS.Constants.Path.profileDir, client.filename);
-    const profFile = new FileUtils.File(filePath);
-    strictEqual(profFile.exists(), false);
-
-    await client.maybeSync(2000, Date.now(), {loadDump: false});
-
-    strictEqual(profFile.exists(), true);
-    const content = await readJSON(profFile.path);
-    equal(content.data[0].blockID, testData[testData.length - 1]);
-  }
-});
-add_task(clear_state);
-
 add_task(async function test_current_server_time_is_saved_in_pref() {
   for (let {client} of gBlocklistClients) {
     // The lastCheckTimePref was customized:
     ok(/services\.blocklist\.(\w+)\.checked/.test(client.lastCheckTimePref), client.lastCheckTimePref);
 
     const serverTime = Date.now();
     await client.maybeSync(2000, serverTime);
     const after = Services.prefs.getIntPref(client.lastCheckTimePref);
     equal(after, Math.round(serverTime / 1000));
   }
 });
 add_task(clear_state);
 
-add_task(async function test_update_json_file_when_addons_has_changes() {
-  for (let {client, testData} of gBlocklistClients) {
-    await client.maybeSync(2000, Date.now() - 1000, {loadDump: false});
-    const filePath = OS.Path.join(OS.Constants.Path.profileDir, client.filename);
-    const profFile = new FileUtils.File(filePath);
-    const fileLastModified = profFile.lastModifiedTime = profFile.lastModifiedTime - 1000;
-    const serverTime = Date.now();
-
-    await client.maybeSync(3001, serverTime);
-
-    // File was updated.
-    notEqual(fileLastModified, profFile.lastModifiedTime);
-    const content = await readJSON(profFile.path);
-    deepEqual(content.data.map((r) => r.blockID), testData);
-    // Server time was updated.
-    const after = Services.prefs.getIntPref(client.lastCheckTimePref);
-    equal(after, Math.round(serverTime / 1000));
-  }
-});
-add_task(clear_state);
-
-add_task(async function test_sends_reload_message_when_blocklist_has_changes() {
-  for (let {client} of gBlocklistClients) {
-    let received = await new Promise((resolve, reject) => {
-      Services.ppmm.addMessageListener("Blocklist:reload-from-disk", {
-        receiveMessage(aMsg) { resolve(aMsg); }
-      });
-
-      client.maybeSync(2000, Date.now() - 1000, {loadDump: false});
-    });
-
-    equal(received.data.filename, client.filename);
-  }
-});
-add_task(clear_state);
-
 add_task(async function test_sync_event_data_is_filtered_for_target() {
   // Here we will synchronize 4 times, the first two to initialize the local DB and
   // the last two about event filtered data.
   const timestamp1 = 2000;
   const timestamp2 = 3000;
   const timestamp3 = 4000;
   const timestamp4 = 5000;
   // Fake a date value obtained from server (used to store a pref, useless here).
--- a/services/settings/remote-settings.js
+++ b/services/settings/remote-settings.js
@@ -187,21 +187,21 @@ async function fetchLatestChanges(url, l
     }
   }
 
   return {changes, currentEtag, serverTimeMillis, backoffSeconds};
 }
 
 /**
  * Load the the JSON file distributed with the release for this collection.
+ * @param {String} bucket
+ * @param {String} collection
  */
-async function loadDumpFile(filename) {
-  // Replace OS specific path separator by / for URI.
-  const { components: folderFile } = OS.Path.split(filename);
-  const fileURI = `resource://app/defaults/settings/${folderFile.join("/")}`;
+async function loadDumpFile(bucket, collection) {
+  const fileURI = `resource://app/defaults/settings/${bucket}/${collection}.json`;
   const response = await fetch(fileURI);
   if (!response.ok) {
     throw new Error(`Could not read from '${fileURI}'`);
   }
   // Will be rejected if JSON is invalid.
   return response.json();
 }
 
@@ -220,22 +220,16 @@ class RemoteSettingsClient {
 
     this._kinto = null;
   }
 
   get identifier() {
     return `${this.bucketName}/${this.collectionName}`;
   }
 
-  get filename() {
-    // Replace slash by OS specific path separator (eg. Windows)
-    const identifier = OS.Path.join(...this.identifier.split("/"));
-    return `${identifier}.json`;
-  }
-
   get lastCheckTimePref() {
     return this._lastCheckTimePref || `services.settings.${this.bucketName}.${this.collectionName}.last_check`;
   }
 
   /**
    * Event emitter: will execute the registered listeners in the order and
    * sequentially.
    *
@@ -297,17 +291,17 @@ class RemoteSettingsClient {
     const { filters = {}, order } = options;
     const c = await this.openCollection();
 
     const timestamp = await c.db.getLastModified();
     // If the local database was never synchronized, then we attempt to load
     // a packaged JSON dump.
     if (timestamp == null) {
       try {
-        const { data } = await loadDumpFile(this.filename);
+        const { data } = await loadDumpFile(this.bucketName, this.collectionName);
         await c.loadDump(data);
       } catch (e) {
         // Report but return an empty list since there will be no data anyway.
         Cu.reportError(e);
         return [];
       }
     }
 
@@ -348,17 +342,17 @@ class RemoteSettingsClient {
       let collectionLastModified = await collection.db.getLastModified();
 
       // If there is no data currently in the collection, attempt to import
       // initial data from the application defaults.
       // This allows to avoid synchronizing the whole collection content on
       // cold start.
       if (!collectionLastModified && loadDump) {
         try {
-          const initialData = await loadDumpFile(this.filename);
+          const initialData = await loadDumpFile(this.bucketName, this.collectionName);
           await collection.loadDump(initialData.data);
           collectionLastModified = await collection.db.getLastModified();
         } catch (e) {
           // Report but go-on.
           Cu.reportError(e);
         }
       }
 
@@ -581,19 +575,18 @@ async function databaseExists(bucket, co
 /**
  * Check if we ship a JSON dump for the specified bucket and collection.
  *
  * @param {String} bucket
  * @param {String} collection
  * @return {bool} Whether it is present or not.
  */
 async function hasLocalDump(bucket, collection) {
-  const filename = OS.Path.join(bucket, `${collection}.json`);
   try {
-    await loadDumpFile(filename);
+    await loadDumpFile(bucket, collection);
     return true;
   } catch (e) {
     return false;
   }
 }
 
 
 function remoteSettingsFunction() {