Reland DownloadItem::Observer::OnDownloadDestroyed() replaces DownloadItem::REMOVING

chrome/browser/download/download_path_reservation_tracker.cc

Index: chrome/browser/download/download_path_reservation_tracker.cc
diff --git a/chrome/browser/download/download_path_reservation_tracker.cc b/chrome/browser/download/download_path_reservation_tracker.cc
index cec817364dc6f1bf2fd45d1b316a367df34c9d7e..2f0e3d86cfa8cffeaeca0469f69cd4e7210472e6 100644
--- a/chrome/browser/download/download_path_reservation_tracker.cc
+++ b/chrome/browser/download/download_path_reservation_tracker.cc
@@ -36,7 +36,7 @@ class DownloadItemObserver : public DownloadItem::Observer {
 
   // DownloadItem::Observer
   virtual void OnDownloadUpdated(DownloadItem* download) OVERRIDE;
-  virtual void OnDownloadOpened(DownloadItem* download) OVERRIDE;
+  virtual void OnDownloadDestroyed(DownloadItem* download) OVERRIDE;
 
   DownloadItem& download_item_;
 
@@ -89,10 +89,6 @@ void DownloadItemObserver::OnDownloadUpdated(DownloadItem* download) {
     case DownloadItem::CANCELLED:
       // We no longer need the reservation if the download is being removed.
 
-    case DownloadItem::REMOVING:
-      // Ditto, but this case shouldn't happen in practice. We should have
-      // received another notification beforehand.
-
     case DownloadItem::INTERRUPTED:
       // The download filename will need to be re-generated when the download is
       // restarted. Holding on to the reservation now would prevent the name
@@ -108,10 +104,11 @@ void DownloadItemObserver::OnDownloadUpdated(DownloadItem* download) {
   }
 }
 
-void DownloadItemObserver::OnDownloadOpened(DownloadItem* download) {
-  // We shouldn't be tracking reservations for a download that has been
-  // externally opened. The tracker should have detached itself when the
-  // download was complete.
+void DownloadItemObserver::OnDownloadDestroyed(DownloadItem* download) {
+  // This shouldn't happen. We should catch either COMPLETE, CANCELLED, or
+  // INTERRUPTED first.

[Randy Smith (Not in Mondays), 2012/07/11 17:55:37]

NOTREACHED()?

[benjhayden, 2012/07/13 20:03:17]

Maybe in another CL. I'd rather this CL not change functionality like that.

+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, revoke_callback_);
+  delete this;
 }
 
 }  // namespace