Bug 1265836 - Part 2: Change normalizeTime to return a date rather than a number. r=aswan
authorBob Silverberg <bsilverberg@mozilla.com>
Tue, 24 May 2016 09:00:17 -0400
changeset 341212 e83eea3e0b6a99fa52c1c5db4b2db91f2029cf12
parent 341211 50edd919870a15dd9e2bd31441882ad18111c1c4
child 341213 d328f6474fcb7a701fafd4a58435674c53a29aeb
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1265836
milestone49.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 1265836 - Part 2: Change normalizeTime to return a date rather than a number. r=aswan Also update browser.history.deleteRange to use normalizeTime MozReview-Commit-ID: EQ3NLSIRTe8
browser/components/extensions/ext-history.js
browser/components/extensions/schemas/history.json
browser/components/extensions/test/browser/browser_ext_history.js
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/ext-downloads.js
--- a/browser/components/extensions/ext-history.js
+++ b/browser/components/extensions/ext-history.js
@@ -48,18 +48,18 @@ function convertNavHistoryContainerResul
 extensions.registerSchemaAPI("history", "history", (extension, context) => {
   return {
     history: {
       deleteAll: function() {
         return History.clear();
       },
       deleteRange: function(filter) {
         let newFilter = {
-          beginDate: new Date(filter.startTime),
-          endDate: new Date(filter.endTime),
+          beginDate: normalizeTime(filter.startTime),
+          endDate: normalizeTime(filter.endTime),
         };
         // History.removeVisitsByFilter returns a boolean, but our API should return nothing
         return History.removeVisitsByFilter(newFilter).then(() => undefined);
       },
       deleteUrl: function(details) {
         let url = details.url;
         // History.remove returns a boolean, but our API should return nothing
         return History.remove(url).then(() => undefined);
--- a/browser/components/extensions/schemas/history.json
+++ b/browser/components/extensions/schemas/history.json
@@ -240,22 +240,22 @@
         "description": "Removes all items within the specified date range from the history.  Pages will not be removed from the history unless all visits fall within the range.",
         "async": "callback",
         "parameters": [
           {
             "name": "range",
             "type": "object",
             "properties": {
               "startTime": {
-                "type": "number",
-                "description": "Items added to history after this date, represented in milliseconds since the epoch."
+                "$ref": "HistoryTime",
+                "description": "Items added to history after this date."
               },
               "endTime": {
-                "type": "number",
-                "description": "Items added to history before this date, represented in milliseconds since the epoch."
+                "$ref": "HistoryTime",
+                "description": "Items added to history before this date."
               }
             }
           },
           {
             "name": "callback",
             "type": "function",
             "parameters": []
           }
--- a/browser/components/extensions/test/browser/browser_ext_history.js
+++ b/browser/components/extensions/test/browser/browser_ext_history.js
@@ -66,30 +66,30 @@ add_task(function* test_delete() {
   let testUrl = visits[6].uri.spec;
   ok(yield PlacesTestUtils.isPageInDB(testUrl), "expected url found in history database");
 
   extension.sendMessage("delete-url", testUrl);
   yield extension.awaitMessage("url-deleted");
   is(yield PlacesTestUtils.isPageInDB(testUrl), false, "expected url not found in history database");
 
   let filter = {
-    startTime: PlacesUtils.toDate(visits[1].visitDate).valueOf(),
-    endTime: PlacesUtils.toDate(visits[3].visitDate).valueOf(),
+    startTime: PlacesUtils.toDate(visits[1].visitDate),
+    endTime: PlacesUtils.toDate(visits[3].visitDate),
   };
 
   extension.sendMessage("delete-range", filter);
   yield extension.awaitMessage("range-deleted");
 
   ok(yield PlacesTestUtils.isPageInDB(visits[0].uri), "expected uri found in history database");
   is(yield PlacesTestUtils.visitsInDB(visits[0].uri), 2, "2 visits for uri found in history database");
   ok(yield PlacesTestUtils.isPageInDB(visits[5].uri), "expected uri found in history database");
   is(yield PlacesTestUtils.visitsInDB(visits[5].uri), 1, "1 visit for uri found in history database");
 
-  filter.startTime = PlacesUtils.toDate(visits[0].visitDate).valueOf();
-  filter.endTime = PlacesUtils.toDate(visits[5].visitDate).valueOf();
+  filter.startTime = PlacesUtils.toDate(visits[0].visitDate);
+  filter.endTime = PlacesUtils.toDate(visits[5].visitDate);
 
   extension.sendMessage("delete-range", filter);
   yield extension.awaitMessage("range-deleted");
 
   is(yield PlacesTestUtils.isPageInDB(visits[0].uri), false, "expected uri not found in history database");
   is(yield PlacesTestUtils.visitsInDB(visits[0].uri), 0, "0 visits for uri found in history database");
   is(yield PlacesTestUtils.isPageInDB(visits[5].uri), false, "expected uri not found in history database");
   is(yield PlacesTestUtils.visitsInDB(visits[5].uri), 0, "0 visits for uri found in history database");
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -1183,32 +1183,31 @@ class ChildAPIManager {
   hasListener(path, name, listener) {
     let ref = path.concat(name).join(".");
     let set = this.listeners.get(ref) || new Set();
     return set.has(listener);
   }
 }
 
 /**
- * Returns a number which represents the number of milliseconds since the epoch
- * for either a startDate or an endDate. Accepts several formats:
+ * Convert any of several different representations of a date/time to a Date object.
+ * Accepts several formats:
  * a Date object, an ISO8601 string, or a number of milliseconds since the epoch as
  * either a number or a string.
  *
  * @param date: (Date) or (String) or (Number)
  *      The date to convert.
- * @returns (Number)
- *      The number of milliseconds since the epoch for the date
+ * @returns (Date)
+ *      A Date object
  */
 function normalizeTime(date) {
   // Of all the formats we accept the "number of milliseconds since the epoch as a string"
   // is an outlier, everything else can just be passed directly to the Date constructor.
-  const result = new Date((typeof date == "string" && /^\d+$/.test(date))
+  return new Date((typeof date == "string" && /^\d+$/.test(date))
                         ? parseInt(date, 10) : date);
-  return result.valueOf();
 }
 
 this.ExtensionUtils = {
   detectLanguage,
   extend,
   flushJarCache,
   ignoreEvent,
   injectAPI,
--- a/toolkit/components/extensions/ext-downloads.js
+++ b/toolkit/components/extensions/ext-downloads.js
@@ -241,17 +241,17 @@ function downloadQuery(query) {
       }
     }
   }
 
   function normalizeDownloadTime(arg, before) {
     if (arg == null) {
       return before ? Number.MAX_VALUE : 0;
     } else {
-      return normalizeTime(arg);
+      return normalizeTime(arg).getTime();
     }
   }
 
   const startedBefore = normalizeDownloadTime(query.startedBefore, true);
   const startedAfter = normalizeDownloadTime(query.startedAfter, false);
   // const endedBefore = normalizeDownloadTime(query.endedBefore, true);
   // const endedAfter = normalizeDownloadTime(query.endedAfter, false);