From 2b8c31c104233cfd7ebd797a0b75436e8dc8c61c Mon Sep 17 00:00:00 2001
From: "sdwilsh@shawnwilsher.com" <sdwilsh@shawnwilsher.com>
Date: Tue, 12 Jun 2007 16:39:15 -0700
Subject: [PATCH] Bug 382839 - getDownload should return any known download,
 not just those in progress.  r=gavin.sharp

---
 .../downloads/public/nsIDownloadManager.idl      |  6 ++++--
 .../downloads/src/nsDownloadManager.cpp          |  6 ++++--
 .../downloads/test/unit/test_download_manager.js |  3 +--
 .../test/unit/test_download_manager_migration.js | 16 +++++++++++++---
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/toolkit/components/downloads/public/nsIDownloadManager.idl b/toolkit/components/downloads/public/nsIDownloadManager.idl
index d002547430e..0da7a1f6558 100644
--- a/toolkit/components/downloads/public/nsIDownloadManager.idl
+++ b/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);
 
diff --git a/toolkit/components/downloads/src/nsDownloadManager.cpp b/toolkit/components/downloads/src/nsDownloadManager.cpp
index 5a803464546..601c030174b 100644
--- a/toolkit/components/downloads/src/nsDownloadManager.cpp
+++ b/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);
diff --git a/toolkit/components/downloads/test/unit/test_download_manager.js b/toolkit/components/downloads/test/unit/test_download_manager.js
index b7a0e031cc4..d0fb811aaa9 100644
--- a/toolkit/components/downloads/test/unit/test_download_manager.js
+++ b/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);
   }
 }
 
diff --git a/toolkit/components/downloads/test/unit/test_download_manager_migration.js b/toolkit/components/downloads/test/unit/test_download_manager_migration.js
index f2999cac891..ebfee4cf0ce 100644
--- a/toolkit/components/downloads/test/unit/test_download_manager_migration.js
+++ b/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()
 {