Bug 1369782 - Require user interaction for downloads.open() r=aswan
☠☠ backed out by 44a734e1209d ☠ ☠
authorMark Striemer <mstriemer@mozilla.com>
Fri, 02 Jun 2017 18:34:08 -0500
changeset 411455 fe04c0fec69b97608c8b7090e4b7e6ff73ac0f6d
parent 411454 9dbf22ca29326b84af6c9c71dac4ed0fa94d4a5f
child 411456 25b6519f489d6c103f794efcbeb0a2266d987fbd
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1369782
milestone55.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 1369782 - Require user interaction for downloads.open() r=aswan MozReview-Commit-ID: 9RkIOfZEfnf
toolkit/components/extensions/ext-c-downloads.js
toolkit/components/extensions/ext-c-toolkit.js
toolkit/components/extensions/ext-downloads.js
toolkit/components/extensions/jar.mn
toolkit/components/extensions/test/mochitest/chrome.ini
toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_open.html
toolkit/components/extensions/test/xpcshell/test_ext_downloads.js
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/ext-c-downloads.js
@@ -0,0 +1,22 @@
+"use strict";
+
+var {
+  ExtensionError,
+} = ExtensionUtils;
+
+this.downloads = class extends ExtensionAPI {
+  getAPI(context) {
+    return {
+      downloads: {
+        open(downloadId) {
+          let winUtils = context.contentWindow.getInterface(Ci.nsIDOMWindowUtils);
+          if (!winUtils.isHandlingUserInput) {
+            throw new ExtensionError("May only open downloads from a user input handler");
+          }
+
+          return context.childManager.callParentAsyncFunction("downloads.open_parent", [downloadId]);
+        },
+      },
+    };
+  }
+};
--- a/toolkit/components/extensions/ext-c-toolkit.js
+++ b/toolkit/components/extensions/ext-c-toolkit.js
@@ -36,16 +36,23 @@ extensions.registerModules({
     url: "chrome://extensions/content/ext-c-backgroundPage.js",
     scopes: ["addon_child"],
     manifest: ["background"],
     paths: [
       ["extension", "getBackgroundPage"],
       ["runtime", "getBackgroundPage"],
     ],
   },
+  downloads: {
+    url: "chrome://extensions/content/ext-c-downloads.js",
+    scopes: ["addon_child"],
+    paths: [
+      ["downloads"],
+    ],
+  },
   extension: {
     url: "chrome://extensions/content/ext-c-extension.js",
     scopes: ["addon_child", "content_child", "devtools_child", "proxy_script"],
     paths: [
       ["extension"],
     ],
   },
   i18n: {
--- a/toolkit/components/extensions/ext-downloads.js
+++ b/toolkit/components/extensions/ext-downloads.js
@@ -640,17 +640,17 @@ this.downloads = class extends Extension
             for (let item of items) {
               promises.push(DownloadMap.erase(item));
               results.push(item.id);
             }
             return Promise.all(promises).then(() => results);
           });
         },
 
