Implement chrome.downloads.erase()

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;
 
@@ skipping 347 matching lines @@
                             optional GetFileIconOptions options,
                             GetFileIconCallback callback);
 
     // Open the downloaded file now if the $ref:DownloadItem is complete;
     // returns an error through $ref:runtime.lastError otherwise. 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);
 
-    // Erase matching $ref:DownloadItem from history
-    [nodoc] static void erase(DownloadQuery query,
-                              optional EraseCallback callback);
+    // 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);

[Randy Smith (Not in Mondays), 2012/12/10 19:04:36]

I don't understand what runs the callback?  Do you have a test testing the
callback behavior?

[benjhayden, 2012/12/10 19:10:48]

This is in the same boat as search(), download(), getFileIcon(), cancel(),
resume(), pause().
There's no way to test that the javascript callback is called properly. That's
something that we have to trust the extension framework to handle correctly.
downloads_api_unittest.cc tests the callback indirectly by testing the
ExtensionFunction's "response" and "error", which the extension framework
promises to pass on to the javascript run-time.

[Randy Smith (Not in Mondays), 2012/12/10 19:17:27]

Sorry, I'm sure that this is because I don't look at the code for months at a
time, but where is the callback fired from?  

The thing I'm confused about is that the code that iterates through the items to
be erased / removed isn't integrated with the extensions framework, but the
callback is specific to this API function invocation.  The only implementation
that puts those two things together I can imagine is that the callback is hooked
up to OnDownloadRemoved(), but only for the duration of the API function call,
and that means that if a remove occurs from some other source (UI), the callback
will trigger for that remove as well, which violates the API description.  

[benjhayden, 2012/12/10 19:26:29]

The code that iterates through the items to be removed is
ExtensionEraseFunction, which is-a SyncExtensionFunction, which calls
ExtensionFunction::SendResponse after calling ExtensionEraseFunction::RunImpl.
SendResponse sends an IPC to the renderer containing ExtensionEraseFunction's
response, error_, and an identifier that was assigned by the javascript
extension framework to match up chrome.* API calls with callbacks.
There is no Observer -- ExtensionEraseFunction::RunImpl trusts that calling
Remove() on a DownloadItem will remove it.

[Randy Smith (Not in Mondays), 2012/12/11 15:28:01]

So what's the source dataflow for the ids that are passed to the callback?  From
looking at the code those ids are generated in ExtensionsEraseFunction::RunImpl,
and aren't stored in the ExtensionsEraseFunction object.  

(Feel free to IM me if this exchange could be improved by quicker interaction.)

[benjhayden, 2012/12/11 18:47:45]

Oh! Sorry, I completely missed that the callback takes an array of the ids.
PTAL at ExtensionEraseFunction and downloads_api_unittest.cc.

 
     // TODO(benjhayden) Comment.
     [nodoc] static void setDestination(long downloadId, DOMString relativePath);
 
     // Prompt the user to either accept or cancel a dangerous download.
     // <code>acceptDanger()</code> does not automatically accept dangerous
     // downloads.
     [nodoc] static void acceptDanger(long downloadId);
 
     // Show the downloaded file in its folder in a file manager.
@@ skipping 12 matching lines @@
     // 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);
   };
 };