Bug 382839 - getDownload should return any known download, not just those in progress. r=gavin.sharp

toolkit/components/downloads/public/nsIDownloadManager.idl

@@ -106,11 +106,13 @@ interface nsIDownloadManager : nsISupports {
                           in nsICancelable aCancelable);
 
   /**
-   * Retrieves an in-progress download managed by the download manager.
+   * Retrieves a download managed by the download manager.  This can be one that
+   * is in progress, or one that has completed in the past and is stored in the
+   * database.
    *
    * @param aID The unique ID of the download.
    * @return The download with the specified ID.
-   * @throws NS_ERROR_FAILURE if the download is not in-progress.
+   * @throws NS_ERROR_NOT_AVAILABLE if the download is not in the database.
    */
   nsIDownload getDownload(in unsigned long aID);
 

toolkit/components/downloads/src/nsDownloadManager.cpp

@@ -611,10 +611,12 @@ nsDownloadManager::GetDownload(PRUint32 aID, nsIDownload **aDownloadItem)
 {
   nsDownload *itm = FindDownload(aID);
 
+  nsRefPtr<nsDownload> dl;
   if (!itm) {
-    *aDownloadItem = nsnull;
+    nsresult rv = GetDownloadFromDB(aID, getter_AddRefs(dl));
+    NS_ENSURE_SUCCESS(rv, rv);
 
-    return NS_ERROR_FAILURE;
+    itm = dl.get();
   }
   
   NS_ADDREF(*aDownloadItem = itm);

toolkit/components/downloads/test/unit/test_download_manager.js

@@ -42,12 +42,11 @@ const dm = Cc["@mozilla.org/download-manager;1"].getService(nsIDownloadManager);
 
 function test_get_download_empty_queue()
 {
-  print("*** DOWNLOAD MANAGER TEST - test_get_download_empty_queue");
   try {
     dm.getDownload(0);
     do_throw("Hey!  We expect to get an excpetion with this!");
   } catch(e) {
-    do_check_eq(Components.lastResult, Cr.NS_ERROR_FAILURE);
+    do_check_eq(Components.lastResult, Cr.NS_ERROR_NOT_AVAILABLE);
   }
 }
 

toolkit/components/downloads/test/unit/test_download_manager_migration.js

@@ -36,8 +36,8 @@
  * ***** END LICENSE BLOCK ***** */
 
 // This tests the migration code to make sure we properly migrate downloads.rdf
-// Also tests cleanUp function of DM since we have a good number of entries to
-// clean up after importing.
+// Also tests cleanUp and getDownload (from the database) since we have a good
+// number of entries in the database after importing.
 
 importDownloadsFile("downloads.rdf");
 
@@ -73,6 +73,15 @@ function test_random_download()
   stmt.reset();
 }
 
+// yey - we have entries in the DM to test this!
+function test_dm_getDownload()
+{
+  // this will get it from the database
+  var dl = dm.getDownload(1);
+  
+  do_check_eq("CVS-Contributor-Form.pdf", dl.displayName);
+}
+
 // This provides us with a lot of download entries to test the cleanup function
 function test_dm_cleanup()
 {
@@ -87,7 +96,8 @@ function test_dm_cleanup()
   stmt.reset();
 }
 
-var tests = [test_count_entries, test_random_download, test_dm_cleanup];
+var tests = [test_count_entries, test_random_download, test_dm_getDownload,
+             test_dm_cleanup];
 
 function run_test()
 {