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

content/browser/download/drag_download_file.cc

Index: content/browser/download/drag_download_file.cc
diff --git a/content/browser/download/drag_download_file.cc b/content/browser/download/drag_download_file.cc
index 38f0df006494221d4ad5a7df9bfbdb6eecf11a12..e119ea3542a658aebde34e946a3a2ac92c100c1c 100644
--- a/content/browser/download/drag_download_file.cc
+++ b/content/browser/download/drag_download_file.cc
@@ -196,8 +196,7 @@ void DragDownloadFile::ModelChanged(DownloadManager* manager) {
 
 void DragDownloadFile::OnDownloadUpdated(content::DownloadItem* download) {
   AssertCurrentlyOnUIThread();
-  if (download->IsCancelled() ||
-      (download->GetState() == DownloadItem::REMOVING)) {
+  if (download->IsCancelled()) {
     RemoveObservers();
     DownloadCompleted(false);
   } else if (download->IsComplete()) {
@@ -207,6 +206,16 @@ void DragDownloadFile::OnDownloadUpdated(content::DownloadItem* download) {
   // Ignore other states.
 }
 
+// If the download completes or is cancelled, then OnDownloadUpdated() will
+// handle it and RemoveObserver() so that OnDownloadDestroyed is never called.
+// OnDownloadDestroyed is only called if OnDownloadUpdated() does not detect
+// completion or cancellation (in which cases it removes this observer).
+void DragDownloadFile::OnDownloadDestroyed(content::DownloadItem* download) {

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

NOTREACHED?

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

TODone. Again, I'd rather not change functionality like that in this CL.

+  AssertCurrentlyOnUIThread();
+  RemoveObservers();
+  DownloadCompleted(false);
+}
+
 void DragDownloadFile::AssertCurrentlyOnDragThread() {
   // Only do the check on Windows where two threads are involved.
 #if defined(OS_WIN)