A few minor changes to the chrome.downloads extension API

chrome/common/extensions/api/downloads.idl

Index: chrome/common/extensions/api/downloads.idl
diff --git a/chrome/common/extensions/api/downloads.idl b/chrome/common/extensions/api/downloads.idl
index af6eac15c19f965681d6178197bef9d32e4b54a8..286f2df5b5867c48cee71db53e52482657a331a6 100644
--- a/chrome/common/extensions/api/downloads.idl
+++ b/chrome/common/extensions/api/downloads.idl
@@ -22,7 +22,7 @@ namespace downloads {
   //     <dt>prompt</dt>
   //     <dd>The user will be prompted with a file chooser dialog.</dd>
   // </dl>
-  [inline_doc] enum FilenameConflictAction {uniquify, overwrite, prompt};
+  enum FilenameConflictAction {uniquify, overwrite, prompt};
 
   [inline_doc] dictionary FilenameSuggestion {
     // The $ref:DownloadItem's new target $ref:DownloadItem.filename, as a path
@@ -32,7 +32,7 @@ namespace downloads {
     DOMString filename;
 
     // The action to take if <code>filename</code> already exists.
-    FilenameConflictAction? conflict_action;
+    FilenameConflictAction? conflictAction;
   };
 
   [inline_doc] enum HttpMethod {GET, POST};
@@ -42,13 +42,17 @@ namespace downloads {
     DOMString url;
 
     // A file path relative to the Downloads directory to contain the downloaded
-    // file. Cannot yet contain subdirectories; support for subdirectories will
-    // be implemented before this API is released to the stable channel. See
-    // $ref:onDeterminingFilename for how to dynamically suggest a filename
-    // after the file's MIME type and a tentative filename have been determined.
+    // file, possibly containing subdirectories. Absolute paths, empty paths,
+    // and paths containing back-references ".." will cause an error.
+    // $ref:onDeterminingFilename allows suggesting a filename after the file's
+    // MIME type and a tentative filename have been determined.
     DOMString? filename;
 
-    // Use a file-chooser to allow the user to select a filename.
+    // The action to take if <code>filename</code> already exists.
+    FilenameConflictAction? conflictAction;
+
+    // Use a file-chooser to allow the user to select a filename regardless of
+    // whether <code>filename</code> is set or already exists.
     boolean? saveAs;
 
     // The HTTP method to use if the URL uses the HTTP[S] protocol.
@@ -100,6 +104,9 @@ namespace downloads {
     // Absolute URL.
     DOMString url;
 
+    // Absolute URL.
+    DOMString referrer;

[asargent_no_longer_on_chrome, 2013/07/17 22:36:57]

I forget, do we tend to have the convention elsewhere in developer-facing API's
to spell this properly as you have here, or to propagate the misspelling of
'referer' from the on-the-wire format
(http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14)

[benjhayden, 2013/07/19 15:53:55]

chrome.privacy.websites.referrersEnabled
chrome.history.VisitItem.referringVisitId

[asargent_no_longer_on_chrome, 2013/07/22 19:15:53]

Ok, let's keep it as you have it here then. 

+
     // Absolute local path.
     DOMString filename;
 
@@ -111,9 +118,6 @@ namespace downloads {
     // suspicious.
     DangerType danger;
 
-    // True if the user has accepted the download's danger.
-    boolean? dangerAccepted;
-
     // The file's MIME type.
     DOMString mime;
 
@@ -129,6 +133,9 @@ namespace downloads {
     // console.log(new Date(item.endTime))})})</code>
     DOMString? endTime;
 
+    // Estimated time when the download will complete in ISO 8601 format.

[asargent_no_longer_on_chrome, 2013/07/17 22:36:57]

Is it worth mentioning in this comment that developers can just pass this string
into the constructor for a javascript Date object?

[benjhayden, 2013/07/19 15:53:55]

Done.

+    DOMString? estimatedEndTime;
+
     // Indicates whether the download is progressing, interrupted, or complete.
     State state;
 
@@ -136,8 +143,17 @@ namespace downloads {
     // connection open.
     boolean paused;
 
-    // Number indicating why a download was interrupted.
-    long? error;
+    // True if the download is in progress and paused, or else if it is
+    // interrupted and can be resumed starting from where it was interrupted.
+    boolean canResume;
+
+    // Why the download was interrupted. Several kinds of HTTP errors may be
+    // grouped under one of the errors beginning with <code>SERVER_</code>.
+    // Errors relating to the network begin with <code>NETWORK_</code>, errors
+    // relating to the process of writing the file to the file system begin with
+    // <code>FILE_</code>, and interruptions initiated by the user begin with
+    // <code>USER_</code>.
+    DOMString? error;

[asargent_no_longer_on_chrome, 2013/07/17 22:36:57]

It's sort of a bummer to take a presumably strongly typed value from the C++
code and lose that type information by stuffing it into a string here. Did you
do this just to avoid tying the C++ code too tightly to the JS API? Were you
just not sure about how we'd handle adding new values on the C++ side?

[benjhayden, 2013/07/19 15:53:55]

I did it because it was easiest, but you're right, an explicit enum is better.
Done.

 
     // Number of bytes received so far from the host, without considering file
     // compression.
@@ -163,12 +179,11 @@ namespace downloads {
   };
 
   [inline_doc] dictionary DownloadQuery {
-    // This space-separated string of search terms that may be grouped using
-    // quotation marks limits results to
-    // $ref:DownloadItem whose <code>filename</code>
-    // or <code>url</code> contain all of the search terms that do not begin with a dash '-'
-    // and none of the search terms that do begin with a dash.
-    DOMString? query;
+    // This array of search terms limits results to $ref:DownloadItem whose
+    // <code>filename</code> or <code>url</code> contain all of the search terms
+    // that do not begin with a dash '-' and none of the search terms that do
+    // begin with a dash.
+    DOMString[]? query;
 
     // Limits results to $ref:DownloadItem that
     // started before the given ms since the epoch.
@@ -202,18 +217,17 @@ namespace downloads {
     // <code>url</code> matches the given regular expression.
     DOMString? urlRegex;
 
-    // Setting this integer limits the number of results. Otherwise, all
-    // matching $ref:DownloadItem will be returned.
+    // The maximum number of matching $ref:DownloadItem returned. Defaults to
+    // 1000. Set to 0 in order to return all matching $ref:DownloadItem. See
+    // $ref:search for how to page through results.
     long? limit;
 
-    // Setting this string to a $ref:DownloadItem
-    // property sorts the $ref:DownloadItem prior
-    // to applying the above filters. For example, setting
-    // <code>orderBy='startTime'</code> sorts the
-    // $ref:DownloadItem by their start time in
-    // ascending order. To specify descending order, prefix <code>orderBy</code>
-    // with a hyphen: '-startTime'.
-    DOMString? orderBy;
+    // Set elements of this array to $ref:DownloadItem properties in order to
+    // sort search results. For example, setting
+    // <code>orderBy=['startTime']</code> sorts the $ref:DownloadItem by their
+    // start time in ascending order. To specify descending order, prefix with a
+    // hyphen: '-startTime'.
+    DOMString[]? orderBy;
 
     // The <code>id</code> of the $ref:DownloadItem to query.
     long? id;
@@ -228,9 +242,6 @@ namespace downloads {
     // suspicious.
     DangerType? danger;
 
-    // True if the user has accepted the download's danger.
-    boolean? dangerAccepted;
-
     // The file's MIME type.
     DOMString? mime;
 
@@ -247,8 +258,8 @@ namespace downloads {
     // connection open.
     boolean? paused;
 
-    // Number indicating why a download was interrupted.
-    long? error;
+    // Why a download was interrupted.
+    DOMString? error;
 
     // Number of bytes received so far from the host, without considering file
     // compression.
@@ -295,9 +306,6 @@ namespace downloads {
     // The change in <code>danger</code>, if any.
     StringDelta? danger;
 
-    // The change in <code>dangerAccepted</code>, if any.
-    BooleanDelta? dangerAccepted;
-
     // The change in <code>mime</code>, if any.
     StringDelta? mime;
 
@@ -310,11 +318,14 @@ namespace downloads {
     // The change in <code>state</code>, if any.
     StringDelta? state;
 
+    // The change in <code>canResume</code>, if any.
+    BooleanDelta? canResume;
+
     // The change in <code>paused</code>, if any.
     BooleanDelta? paused;
 
     // The change in <code>error</code>, if any.
-    LongDelta? error;
+    StringDelta? error;
 
     // The change in <code>totalBytes</code>, if any.
     LongDelta? totalBytes;
@@ -361,7 +372,10 @@ namespace downloads {
     // <code>query</code> to the empty object to get all
     // $ref:DownloadItem. To get a specific
     // $ref:DownloadItem, set only the <code>id</code>
-    // field.
+    // field. To page through a large number of items, set
+    // <code>orderBy: ['-startTime']</code>, set <code>limit</code> to
+    // the number of items per page, and set <code>startedAfter</code> to the
+    // <code>startTime</code> of the last item from the last page.
     static void search(DownloadQuery query, SearchCallback callback);
 
     // Pause the download. If the request was successful the download is in a
@@ -406,21 +420,28 @@ namespace downloads {
                             GetFileIconCallback callback);
 
     // Open the downloaded file now if the $ref:DownloadItem is complete;
-    // returns an error through $ref:runtime.lastError otherwise. An
+    // otherwise return an error through $ref:runtime.lastError. An
     // $ref:onChanged event will fire when the item is opened for the first
     // time.
     // |downloadId|: The identifier for the downloaded file.
     static void open(long downloadId);
 
-    // Show the downloaded file in its folder in a file manager.
+    // Show the downloaded file in its folder in a file manager. If
+    // <code>downloadId</code> is not specified, opens the default Downloads
+    // folder in a file manager.
     // |downloadId|: The identifier for the downloaded file.
-    static void show(long downloadId);
+    static void show(optional long downloadId);

[asargent_no_longer_on_chrome, 2013/07/17 22:36:57]

Another option instead of making this argument optional would be to just have
two methods:

show(long downloadId)
showFolder()

Any reason not to do it this way?

[benjhayden, 2013/07/19 15:53:55]

It is a little strange that this one function does two slightly different
things, so I'm happy to split it into a different function, but 'showFolder'
doesn't feel quite right because show() also shows a folder. showDefaultFolder()
seems long. Any other suggestions?

[asargent_no_longer_on_chrome, 2013/07/22 19:15:53]

That doesn't seem all that long to me. But here are some other ideas for you:

showItem / showFolder
showDownloadItem / showDownloadFolder

 
-    // Erase matching $ref:DownloadItem from history. An $ref:onErased event
-    // will fire for each $ref:DownloadItem that matches <code>query</code>,
-    // then <code>callback</code> will be called.
+    // Erase matching $ref:DownloadItem from history without deleting the
+    // downloaded file. An $ref:onErased event will fire for each
+    // $ref:DownloadItem that matches <code>query</code>, then
+    // <code>callback</code> will be called.
     static void erase(DownloadQuery query, optional EraseCallback callback);
 
+    // Delete the downloaded file if it exists and the $ref:DownloadItem is
+    // complete; otherwise return an error through $ref:runtime.lastError.
+    static void deleteFile(long downloadId, optional NullCallback callback);
+
     // Prompt the user to accept a dangerous download. Does not automatically
     // accept dangerous downloads. If the download is accepted, then an
     // $ref:onChanged event will fire, otherwise nothing will happen.  When all
@@ -429,10 +450,16 @@ namespace downloads {
     // renamed to the target filename, the |state| changes to 'complete', and
     // $ref:onChanged fires.
     // |downloadId|: The identifier for the $ref:DownloadItem.
-    static void acceptDanger(long downloadId);
+    // |callback|: Called when the danger prompt dialog closes.
+    static void acceptDanger(long downloadId, optional NullCallback callback);
 
-    // Initiate dragging the downloaded file to another application.
+    // Initiate dragging the downloaded file to another application. Call in a
+    // javascript <code>ondragstart</code> handler.
     static void drag(long downloadId);
+
+    // Set <code>visible</code> to true to show the download shelf, false to
+    // hide it.
+    static void setShelfVisible(boolean visible);
   };
 
   interface Events {
@@ -447,9 +474,9 @@ namespace downloads {
     static void onErased(long downloadId);
 
     // When any of a $ref:DownloadItem's properties except
-    // <code>bytesReceived</code> changes, this event fires with the
-    // <code>downloadId</code> and an object containing the properties that
-    // changed.
+    // <code>bytesReceived</code> and <code>estimatedEndTime</code> changes,
+    // this event fires with the <code>downloadId</code> and an object
+    // containing the properties that changed.
     static void onChanged(DownloadDelta downloadDelta);
 
     // During the filename determination process, extensions will be given the