Implement chrome.experimental.downloads.onChanged

chrome/test/data/extensions/api_test/downloads/test.js

Index: chrome/test/data/extensions/api_test/downloads/test.js
diff --git a/chrome/test/data/extensions/api_test/downloads/test.js b/chrome/test/data/extensions/api_test/downloads/test.js
index 0a1368c87aaebfcd15db05b3747c253b60c14074..988a3939f2e5c84a46186580a31fc4a702d60be0 100644
--- a/chrome/test/data/extensions/api_test/downloads/test.js
+++ b/chrome/test/data/extensions/api_test/downloads/test.js
@@ -42,170 +42,457 @@ chrome.test.getConfig(function(testConfig) {
 
   // The "/slow" handler waits a specified amount of time before returning a
   // safe file. Specify zero seconds to return quickly.
-  var SAFE_FAST_URL = getURL('slow?0');
-  var NEVER_FINISH_URL = getURL('download-known-size');
-  var ERROR_GENERIC = downloads.ERROR_GENERIC;
-  var ERROR_INVALID_URL = downloads.ERROR_INVALID_URL;
-  var ERROR_INVALID_OPERATION = downloads.ERROR_INVALID_OPERATION;
+  const SAFE_FAST_URL = getURL('slow?0');

[asanka, 2012/02/13 19:45:47]

Avoid 'const', here and elsewhere. See http://crbug.com/80149 (not really a big
deal for test code, but is a style violation.)

[benjhayden, 2012/02/13 21:05:31]

Done.

The style guide suggested that the reason to avoid const is that it is
unsupported on IE, which is irrelevant here. Perhaps it should also mention that
it slows down V8, which is counter-intuitive.

+
+  const NEVER_FINISH_URL = getURL('download-known-size');
+
+  // This URL should only work with the POST method and a request body
+  // containing 'BODY'.
+  const POST_URL = getURL('files/post/downloads/a_zip_file.zip?' +
+                          'expected_body=BODY');
+
+  // This URL should only work with headers 'Foo: bar' and 'Qx: yo'.
+  const HEADERS_URL = getURL('files/downloads/a_zip_file.zip?' +
+                             'expected_headers=Foo:bar&expected_headers=Qx:yo');
 
   chrome.test.runTests([
     // TODO(benjhayden): Test onErased using remove().
+
+    // TODO(benjhayden): Sub-directories depend on http://crbug.com/109443
+    // TODO(benjhayden): Windows slashes.
+    // function downloadSubDirectoryFilename() {
+    //   const downloadId = getNextId();
+    //   var callbackCompleted = chrome.test.callbackAdded();
+    //   function myListener(delta) {
+    //     if ((delta.id != downloadId) ||
+    //         !delta.filename ||
+    //         (delta.filename.new.indexOf('/foo/slow') == -1))
+    //       return;
+    //     downloads.onChanged.removeListener(myListener);
+    //     callbackCompleted();
+    //   }
+    //   downloads.onChanged.addListener(myListener);
+    //   downloads.download(
+    //       {'url': SAFE_FAST_URL, 'filename': 'foo/slow'},
+    //       chrome.test.callback(function(id) {
+    //         chrome.test.assertEq(downloadId, id);
+    //       }));
+    // },
+
+    function downloadSimple() {
+      // Test that we can begin a download.
+      const downloadId = getNextId();
+      downloads.download(
+          {'url': SAFE_FAST_URL},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
+    function downloadPostSuccess() {
+      // Test the |method| download option.
+      const downloadId = getNextId();
+      var changedCompleted = chrome.test.callbackAdded();
+      function changedListener(delta) {
+        // Ignore onChanged events for downloads besides our own, or events that
+        // signal any change besides completion.
+        if ((delta.id != downloadId) ||
+            !delta.state ||
+            (delta.state.new != downloads.STATE_COMPLETE))
+          return;
+        downloads.search({id: downloadId},
+                          chrome.test.callback(function(items) {
+          chrome.test.assertEq(1, items.length);
+          chrome.test.assertEq(downloadId, items[0].id);
+          const EXPECTED_SIZE = 164;
+          chrome.test.assertEq(EXPECTED_SIZE, items[0].totalBytes);
+          chrome.test.assertEq(EXPECTED_SIZE, items[0].fileSize);
+          chrome.test.assertEq(EXPECTED_SIZE, items[0].bytesReceived);
+        }));
+        downloads.onChanged.removeListener(changedListener);
+        changedCompleted();
+      }
+      downloads.onChanged.addListener(changedListener);
+
+      downloads.download(
+          {'url': POST_URL,
+           'method': 'POST',
+           'filename': downloadId + '.txt',  // Prevent 'file' danger.
+           'body': 'BODY'},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
+    function downloadPostWouldFailWithoutMethod() {
+      // Test that downloadPostSuccess would fail if the resource requires the
+      // POST method, and chrome fails to propagate the |method| parameter back
+      // to the server. This tests both that testserver.py does not succeed when
+      // it should fail, and this tests how the downloads extension api exposes
+      // the failure to extensions.
+      const downloadId = getNextId();
+
+      var changedCompleted = chrome.test.callbackAdded();
+      function changedListener(delta) {
+        // Ignore onChanged events for downloads besides our own, or events that
+        // signal any change besides interruption.
+        if ((delta.id != downloadId) ||
+            !delta.state ||
+            (delta.state.new != downloads.STATE_COMPLETE))
+          return;
+        // TODO(benjhayden): Change COMPLETE to INTERRUPTED after
+        // http://crbug.com/112342
+        downloads.search({id: downloadId},
+                          chrome.test.callback(function(items) {
+          chrome.test.assertEq(1, items.length);
+          chrome.test.assertEq(downloadId, items[0].id);
+          chrome.test.assertEq(0, items[0].totalBytes);
+        }));
+        downloads.onChanged.removeListener(changedListener);
+        changedCompleted();
+      }
+      downloads.onChanged.addListener(changedListener);
+
+      downloads.download(
+          {'url': POST_URL,
+           'filename': downloadId + '.txt',  // Prevent 'file' danger.
+           'body': 'BODY'},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
+    function downloadPostWouldFailWithoutBody() {
+      // Test that downloadPostSuccess would fail if the resource requires the
+      // POST method and a request body, and chrome fails to propagate the
+      // |body| parameter back to the server. This tests both that testserver.py
+      // does not succeed when it should fail, and this tests how the downloads
+      // extension api exposes the failure to extensions.
+      const downloadId = getNextId();
+
+      var changedCompleted = chrome.test.callbackAdded();
+      function changedListener(delta) {
+        // Ignore onChanged events for downloads besides our own, or events that
+        // signal any change besides interruption.
+        if ((delta.id != downloadId) ||
+            !delta.state ||
+            (delta.state.new != downloads.STATE_COMPLETE))
+          return;
+        // TODO(benjhayden): Change COMPLETE to INTERRUPTED after
+        // http://crbug.com/112342
+        downloads.search({id: downloadId},
+                          chrome.test.callback(function(items) {
+          chrome.test.assertEq(1, items.length);
+          chrome.test.assertEq(downloadId, items[0].id);
+          chrome.test.assertEq(0, items[0].totalBytes);
+        }));
+        downloads.onChanged.removeListener(changedListener);
+        changedCompleted();
+      }
+      downloads.onChanged.addListener(changedListener);
+
+      downloads.download(
+          {'url': POST_URL,
+           'filename': downloadId + '.txt',  // Prevent 'file' danger.
+           'method': 'POST'},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
+    function downloadHeadersSuccess() {
+      // Test the |header| download option.
+      const downloadId = getNextId();
+      var changedCompleted = chrome.test.callbackAdded();
+      function changedListener(delta) {
+        // Ignore onChanged events for downloads besides our own, or events that
+        // signal any change besides completion.
+        if ((delta.id != downloadId) ||
+            !delta.state ||
+            (delta.state.new != downloads.STATE_COMPLETE))
+          return;
+        downloads.search({id: downloadId},
+                          chrome.test.callback(function(items) {
+          chrome.test.assertEq(1, items.length);
+          chrome.test.assertEq(downloadId, items[0].id);
+          const EXPECTED_SIZE = 164;
+          chrome.test.assertEq(EXPECTED_SIZE, items[0].totalBytes);
+          chrome.test.assertEq(EXPECTED_SIZE, items[0].fileSize);
+          chrome.test.assertEq(EXPECTED_SIZE, items[0].bytesReceived);
+        }));
+        downloads.onChanged.removeListener(changedListener);
+        changedCompleted();
+      }
+      downloads.onChanged.addListener(changedListener);
+
+      downloads.download(
+          {'url': HEADERS_URL,
+           'filename': downloadId + '.txt',  // Prevent 'file' danger.
+           'headers': [{'name': 'Foo', 'value': 'bar'},
+                       {'name': 'Qx', 'value': 'yo'}]},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
+    function downloadHeadersWouldFail() {
+      // Test that downloadHeadersSuccess() would fail if the resource requires
+      // the headers, and chrome fails to propagate them back to the server.
+      // This tests both that testserver.py does not succeed when it should
+      // fail as well as how the downloads extension api exposes the
+      // failure to extensions.
+      const downloadId = getNextId();
+
+      var changedCompleted = chrome.test.callbackAdded();
+      function changedListener(delta) {
+        // Ignore onChanged events for downloads besides our own, or events that
+        // signal any change besides interruption.
+        if ((delta.id != downloadId) ||
+            !delta.state ||
+            (delta.state.new != downloads.STATE_COMPLETE))
+          return;
+        // TODO(benjhayden): Change COMPLETE to INTERRUPTED after
+        // http://crbug.com/112342
+        downloads.search({id: downloadId},
+                          chrome.test.callback(function(items) {
+          chrome.test.assertEq(1, items.length);
+          chrome.test.assertEq(downloadId, items[0].id);
+          chrome.test.assertEq(0, items[0].totalBytes);
+        }));
+        downloads.onChanged.removeListener(changedListener);
+        changedCompleted();
+      }
+      downloads.onChanged.addListener(changedListener);
+
+      downloads.download(
+          {'url': HEADERS_URL},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
+    function downloadInterrupted() {
+      // Test that cancel()ing an in-progress download causes its state to
+      // transition to interrupted, and test that that state transition is
+      // detectable by an onChanged event listener.
+      // TODO(benjhayden): Test other sources of interruptions such as server
+      // death.
+      const downloadId = getNextId();
+
+      var createdCompleted = chrome.test.callbackAdded();
+      function createdListener(createdItem) {
+        // Ignore onCreated events for any download besides our own.
+        if (createdItem.id != downloadId)
+          return;
+        // TODO(benjhayden) Move this cancel() into the download() callback
+        // after ensuring that DownloadItems are created before that callback
+        // is fired.
+        downloads.cancel(downloadId, chrome.test.callback(function() {
+        }));
+        downloads.onCreated.removeListener(createdListener);
+        createdCompleted();
+      }
+      downloads.onCreated.addListener(createdListener);
+
+      var changedCompleted = chrome.test.callbackAdded();
+      function changedListener(delta) {
+        // Ignore onChanged events for downloads besides our own, or events that
+        // signal any change besides interruption.
+        if ((delta.id != downloadId) ||
+            !delta.state ||
+            (delta.state.new != downloads.STATE_INTERRUPTED))
+          return;

[asanka, 2012/02/13 19:45:47]

Nit: Check delta.error.new?

[benjhayden, 2012/02/13 21:05:31]

Done.

+        downloads.onChanged.removeListener(changedListener);
+        changedCompleted();
+      }
+      downloads.onChanged.addListener(changedListener);
+
+      downloads.download(
+          {'url': NEVER_FINISH_URL},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
+    function downloadOnChanged() {
+      // Test that download completion is detectable by an onChanged event
+      // listener.
+      const downloadId = getNextId();
+      var callbackCompleted = chrome.test.callbackAdded();
+      function myListener(delta) {
+        if ((delta.id != downloadId) ||
+            !delta.state ||
+            (delta.state.new != downloads.STATE_COMPLETE))
+          return;
+        downloads.onChanged.removeListener(myListener);
+        callbackCompleted();
+      }
+      downloads.onChanged.addListener(myListener);
+      downloads.download(
+        {"url": SAFE_FAST_URL},
+        chrome.test.callback(function(id) {
+          chrome.test.assertEq(downloadId, id);
+      }));
+    },
+
     function downloadFilename() {
+      // Test that we can suggest a filename for a new download, and test that
+      // we can detect filename changes with an onChanged event listener.
+      const FILENAME = 'owiejtoiwjrfoiwjroiwjroiwjroiwjrfi';
+      const downloadId = getNextId();
+      var callbackCompleted = chrome.test.callbackAdded();
+      function myListener(delta) {
+        if ((delta.id != downloadId) ||
+            !delta.filename ||
+            (delta.filename.new.indexOf(FILENAME) == -1))

[asanka, 2012/02/13 19:45:47]

Is .filename intended to be the target (final) path? or can it be the
intermediate path while the download is in progress? You might be succeeding
here when we see the path to FILENAME.crdownload. Perhaps that is okay.

The extension API doc is (at least to me) ambiguous on the precise meaning of
.filename. Also .filename is actually a path.

[benjhayden, 2012/02/13 21:05:31]

Yeah, there might be some more discussion to be had about |filename| before
launching, but not for this CL, especially when we don't know if search() or
onChanged will ever be launched...

+          return;
+        downloads.onChanged.removeListener(myListener);
+        callbackCompleted();
+      }
+      downloads.onChanged.addListener(myListener);
       downloads.download(
-          {'url': SAFE_FAST_URL, 'filename': 'foo'},
-          chrome.test.callbackPass(function(id) {
-            chrome.test.assertEq(getNextId(), id);
+          {'url': SAFE_FAST_URL, 'filename': FILENAME},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
           }));
-      // TODO(benjhayden): Test the filename using onChanged.
     },
+
     function downloadOnCreated() {
-      chrome.test.listenOnce(downloads.onCreated,
-                             chrome.test.callbackPass(function(item) {}));
+      // Test that the onCreated event fires when we start a download.
+      const downloadId = getNextId();
+      var createdCompleted = chrome.test.callbackAdded();
+      function createdListener(item) {
+        if (item.id == downloadId) {
+          createdCompleted();
+          downloads.onCreated.removeListener(createdListener);
+        }
+      };
+      downloads.onCreated.addListener(createdListener);
       downloads.download(
           {'url': SAFE_FAST_URL},
-          function(id) {
-            chrome.test.assertEq(getNextId(), id);
-          });
-    },
-    function downloadSubDirectoryFilename() {
-      downloads.download(
-          {'url': SAFE_FAST_URL, 'filename': 'foo/slow'},
-          chrome.test.callbackPass(function(id) {
-            chrome.test.assertEq(getNextId(), id);
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
           }));
-      // TODO(benjhayden): Test the filename using onChanged.
     },
+
     function downloadInvalidFilename() {
+      // Test that we disallow invalid filenames for new downloads.
       downloads.download(
           {'url': SAFE_FAST_URL, 'filename': '../../../../../etc/passwd'},
-          chrome.test.callbackFail(ERROR_GENERIC));
-      // TODO(benjhayden): Give a better error message.
+          chrome.test.callbackFail(downloads.ERROR_GENERIC));
     },
+
     function downloadEmpty() {
       assertThrows(('Invalid value for argument 1. Property \'url\': ' +
                     'Property is required.'),
                    downloads.download, {});
     },
+
     function downloadInvalidSaveAs() {
       assertThrows(('Invalid value for argument 1. Property \'saveAs\': ' +
                     'Expected \'boolean\' but got \'string\'.'),
                    downloads.download,
                    {'url': SAFE_FAST_URL, 'saveAs': 'GOAT'});
     },
+
     function downloadInvalidHeadersOption() {
       assertThrows(('Invalid value for argument 1. Property \'headers\': ' +
                     'Expected \'array\' but got \'string\'.'),
                    downloads.download,
                    {'url': SAFE_FAST_URL, 'headers': 'GOAT'});
     },
+
     function downloadInvalidURL() {
+      // Test that download() requires a valid url.
       downloads.download(
           {'url': 'foo bar'},
-          chrome.test.callbackFail(ERROR_INVALID_URL));
+          chrome.test.callbackFail(downloads.ERROR_INVALID_URL));
     },
+
     function downloadInvalidMethod() {
       assertThrows(('Invalid value for argument 1. Property \'method\': ' +
                     'Value must be one of: [GET, POST].'),
                    downloads.download,
                    {'url': SAFE_FAST_URL, 'method': 'GOAT'});
     },
-    function downloadSimple() {
-      downloads.download(
-          {'url': SAFE_FAST_URL},
-          chrome.test.callbackPass(function(id) {
-            chrome.test.assertEq(getNextId(), id);
-          }));
-    },
-    function downloadPost() {
-      downloads.download(
-          {'url': getURL('files/post/downloads/a_zip_file.js'),
-           'method': 'POST',
-           'body': 'WOOHOO'},
-          chrome.test.callbackPass(function(id) {
-              chrome.test.assertEq(getNextId(), id);
-          }));
-    },
-    function downloadHeader() {
-      downloads.download(
-          {'url': SAFE_FAST_URL,
-           'headers': [{'name': 'Foo', 'value': 'bar'}]
-          },
-          chrome.test.callbackPass(function(id) {
-            chrome.test.assertEq(getNextId(), id);
-          }));
-    },
-    function downloadInterrupted() {
-      // TODO(benjhayden): Find a suitable URL and test that this id is
-      // eventually interrupted using onChanged.
-      downloads.download(
-          {'url': SAFE_FAST_URL},
-          chrome.test.callbackPass(function(id) {
-            chrome.test.assertEq(getNextId(), id);
-          }));
-    },
+
     function downloadInvalidHeader() {
+      // Test that download() disallows setting the Cookie header.
       downloads.download(
           {'url': SAFE_FAST_URL,
            'headers': [{ 'name': 'Cookie', 'value': 'fake'}]
         },
-        chrome.test.callbackFail(ERROR_GENERIC));
-      // TODO(benjhayden): Give a better error message.
+        chrome.test.callbackFail(downloads.ERROR_GENERIC));
     },
+
     function downloadGetFileIconInvalidOptions() {
       assertThrows(('Invalid value for argument 2. Property \'cat\': ' +
                     'Unexpected property.'),
                    downloads.getFileIcon,
                    -1, {cat: 'mouse'});
     },
+
     function downloadGetFileIconInvalidSize() {
       assertThrows(('Invalid value for argument 2. Property \'size\': ' +
                     'Value must be one of: [16, 32].'),
                    downloads.getFileIcon, -1, {size: 31});
     },
+
     function downloadGetFileIconInvalidId() {
       downloads.getFileIcon(-42, {size: 32},
-        chrome.test.callbackFail(ERROR_INVALID_OPERATION));
-    },
-    function downloadNoComplete() {
-      // This is used partly to test cleanUp.
-      downloads.download(
-          {'url': NEVER_FINISH_URL},
-          chrome.test.callbackPass(function(id) {
-            chrome.test.assertEq(getNextId(), id);
-          }));
+        chrome.test.callbackFail(downloads.ERROR_INVALID_OPERATION));
     },
+
     function downloadPauseInvalidId() {
-      downloads.pause(-42, chrome.test.callbackFail(ERROR_INVALID_OPERATION));
+      downloads.pause(-42, chrome.test.callbackFail(
+            downloads.ERROR_INVALID_OPERATION));
     },
+
     function downloadPauseInvalidType() {
       assertThrows(('Invalid value for argument 1. Expected \'integer\' ' +
                     'but got \'string\'.'),
                    downloads.pause,
                    'foo');
     },
+
     function downloadResumeInvalidId() {
-      downloads.resume(-42, chrome.test.callbackFail(ERROR_INVALID_OPERATION));
+      downloads.resume(-42, chrome.test.callbackFail(
+            downloads.ERROR_INVALID_OPERATION));
     },
+
     function downloadResumeInvalidType() {
       assertThrows(('Invalid value for argument 1. Expected \'integer\' ' +
                     'but got \'string\'.'),
                    downloads.resume,
                    'foo');
     },
+
     function downloadCancelInvalidId() {
       // Canceling a non-existent download is not considered an error.
-      downloads.cancel(-42, chrome.test.callbackPass(function() {}));
+      downloads.cancel(-42, chrome.test.callback(function() {}));
     },
+
     function downloadCancelInvalidType() {
       assertThrows(('Invalid value for argument 1. Expected \'integer\' ' +
                     'but got \'string\'.'),
                    downloads.cancel, 'foo');
     },
+
+    function downloadNoComplete() {
+      // This is used partly to test cleanUp.
+      const downloadId = getNextId();
+      downloads.download(
+          {'url': NEVER_FINISH_URL},
+          chrome.test.callback(function(id) {
+            chrome.test.assertEq(downloadId, id);
+          }));
+    },
+
     function cleanUp() {
       // cleanUp must come last. It clears out all in-progress downloads
       // so the browser can shutdown cleanly.
       for (var id = 0; id < nextId; ++id) {
-        downloads.cancel(id, chrome.test.callbackPass(function() {}));
+        downloads.cancel(id, chrome.test.callback(function() {}));
       }
     }
   ]);