Bug 1519762 Fix a regression with browser.downloads.search() r=kmag
authorAndrew Swan <aswan@mozilla.com>
Thu, 17 Jan 2019 11:12:46 -0800
changeset 514386 dd74d02dfc11bd73afe4a3e3ae2def4c866fa98f
parent 514385 732c870d85fcb67eb1a9e770d6200058332462a0
child 514387 1fc8677c25f0fc9a7e55fd82984c30653b204914
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1519762, 1503760
milestone66.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 1519762 Fix a regression with browser.downloads.search() r=kmag The fixes in bug 1503760 inadvertently broke searches of downloads that have not yet started receiving data (which have a value of -1 for the totalBytes property). That regression is fixed here. Differential Revision: https://phabricator.services.mozilla.com/D16889
toolkit/components/extensions/parent/ext-downloads.js
toolkit/components/extensions/schemas/downloads.json
toolkit/components/extensions/test/xpcshell/test_ext_downloads_search.js
--- a/toolkit/components/extensions/parent/ext-downloads.js
+++ b/toolkit/components/extensions/parent/ext-downloads.js
@@ -261,18 +261,18 @@ const downloadQuery = query => {
     return ExtensionCommon.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);
 
-  const totalBytesGreater = query.totalBytesGreater;
-  const totalBytesLess = query.totalBytesLess != null ? query.totalBytesLess : Number.MAX_VALUE;
+  const totalBytesGreater = query.totalBytesGreater !== null ? query.totalBytesGreater : -1;
+  const totalBytesLess = query.totalBytesLess !== null ? query.totalBytesLess : Number.MAX_VALUE;
 
   // Handle options for which we can have a regular expression and/or
   // an explicit value to match.
   function makeMatch(regex, value, field) {
     if (value == null && regex == null) {
       return input => true;
     }
 
@@ -318,17 +318,17 @@ const downloadQuery = query => {
       }
     } else if (item.startTime > startedBefore || item.startTime < startedAfter) {
       return false;
     }
 
     // todo endedBefore, endedAfter
 
     if (item.totalBytes == -1) {
-      if (query.totalBytesGreater != null || query.totalBytesLess != null) {
+      if (query.totalBytesGreater !== null || query.totalBytesLess !== null) {
         return false;
       }
     } else if (item.totalBytes <= totalBytesGreater || item.totalBytes >= totalBytesLess) {
       return false;
     }
 
     // todo: include danger
     const SIMPLE_ITEMS = ["id", "mime", "startTime", "endTime", "state",
--- a/toolkit/components/extensions/schemas/downloads.json
+++ b/toolkit/components/extensions/schemas/downloads.json
@@ -107,17 +107,18 @@
             "type": "boolean"
           },
           "danger": {
             "$ref": "DangerType",
             "description": "Indication of whether this download is thought to be safe or known to be suspicious."
           },
           "mime": {
             "description": "The file's MIME type.",
-            "type": "string"
+            "type": "string",
+            "optional": true
           },
           "startTime": {
             "description": "Number of milliseconds between the unix epoch and when this download began.",
             "type": "string"
           },
           "endTime": {
             "description": "Number of milliseconds between the unix epoch and when this download ended.",
             "optional": true,
@@ -252,18 +253,17 @@
           "endedAfter": {
             "description": "Limits results to downloads that ended after the given ms since the epoch.",
             "optional": true,
             "$ref": "DownloadTime"
           },
           "totalBytesGreater": {
             "description": "Limits results to downloads whose totalBytes is greater than the given integer.",
             "optional": true,
-            "type": "number",
-            "default": -1
+            "type": "number"
           },
           "totalBytesLess": {
             "description": "Limits results to downloads whose totalBytes is less than the given integer.",
             "optional": true,
             "type": "number"
           },
           "filenameRegex": {
             "description": "Limits results to <a href='#type-DownloadItem'>DownloadItems</a> whose <code>filename</code> matches the given regular expression.",
--- a/toolkit/components/extensions/test/xpcshell/test_ext_downloads_search.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_search.js
@@ -1,12 +1,14 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
+PromiseTestUtils.whitelistRejectionsGlobally(/Message manager disconnected/);
+
 ChromeUtils.import("resource://gre/modules/Downloads.jsm");
 
 const server = createHttpServer();
 server.registerDirectory("/data/", do_get_file("data"));
 
 const BASE = `http://localhost:${server.identity.primaryPort}/data`;
 const TXT_FILE = "file_download.txt";
 const TXT_URL = BASE + "/" + TXT_FILE;
@@ -419,8 +421,54 @@ add_task(async function test_search() {
   await checkBadSearch({endedAfter: "i am not a time"}, /Type error/, "query.endedAfter is not a valid time");
   await checkBadSearch({urlRegex: "["}, /Invalid urlRegex/, "query.urlRegexp is not a valid regular expression");
   await checkBadSearch({filenameRegex: "["}, /Invalid filenameRegex/, "query.filenameRegexp is not a valid regular expression");
   await checkBadSearch({orderBy: "startTime"}, /Expected array/, "query.orderBy is not an array");
   await checkBadSearch({orderBy: ["bogus"]}, /Invalid orderBy field/, "query.orderBy references a non-existent field");
 
   await extension.unload();
 });
+
+// Test that downloads with totalBytes of -1 (ie, that have not yet started)
+// work properly.  See bug 1519762 for details of a past regression in
+// this area.
+add_task(async function test_inprogress() {
+  let resume, resumePromise = new Promise(resolve => { resume = resolve; });
+  server.registerPathHandler("/slow", async (request, response) => {
+    response.processAsync();
+    await resumePromise;
+    response.setHeader("Content-type", "text/plain");
+    response.write("");
+    response.finish();
+  });
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["downloads"],
+    },
+    background() {
+      browser.test.onMessage.addListener(async (msg, url) => {
+        let id = await browser.downloads.download({url});
+        let full = await browser.downloads.search({id});
+
+        browser.test.assertEq(full.length, 1,
+                              "Found new download in search results");
+        browser.test.assertEq(full[0].totalBytes, -1,
+                              "New download still has totalBytes == -1");
+
+        browser.downloads.onChanged.addListener(info => {
+          if (info.id == id && info.state.current == "complete") {
+            browser.test.notifyPass("done");
+          }
+        });
+
+        browser.test.sendMessage("started");
+      });
+    },
+  });
+
+  await extension.startup();
+  extension.sendMessage("go", `${BASE}/slow`);
+  await extension.awaitMessage("started");
+  resume();
+  await extension.awaitFinish("done");
+  await extension.unload();
+});