Bug 1469525 - Remove blocklist clients files from profile r=Gijs
authorMathieu Leplatre <mathieu@mozilla.com>
Mon, 06 Aug 2018 17:34:28 +0200
changeset 430334 ace07e18cb34adfa923f2654edd86562f5594aa0
parent 430333 2db828f877c866a4908eb159ba5ad70be5f0f17f
child 430335 b0703897aa8a89549a1cdf4154e2d19ca0021778
push id67433
push usermleplatre@mozilla.com
push dateTue, 07 Aug 2018 12:30:17 +0000
treeherderautoland@ace07e18cb34 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1469525
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -1800,17 +1800,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);
@@ -2137,16 +2137,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() {