Bug 394039 - Calling removeDownload() works but doesn't update download window. r=sdwilsh, b-ff3=mconnor

toolkit/components/downloads/public/nsIDownloadManager.idl

@@ -132,6 +132,10 @@ interface nsIDownloadManager : nsISupports {
    * in-progress.  Whereas cancelDownload simply cancels the transfer, but
    * retains information about it, removeDownload removes all knowledge of it.
    *
+   * Also notifies observers of the "download-manager-remove-download" topic
+   * with the download id as the subject to allow any DM consumers to react to
+   * the removal.
+   *
    * @param aID The unique ID of the download.
    * @throws NS_ERROR_FAILURE if the download is active.
    */
@@ -178,8 +182,8 @@ interface nsIDownloadManager : nsISupports {
   /** 
    * Removes completed, failed, and canceled downloads from the list.
    *
-   * Also notifies observers of the "download-manager-clear-history"
-   * topic, to allow any DM consumers to react to the removals.
+   * Also notifies observers of the "download-manager-remove-download" topic
+   * with a null subject to allow any DM consumers to react to the removals.
    */
   void cleanUp();
 

toolkit/components/downloads/src/nsDownloadManager.cpp

@@ -1232,9 +1232,23 @@ nsDownloadManager::RemoveDownload(PRUint32 aID)
     "DELETE FROM moz_downloads "
     "WHERE id = ?1"), getter_AddRefs(stmt));
   NS_ENSURE_SUCCESS(rv, rv);
+
   rv = stmt->BindInt64Parameter(0, aID); // unsigned; 64-bit to prevent overflow
   NS_ENSURE_SUCCESS(rv, rv);
-  return stmt->Execute();
+
+  rv = stmt->Execute();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsISupportsPRUint32> id =
+    do_CreateInstance(NS_SUPPORTS_PRUINT32_CONTRACTID, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = id->SetData(aID);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Notify the UI with the topic and download id
+  return mObserverService->NotifyObservers(id,
+                                           "download-manager-remove-download",
+                                           nsnull);
 }
 
 NS_IMETHODIMP
@@ -1261,9 +1275,9 @@ nsDownloadManager::CleanUp()
   rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Notify the UI
+  // Notify the UI with the topic and null subject to indicate "remove all"
   return mObserverService->NotifyObservers(nsnull,
-                                           "download-manager-clear-history",
+                                           "download-manager-remove-download",
                                            nsnull);
 }
 

toolkit/mozapps/downloads/content/downloads.js

@@ -47,7 +47,6 @@
 
 const PREF_BDM_CLOSEWHENDONE = "browser.download.manager.closeWhenDone";
 const PREF_BDM_ALERTONEXEOPEN = "browser.download.manager.alertOnEXEOpen";
-const PREF_BDM_RETENTION = "browser.download.manager.retention";
 const PREF_BDM_DISPLAYEDHISTORYDAYS =
   "browser.download.manager.displayedHistoryDays";
 
@@ -166,7 +165,7 @@ function downloadCompleted(aDownload)
     if (!gSearching)
       gDownloadsView.insertBefore(dl, gDownloadsOtherTitle.nextSibling);
     else
-      gDownloadsView.removeChild(dl);
+      removeFromView(dl);
 
     // getTypeFromFile fails if it can't find a type for this file.
     try {
@@ -196,13 +195,6 @@ function autoRemoveAndClose(aDownload)
   var pref = Cc["@mozilla.org/preferences-service;1"].
              getService(Ci.nsIPrefBranch);
 
-  if (aDownload && (pref.getIntPref(PREF_BDM_RETENTION) == 0)) {
-    // The download manager backend removes this, but we have to update the UI!
-    var dl = getDownload(aDownload.id);
-    if (dl)
-      dl.parentNode.removeChild(dl);
-  }
-  
   if (gDownloadManager.activeDownloadCount == 0) {
     // For the moment, just use the simple heuristic that if this window was
     // opened by the download process, rather than by the user, it should
@@ -259,7 +251,6 @@ function resumeDownload(aDownload)
 
 function removeDownload(aDownload)
 {
-  removeFromView(aDownload);
   gDownloadManager.removeDownload(aDownload.getAttribute("dlid"));
 }
 
@@ -488,7 +479,7 @@ function Startup()
 
   var obs = Components.classes["@mozilla.org/observer-service;1"].
             getService(Components.interfaces.nsIObserverService);
-  obs.addObserver(clearDownloadsObserver, "download-manager-clear-history", false);
+  obs.addObserver(gDownloadObserver, "download-manager-remove-download", false);
 }
 
 function Shutdown() 
@@ -497,13 +488,26 @@ function Shutdown()
 
   var obs = Components.classes["@mozilla.org/observer-service;1"].
             getService(Components.interfaces.nsIObserverService);
-  obs.removeObserver(clearDownloadsObserver, "download-manager-clear-history");
+  obs.removeObserver(gDownloadObserver, "download-manager-remove-download");
 }
 
-var clearDownloadsObserver = {
-  observe: function cdo_observe() {
-    // Rebuild the default view
-    buildDefaultView();
+let gDownloadObserver = {
+  observe: function gdo_observe(aSubject, aTopic, aData) {
+    switch (aTopic) {
+      case "download-manager-remove-download":
+        // A null subject here indicates "remove all"
+        if (!aSubject) {
+          // Rebuild the default view
+          buildDefaultView();
+          break;
+        }
+
+        // Otherwise, remove a single download
+        let id = aSubject.QueryInterface(Ci.nsISupportsPRUint32);
+        let dl = getDownload(id.data);
+        removeFromView(dl);
+        break;
+    }
   }
 };
 
@@ -843,6 +847,9 @@ function replaceInsert(aText, aIndex, aValue)
 
 function removeFromView(aDownload)
 {
+  // Make sure we have an item to remove
+  if (!aDownload) return;
+
   let index = gDownloadsView.selectedIndex;
   gDownloadsView.removeChild(aDownload);
   gDownloadsView.selectedIndex = Math.min(index, gDownloadsView.itemCount - 1);