-        open(downloadId) {
+        open_parent(downloadId) {
           return DownloadMap.lazyInit().then(() => {
             let download = DownloadMap.fromId(downloadId).download;
             if (download.succeeded) {
               return download.launch();
             }
             return Promise.reject({message: "Download has not completed."});
           }).catch((error) => {
             return Promise.reject({message: error.message});
--- a/toolkit/components/extensions/jar.mn
+++ b/toolkit/components/extensions/jar.mn
@@ -25,16 +25,17 @@ toolkit.jar:
     content/extensions/ext-theme.js
     content/extensions/ext-toolkit.js
     content/extensions/ext-topSites.js
     content/extensions/ext-webRequest.js
     content/extensions/ext-webNavigation.js
     # Below is a separate group using the naming convention ext-c-*.js that run
     # in the child process.
     content/extensions/ext-c-backgroundPage.js
+    content/extensions/ext-c-downloads.js
     content/extensions/ext-c-extension.js
 #ifndef ANDROID
     content/extensions/ext-c-identity.js
 #endif
     content/extensions/ext-c-permissions.js
     content/extensions/ext-c-runtime.js
     content/extensions/ext-c-storage.js
     content/extensions/ext-c-test.js
--- a/toolkit/components/extensions/test/mochitest/chrome.ini
+++ b/toolkit/components/extensions/test/mochitest/chrome.ini
@@ -12,16 +12,17 @@ support-files =
   oauth.html
   redirect_auto.sjs
 tags = webextensions in-process-webextensions
 
 [test_chrome_ext_background_page.html]
 skip-if = (toolkit == 'android') # android doesn't have devtools
 [test_chrome_ext_contentscript_data_uri.html]
 [test_chrome_ext_contentscript_unrecognizedprop_warning.html]
+[test_chrome_ext_downloads_open.html]
 [test_chrome_ext_downloads_saveAs.html]
 [test_chrome_ext_eventpage_warning.html]
 [test_chrome_ext_hybrid_addons.html]
 [test_chrome_ext_idle.html]
 [test_chrome_ext_identity.html]
 skip-if = os == 'android' # unsupported.
 [test_chrome_ext_permissions.html]
 skip-if = os == 'android' # Bug 1350559
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_open.html
@@ -0,0 +1,112 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for permissions</title>
+  <script src="chrome://mochikit/content/tests/SimpleTest/SpawnTask.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <link rel="stylesheet" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+add_task(async function test_downloads_open_permission() {
+  function backgroundScript() {
+    browser.test.assertEq(browser.downloads.open, undefined,
+                             "`downloads.open` permission is required.");
+    browser.test.notifyPass("downloads tests");
+  }
+
+  let extensionData = {
+    background: backgroundScript,
+    manifest: {
+      permissions: ["downloads"],
+    },
+  };
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  await extension.startup();
+  await extension.awaitFinish("downloads tests");
+  await extension.unload();
+});
+
+add_task(async function test_downloads_open_requires_user_interaction() {
+  async function backgroundScript() {
+    await browser.test.assertRejects(
+      browser.downloads.open(10),
+      "May only open downloads from a user input handler",
+      "The error is informative.");
+
+    browser.test.notifyPass("downloads tests");
+  }
+
+  let extensionData = {
+    background: backgroundScript,
+    manifest: {
+      permissions: ["downloads", "downloads.open"],
+    },
+  };
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  await extension.startup();
+  await extension.awaitFinish("downloads tests");
+  await extension.unload();
+});
+
+add_task(async function downloads_open_invalid_id() {
+  async function pageScript() {
+    window.addEventListener("keypress", () => {
+      try {
+        browser.downloads.open(10);
+        browser.test.sendMessage("download-open.result", { success: true });
+      } catch (e) {
+        browser.test.sendMessage("download-open.result", {
+          success: false,
+          error: e.message,
+        });
+      }
+    });
+
+    browser.test.sendMessage("page-ready");
+  }
+
+  let extensionData = {
+    background() {
+      browser.test.sendMessage("ready", browser.runtime.getURL("page.html"));
+    },
+    files: {
+      "foo.txt": "It's the file called foo.txt.",
+      "page.html": `<html><head>
+        <script src="page.js"><\/script>
+      </head></html>`,
+      "page.js": pageScript,
+    },
+    manifest: {
+      permissions: ["downloads", "downloads.open"],
+    },
+  };
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  await extension.startup();
+
+  let url = await extension.awaitMessage("ready");
+  let win = window.open(url);
+  await extension.awaitMessage("page-ready");
+
+  let winutils = SpecialPowers.getDOMWindowUtils(win);
+  winutils.sendKeyEvent("keypress", KeyEvent.DOM_VK_A, 0, 0);
+  let result = await extension.awaitMessage("download-open.result");
+
+  is(result.success, false, "Opening download fails.");
+  is(result.error, "Invalid download id 10", "The error is informative.");
+
+
+  await extension.unload();
+});
+
+</script>
+
+</body>
+</html>
--- a/toolkit/components/extensions/test/xpcshell/test_ext_downloads.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_downloads.js
@@ -23,54 +23,8 @@ add_task(async function test_downloads_a
     },
   };
 
   let extension = ExtensionTestUtils.loadExtension(extensionData);
   await extension.startup();
   await extension.awaitFinish("downloads tests");
   await extension.unload();
 });
-
-add_task(async function test_downloads_open_permission() {
-  function backgroundScript() {
-    browser.test.assertEq(browser.downloads.open, undefined,
-                             "`downloads.open` permission is required.");
-    browser.test.notifyPass("downloads tests");
-  }
-
-  let extensionData = {
-    background: backgroundScript,
-    manifest: {
-      permissions: ["downloads"],
-    },
-  };
-
-  let extension = ExtensionTestUtils.loadExtension(extensionData);
-  await extension.startup();
-  await extension.awaitFinish("downloads tests");
-  await extension.unload();
-});
-
-add_task(async function test_downloads_open() {
-  async function backgroundScript() {
-    await browser.test.assertRejects(
-      browser.downloads.open(10),
-      "Invalid download id 10",
-      "The error is informative.");
-
-    browser.test.notifyPass("downloads tests");
-
-    // TODO: Once downloads.{pause,cancel,resume} lands (bug 1245602) test that this gives a good
-    // error when called with an incompleted download.
-  }
-
-  let extensionData = {
-    background: backgroundScript,
-    manifest: {
-      permissions: ["downloads", "downloads.open"],
-    },
-  };
-
-  let extension = ExtensionTestUtils.loadExtension(extensionData);
-  await extension.startup();
-  await extension.awaitFinish("downloads tests");
-  await extension.unload();
-});