Implement chrome.downloads.onDeterminingFilename()

chrome/common/extensions/api/downloads.idl

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 [permissions=downloads]
 namespace downloads {
   [inline_doc] dictionary HeaderNameValuePair {
     // Name of the HTTP header.
     DOMString name;
 
     // Value of the HTTP header.
     DOMString value;
   };
 
+  // $ref:onDeterminingFilename handlers may return a FilenameDetermination
+  // object in order to override a download's target filename.

[mkearney1, 2013/02/05 22:00:02]

Should this link to filename? I can't see the staged docs at the moment to check
the flow, so maybe it isn't necessary.

[benjhayden, 2013/02/21 22:29:33]

Done.

+  dictionary FilenameDetermination {
+    // The download's new target filename, as a path relative to the user's
+    // default Downloads directory, possibly containing subdirectories. Absolute

[Matt Perry, 2013/02/04 22:30:19]

How do we handle path separators in a cross-platform manner? On Windows, will
"../foo.txt" show up as a back reference given that \ is the path separator?

[benjhayden, 2013/02/05 15:08:05]

FilePath allows either / or \ as a path separator on windows.
https://code.google.com/p/chromium/codesearch#chrome/src/base/file_path.h&q=f...

+    // paths, empty paths, and paths containing back-references ".." will be
+    // ignored.
+    DOMString filename;
+
+    // Whether to overwrite any existing files.
+    boolean? overwrite;
+  };
+
   [inline_doc] enum HttpMethod {GET, POST};
 
   [inline_doc] dictionary DownloadOptions {
     // The URL to download.
     DOMString url;
 
     // A file path relative to the Downloads directory to contain the downloaded
     // file.
     DOMString? filename;
 
@@ skipping 364 matching lines @@
 
     // Show the downloaded file in its folder in a file manager.
     // |downloadId|: The identifier for the downloaded file.
     static void show(long downloadId);
 
     // 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.
     static void erase(DownloadQuery query, optional EraseCallback callback);
 
-    // TODO(benjhayden) Comment.
-    [nodoc] static void setDestination(long downloadId, DOMString relativePath);
-
     // 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
     // the data is fetched into a temporary file and either the download is not
     // dangerous or the danger has been accepted, then the temporary file is
     // 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);
 
     // Initiate dragging the downloaded file to another application.
     static void drag(long downloadId);
   };
 
   interface Events {
     // This event fires with the $ref:DownloadItem
     // object when a download begins.
     static void onCreated(DownloadItem downloadItem);
 
     // Fires with the <code>downloadId</code> when a download is erased from
     // history.
     // |downloadId|: The <code>id</code> of the $ref:DownloadItem that was erased.
     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.
     static void onChanged(DownloadDelta downloadDelta);
+
+    // During the filename determination process, extensions will be given the
+    // opportunity to override the target filename. Handlers may return null in

[mkearney1, 2013/02/05 22:00:02]

Should target filename be a link?

[benjhayden, 2013/02/21 22:29:33]

Done.

+    // order to allow the target filename to be used, or return a
+    // $ref:FilenameDetermination object in order to override the target
+    // filename. If more than one extension overrides the filename, then the
+    // last handler registered in the last extension installed that returns a
+    // $ref:FilenameDetermination object wins. Users should not install
+    // extensions that may conflict.
+    static void onDeterminingFilename(DownloadItem downloadItem);

[Matt Perry, 2013/02/04 22:30:19]

Does this really need to be a synchronous API? What if we did this instead:
- Downloads system starts writing to the default filename immediately.
- Extensions have a certain amount of time to rename the file to something else.

[benjhayden, 2013/02/05 15:08:05]

The download system does already start writing to a temporary "Unconfirmed
XXXXX.crdownload" filename immediately.
I think that Asanka explained to me that it would be untenable in the current
download system architecture to determine the filename multiple times.
How much time? What if the system is being slow? What about when I change the
API to allow extensions to pop up a dialog to allow the user to choose whether
to overwrite?

[Randy Smith (Not in Mondays), 2013/02/05 16:40:41]

I'd like to second this point--this type of race strikes me as a really bad
idea.  The extension writer will not be able to depend on the API to react
reliably; whether the rename happens will depend on system loader, user
distraction (in the case Ben raises), and the phase of the moon.  Let's please
not do timing window dependent extension APIs.

[Matt Perry, 2013/02/05 20:49:39]

OK, sounds like this won't work.
Is the prompt blocking from the JavaScript context's point of view? The
onDeterminingFilename event requires a synchronous response - any asynchronous
query won't work.

[benjhayden, 2013/02/05 21:12:19]

I am planning on augmenting the onDeterminingFilename event in a followup CL to
allow extensions to request downloads_custom_bindings.js to pass the handler a
callback. This future change will only affect downloads_custom_bindings.js.
Handlers may then look like this:
chrome.downloads.onDeterminingFilename.addListener(function(item, callback) {
  chrome.tabs.sendMessage(some_tab_id, item, function(response) {
    callback(response);
  });
}, true/*handler is asynchronous*/);

[Matt Perry, 2013/02/05 21:57:10]

I see. In that case, I'd prefer we always use the callback for consistency. See
https://developer.chrome.com/trunk/extensions/runtime.html#event-onMessage for
an API that functions similarly (you return true from the event handler to
indicate it's asynchronous).

[benjhayden, 2013/02/21 22:29:33]

Done.

   };
 };