Revert 149794 - DownloadItem::Observer::OnDownloadDestroyed() replaces DownloadItem::REMOVING

content/browser/download/download_manager_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/download/download_manager_impl.h"
 
 #include <iterator>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 303 matching lines @@
       download->Cancel(false);
       if (delegate_)
         delegate_->UpdateItemInPersistentStore(download);
     }
   }
 
   // At this point, all dangerous downloads have had their files removed
   // and all in progress downloads have been cancelled.  We can now delete
   // anything left.
 
+  // Copy downloads_ to separate container so as not to set off checks
+  // in DownloadItem destruction.
+  DownloadMap downloads_to_delete;
+  downloads_to_delete.swap(downloads_);
+
   active_downloads_.clear();
-  STLDeleteValues(&downloads_);
-  downloads_.clear();
+  STLDeleteValues(&downloads_to_delete);
 
   // We'll have nothing more to report to the observers after this point.
   observers_.Clear();
 
   file_manager_ = NULL;
   if (delegate_)
     delegate_->Shutdown();
   delegate_ = NULL;
 }
 
@@ skipping 269 matching lines @@
   if (active_downloads_.count(download_id) == 0)
     return;
 
   DownloadItemImpl* download = active_downloads_[download_id];
   download->OnAllDataSaved(size, hash);
   MaybeCompleteDownload(download);
 }
 
 void DownloadManagerImpl::AssertStateConsistent(
     DownloadItemImpl* download) const {
+  if (download->GetState() == DownloadItem::REMOVING) {
+    DCHECK(!ContainsKey(downloads_, download->GetId()));
+    DCHECK(!ContainsKey(active_downloads_, download->GetId()));
+    return;
+  }
+
+  // Should be in downloads_ if we're not REMOVING.
   CHECK(ContainsKey(downloads_, download->GetId()));
 
   int64 state = download->GetState();
   base::debug::Alias(&state);
   if (ContainsKey(active_downloads_, download->GetId())) {
     if (download->IsPersisted())
       CHECK_EQ(DownloadItem::IN_PROGRESS, download->GetState());
     if (DownloadItem::IN_PROGRESS != download->GetState())
       CHECK_EQ(DownloadItem::kUninitializedHandle, download->GetDbHandle());
   }
@@ skipping 138 matching lines @@
   return delegate_ && delegate_->GenerateFileHash();
 }
 
 int DownloadManagerImpl::RemoveDownloadItems(
     const DownloadItemImplVector& pending_deletes) {
   if (pending_deletes.empty())
     return 0;
 
   // Delete from internal maps.
   for (DownloadItemImplVector::const_iterator it = pending_deletes.begin();
-       it != pending_deletes.end();
-       ++it) {
+      it != pending_deletes.end();
+      ++it) {
     DownloadItemImpl* download = *it;
     DCHECK(download);
-    int32 download_id = download->GetId();
-    delete download;
-    downloads_.erase(download_id);
+    downloads_.erase(download->GetId());
   }
+
+  // Tell observers to refresh their views.
   NotifyModelChanged();
-  return static_cast<int>(pending_deletes.size());
+
+  // Delete the download items themselves.
+  const int num_deleted = static_cast<int>(pending_deletes.size());
+  STLDeleteContainerPointers(pending_deletes.begin(), pending_deletes.end());
+  return num_deleted;
 }
 
 void DownloadManagerImpl::DownloadRemoved(DownloadItemImpl* download) {
   if (!download ||
       downloads_.find(download->GetId()) == downloads_.end())
     return;
 
   // TODO(benjhayden,rdsmith): Remove this.
   if (!download->IsPersisted())
     return;
 
   // Make history update.
   if (delegate_)
     delegate_->RemoveItemFromPersistentStore(download);
 
   // Remove from our tables and delete.
   int downloads_count =
       RemoveDownloadItems(DownloadItemImplVector(1, download));
   DCHECK_EQ(1, downloads_count);
 }
 
 int DownloadManagerImpl::RemoveDownloadsBetween(base::Time remove_begin,
                                                 base::Time remove_end) {
   if (delegate_)
     delegate_->RemoveItemsFromPersistentStoreBetween(remove_begin, remove_end);
 
+  // All downloads visible to the user will be in the history,
+  // so scan that map.
   DownloadItemImplVector pending_deletes;
   for (DownloadMap::const_iterator it = downloads_.begin();
       it != downloads_.end();
       ++it) {
     DownloadItemImpl* download = it->second;
     if (download->IsPersisted() &&
         download->GetStartTime() >= remove_begin &&
         (remove_end.is_null() || download->GetStartTime() < remove_end) &&
         (download->IsComplete() || download->IsCancelled())) {
       AssertStateConsistent(download);
-      download->NotifyRemoved();
       pending_deletes.push_back(download);
     }
   }
   return RemoveDownloadItems(pending_deletes);
 }
 
 int DownloadManagerImpl::RemoveDownloads(base::Time remove_begin) {
   return RemoveDownloadsBetween(remove_begin, base::Time());
 }
 
@@ skipping 268 matching lines @@
 void DownloadManagerImpl::DownloadRenamedToFinalName(
     DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // If the rename failed, we receive an OnDownloadInterrupted() call before we
   // receive the DownloadRenamedToFinalName() call.
   if (delegate_) {
     delegate_->UpdatePathForItemInPersistentStore(
         download, download->GetFullPath());
   }
 }