Bug 1265836 - Part 2: Change normalizeTime to return a date rather than a number, r?aswan draft
authorBob Silverberg <bsilverberg@mozilla.com>
Tue, 24 May 2016 09:00:17 -0400
changeset 374249 0f337f8959c36d86eb910a1bff4d776819afe223
parent 374248 5ab58b826c42db6b1071dc1050a8217c4cc459f9
child 374250 02ab9d5a8fe84552ae2a7ea34c89dc116737dfc4
push id19960
push userbmo:bob.silverberg@gmail.com
push dateThu, 02 Jun 2016 02:42:32 +0000
reviewersaswan
bugs1265836
milestone49.0a1
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);