Bug 1339868 - browsingData.settings includes an invalid property in its returned objects. r=aswan, a=gchang
authorBob Silverberg <bsilverberg@mozilla.com>
Wed, 15 Feb 2017 12:39:03 -0500
changeset 378667 afd377db9e37dc3460f5d8acf3f7fa1af8a5f049
parent 378666 c2afe6af497e9e4b6510c3f5f852e300f808a728
child 378668 3383ac0e02e1a0c308c1e5c0f67e2796d54541f5
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, gchang
bugs1339868
milestone53.0a2
Bug 1339868 - browsingData.settings includes an invalid property in its returned objects. r=aswan, a=gchang Also fixes browsingData.settings() does not work if "Time range to clear" is set to "Everything". MozReview-Commit-ID: 5GtwYF6xCSj
browser/components/extensions/ext-browsingData.js
browser/components/extensions/test/xpcshell/test_ext_browsingData_settings.js
--- a/browser/components/extensions/ext-browsingData.js
+++ b/browser/components/extensions/ext-browsingData.js
@@ -182,31 +182,34 @@ extensions.registerSchemaAPI("browsingDa
       settings() {
         const PREF_DOMAIN = "privacy.cpd.";
         // The following prefs are the only ones in Firefox that match corresponding
         // values used by Chrome when rerturning settings.
         const PREF_LIST = ["cache", "cookies", "history", "formdata", "downloads"];
 
         // since will be the start of what is returned by Sanitizer.getClearRange
         // divided by 1000 to convert to ms.
-        let since = Sanitizer.getClearRange()[0] / 1000;
+        // If Sanitizer.getClearRange returns undefined that means the range is
+        // currently "Everything", so we should set since to 0.
+        let clearRange = Sanitizer.getClearRange();
+        let since = clearRange ? clearRange[0] / 1000 : 0;
         let options = {since};
 
         let dataToRemove = {};
         let dataRemovalPermitted = {};
 
         for (let item of PREF_LIST) {
-          dataToRemove[item] = Preferences.get(`${PREF_DOMAIN}${item}`);
+          // The property formData needs a different case than the
+          // formdata preference.
+          const name = item === "formdata" ? "formData" : item;
+          dataToRemove[name] = Preferences.get(`${PREF_DOMAIN}${item}`);
           // Firefox doesn't have the same concept of dataRemovalPermitted
           // as Chrome, so it will always be true.
-          dataRemovalPermitted[item] = true;
+          dataRemovalPermitted[name] = true;
         }
-        // formData has a different case than the pref formdata.
-        dataToRemove.formData = Preferences.get(`${PREF_DOMAIN}formdata`);
-        dataRemovalPermitted.formData = true;
 
         return Promise.resolve({options, dataToRemove, dataRemovalPermitted});
       },
       remove(options, dataToRemove) {
         return doRemoval(options, dataToRemove, extension);
       },
       removeCache(options) {
         return doRemoval(options, {cache: true});
--- a/browser/components/extensions/test/xpcshell/test_ext_browsingData_settings.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_browsingData_settings.js
@@ -1,18 +1,21 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
+XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
+                                  "resource://gre/modules/Preferences.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Sanitizer",
                                   "resource:///modules/Sanitizer.jsm");
 
 const PREF_DOMAIN = "privacy.cpd.";
+const SETTINGS_LIST = ["cache", "cookies", "history", "formData", "downloads"].sort();
 
-add_task(function* testSettings() {
+add_task(function* testSettingsProperties() {
   function background() {
     browser.test.onMessage.addListener(msg => {
       browser.browsingData.settings().then(settings => {
         browser.test.sendMessage("settings", settings);
       });
     });
   }
 
@@ -20,57 +23,95 @@ add_task(function* testSettings() {
     background,
     manifest: {
       permissions: ["browsingData"],
     },
   });
 
   yield extension.startup();
 
-  let branch = Services.prefs.getBranch(PREF_DOMAIN);
-
   extension.sendMessage("settings");
   let settings = yield extension.awaitMessage("settings");
 
-  let since = Sanitizer.getClearRange()[0] / 1000;
-
-  // Because it is based on the current timestamp, we cannot know the exact
-  // value to expect for since, so allow a 10s variance.
-  ok(Math.abs(settings.options.since - since) < 10000,
-     "settings.options contains the expected since value.");
+  // Verify that we get the keys back we expect.
+  deepEqual(Object.keys(settings.dataToRemove).sort(), SETTINGS_LIST,
+    "dataToRemove contains expected properties.");
+  deepEqual(Object.keys(settings.dataRemovalPermitted).sort(), SETTINGS_LIST,
+    "dataToRemove contains expected properties.");
 
   let dataTypeSet = settings.dataToRemove;
   for (let key of Object.keys(dataTypeSet)) {
-    equal(branch.getBoolPref(key.toLowerCase()), dataTypeSet[key], `${key} property of dataToRemove matches the expected pref.`);
+    equal(Preferences.get(`${PREF_DOMAIN}${key.toLowerCase()}`), dataTypeSet[key],
+      `${key} property of dataToRemove matches the expected pref.`);
   }
 
   dataTypeSet = settings.dataRemovalPermitted;
   for (let key of Object.keys(dataTypeSet)) {
     equal(true, dataTypeSet[key], `${key} property of dataRemovalPermitted is true.`);
   }
 
   // Explicitly set a pref to both true and false and then check.
-  const SINGLE_PREF = "cache";
+  const SINGLE_OPTION = "cache";
+  const SINGLE_PREF = "privacy.cpd.cache";
 
   do_register_cleanup(() => {
-    branch.clearUserPref(SINGLE_PREF);
+    Preferences.reset(SINGLE_PREF);
   });
 
-  branch.setBoolPref(SINGLE_PREF, true);
+  Preferences.set(SINGLE_PREF, true);
+
+  extension.sendMessage("settings");
+  settings = yield extension.awaitMessage("settings");
+  equal(settings.dataToRemove[SINGLE_OPTION], true, "Preference that was set to true returns true.");
+
+  Preferences.set(SINGLE_PREF, false);
 
   extension.sendMessage("settings");
   settings = yield extension.awaitMessage("settings");
-
-  equal(settings.dataToRemove[SINGLE_PREF], true, "Preference that was set to true returns true.");
-
-  branch.setBoolPref(SINGLE_PREF, false);
-
-  extension.sendMessage("settings");
-  settings = yield extension.awaitMessage("settings");
-
-  equal(settings.dataToRemove[SINGLE_PREF], false, "Preference that was set to false returns false.");
-
-  do_register_cleanup(() => {
-    branch.clearUserPref(SINGLE_PREF);
-  });
+  equal(settings.dataToRemove[SINGLE_OPTION], false, "Preference that was set to false returns false.");
 
   yield extension.unload();
 });
+
+add_task(function* testSettingsSince() {
+  const TIMESPAN_PREF = "privacy.sanitize.timeSpan";
+  const TEST_DATA = {
+    TIMESPAN_5MIN: Date.now() - 5 * 60 * 1000,
+    TIMESPAN_HOUR: Date.now() - 60 * 60 * 1000,
+    TIMESPAN_2HOURS: Date.now() - 2 * 60 * 60 * 1000,
+    TIMESPAN_EVERYTHING: 0,
+  };
+
+  function background() {
+    browser.test.onMessage.addListener(msg => {
+      browser.browsingData.settings().then(settings => {
+        browser.test.sendMessage("settings", settings);
+      });
+    });
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    background,
+    manifest: {
+      permissions: ["browsingData"],
+    },
+  });
+
+  yield extension.startup();
+
+  do_register_cleanup(() => {
+    Preferences.reset(TIMESPAN_PREF);
+  });
+
+  for (let timespan in TEST_DATA) {
+    Preferences.set(TIMESPAN_PREF, Sanitizer[timespan]);
+
+    extension.sendMessage("settings");
+    let settings = yield extension.awaitMessage("settings");
+
+    // Because it is based on the current timestamp, we cannot know the exact
+    // value to expect for since, so allow a 10s variance.
+    ok(Math.abs(settings.options.since - TEST_DATA[timespan]) < 10000,
+       "settings.options contains the expected since value.");
+  }
+
+  yield extension.unload();
+});