Bug 1519762 Fix a regression with browser.downloads.search() r=kmag
☠☠ backed out by cc2853aad2c4 ☠ ☠
authorAndrew Swan <aswan@mozilla.com>
Thu, 17 Jan 2019 19:20:58 +0000
changeset 511433 ee03992cef6e9e16a3bb99ab81d3b0468ff85faa
parent 511421 24982570fc8338ba51f32f142c7d0e6f342cce7d
child 511434 aee9f213f3c7b28b6c69ba0ecf4a91b4f9539ea1
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [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
@@ -252,18 +252,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();
+});