Revert 102126 - Make cancel remove cancelled download from active queues at time of cancel.

content/browser/download/download_manager.cc

 // Copyright (c) 2011 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.h"
 
 #include <iterator>
 
 #include "base/callback.h"
 #include "base/file_util.h"
@@ skipping 94 matching lines @@
       // The user hasn't accepted it, so we need to remove it
       // from the disk.  This may or may not result in it being
       // removed from the DownloadManager queues and deleted
       // (specifically, DownloadManager::RemoveDownload only
       // removes and deletes it if it's known to the history service)
       // so the only thing we know after calling this function is that
       // the download was deleted if-and-only-if it was removed
       // from all queues.
       download->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
     } else if (download->IsPartialDownload()) {
-      download->Cancel();
+      download->Cancel(false);
+      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.
   DownloadSet downloads_to_delete;
@@ skipping 141 matching lines @@
            << " download = " << download->DebugString(true);
 
   FilePath suggested_path = download->suggested_path();
 
   if (download->prompt_user_for_save_location()) {
     // We must ask the user for the place to put the download.
     DownloadRequestHandle request_handle = download->request_handle();
     TabContents* contents = request_handle.GetTabContents();
 
     // |id_ptr| will be deleted in either FileSelected() or
-    // FileSelectionCanceled().
+    // FileSelectionCancelled().
     int32* id_ptr = new int32;
     *id_ptr = download_id;
 
     delegate_->ChooseDownloadPath(
         contents, suggested_path, reinterpret_cast<void*>(id_ptr));
 
     FOR_EACH_OBSERVER(Observer, observers_,
                       SelectFileDialogDisplayed(download_id));
   } else {
     // No prompting for download, just continue with the suggested name.
@@ skipping 209 matching lines @@
       NewRunnableMethod(
           file_manager_, &DownloadFileManager::CompleteDownload, download_id));
 
   if (uniquifier)
     item->set_path_uniquifier(uniquifier);
 
   item->OnDownloadRenamedToFinalName(full_path);
   delegate_->UpdatePathForItemInPersistentStore(item, full_path);
 }
 
-void DownloadManager::DownloadStopped(DownloadItem* download) {
+void DownloadManager::CancelDownload(int32 download_id) {
+  DownloadItem* download = GetActiveDownload(download_id);
+  // A cancel at the right time could remove the download from the
+  // |active_downloads_| map before we get here.
+  if (!download)
+    return;
+
+  download->Cancel(true);
+}
+
+void DownloadManager::DownloadCancelledInternal(DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  CHECK(ContainsKey(active_downloads_, download->id()));
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
-  in_progress_.erase(download->id());
-  active_downloads_.erase(download->id());
-  UpdateDownloadProgress();  // Reflect removal from in_progress_.
-  if (download->db_handle() != DownloadItem::kUninitializedHandle)
-    delegate_->UpdateItemInPersistentStore(download);
-
+  RemoveFromActiveList(download);
   // This function is called from the DownloadItem, so DI state
   // should already have been updated.
   AssertQueueStateConsistent(download);
 
   download->OffThreadCancel(file_manager_);
 }
 
 void DownloadManager::OnDownloadError(int32 download_id,
                                       int64 size,
                                       net::Error error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download = GetActiveDownload(download_id);
   if (!download)
     return;
 
   VLOG(20) << __FUNCTION__ << "()" << " Error " << error
            << " at offset " << download->received_bytes()
            << " size = " << size
            << " download = " << download->DebugString(true);
 
-  download->Interrupt(size, error);
+  RemoveFromActiveList(download);
+  download->Interrupted(size, error);
+  download->OffThreadCancel(file_manager_);
 }
 
 DownloadItem* DownloadManager::GetActiveDownload(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = active_downloads_.find(download_id);
   if (it == active_downloads_.end())
     return NULL;
 
   DownloadItem* download = it->second;
 
   DCHECK(download);
   DCHECK_EQ(download_id, download->id());
 
   return download;
 }
 
+void DownloadManager::RemoveFromActiveList(DownloadItem* download) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(download);
+
+  // Clean up will happen when the history system create callback runs if we
+  // don't have a valid db_handle yet.
+  if (download->db_handle() != DownloadItem::kUninitializedHandle) {
+    in_progress_.erase(download->id());
+    active_downloads_.erase(download->id());
+    UpdateDownloadProgress();  // Reflect removal from in_progress_.
+    delegate_->UpdateItemInPersistentStore(download);
+  }
+}
+
 void DownloadManager::UpdateDownloadProgress() {
   delegate_->DownloadProgressUpdated();
 }
 
 int DownloadManager::RemoveDownloadItems(
     const DownloadVector& pending_deletes) {
   if (pending_deletes.empty())
     return 0;
 
   // Delete from internal maps.
   for (DownloadVector::const_iterator it = pending_deletes.begin();
       it != pending_deletes.end();
       ++it) {
     DownloadItem* download = *it;
     DCHECK(download);
     history_downloads_.erase(download->db_handle());
     save_page_downloads_.erase(download->id());
     downloads_.erase(download);
   }
 
   // Tell observers to refresh their views.
   NotifyModelChanged();
 
   // 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 DownloadManager::RemoveDownload(DownloadItem* download) {
-  // Make history update.  Ignores if db_handle isn't in history.
-  delegate_->RemoveItemFromPersistentStore(download->db_handle());
+void DownloadManager::RemoveDownload(int64 download_handle) {
+  DownloadMap::iterator it = history_downloads_.find(download_handle);
+  if (it == history_downloads_.end())
+    return;
+
+  // Make history update.
+  DownloadItem* download = it->second;
+  delegate_->RemoveItemFromPersistentStore(download);
 
   // Remove from our tables and delete.
   int downloads_count = RemoveDownloadItems(DownloadVector(1, download));
   DCHECK_EQ(1, downloads_count);
 }
 
 int DownloadManager::RemoveDownloadsBetween(const base::Time remove_begin,
                                             const base::Time remove_end) {
   delegate_->RemoveItemsFromPersistentStoreBetween(remove_begin, remove_end);
 
@@ skipping 129 matching lines @@
   int32 download_id = *id_ptr;
   delete id_ptr;
 
   DownloadItem* download = GetActiveDownloadItem(download_id);
   if (!download)
     return;
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
-  download->Cancel();
+  // TODO(ahendrickson) -- This currently has no effect, as the download is
+  // not put on the active list until the file selection is complete.  Need
+  // to put it on the active list earlier in the process.
+  RemoveFromActiveList(download);
+
+  download->OffThreadCancel(file_manager_);
 }
 
 // Operations posted to us from the history service ----------------------------
 
 // The history service has retrieved all download entries. 'entries' contains
 // 'DownloadPersistentStoreInfo's in sorted order (by ascending start_time).
 void DownloadManager::OnPersistentStoreQueryComplete(
     std::vector<DownloadPersistentStoreInfo>* entries) {
   // TODO(rdsmith): Remove this and related logic when
   // http://crbug.com/84508 is fixed.
@@ skipping 50 matching lines @@
   // It's valid that we don't find a matching item, i.e. on shutdown.
 }
 
 // Once the new DownloadItem's creation info has been committed to the history
 // service, we associate the DownloadItem with the db handle, update our
 // 'history_downloads_' map and inform observers.
 void DownloadManager::OnDownloadItemAddedToPersistentStore(int32 download_id,
                                                            int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download) {
-    // The download was cancelled while the persistent store was entering it.
-    // We resolve this race by turning around and deleting it in the
-    // persistent store (implicitly treating it as a failure in download
-    // initiation, which is appropriate as the only places the cancel could
-    // have come from were in resolving issues (like the file name) which
-    // we need to have resolved for persistent store insertion).
-
-    // Make sure we haven't already been shutdown (the callback raced
-    // with shutdown), as that would mean that we no longer have access
-    // to the persistent store.  In that case, the history will be cleaned up
-    // on next persistent store query.
-    if (shutdown_needed_)
-      delegate_->RemoveItemFromPersistentStore(db_handle);
+  if (!download)
     return;
-  }
 
   VLOG(20) << __FUNCTION__ << "()" << " db_handle = " << db_handle
            << " download_id = " << download_id
            << " download = " << download->DebugString(true);
 
   // TODO(rdsmith): Remove after http://crbug.com/85408 resolved.
   int64 largest_handle = largest_db_handle_in_history_;
   base::debug::Alias(&largest_handle);
   int32 matching_item_download_id
       = (ContainsKey(history_downloads_, db_handle) ?
          history_downloads_[db_handle]->id() : 0);
   base::debug::Alias(&matching_item_download_id);
 
   CHECK(!ContainsKey(history_downloads_, db_handle));
 
-  CHECK(download->IsInProgress());
   AddDownloadItemToHistory(download, db_handle);
 
-  MaybeCompleteDownload(download);
+  // If the download is still in progress, try to complete it.
+  //
+  // Otherwise, download has been cancelled or interrupted before we've
+  // received the DB handle.  We post one final message to the history
+  // service so that it can be properly in sync with the DownloadItem's
+  // completion status, and also inform any observers so that they get
+  // more than just the start notification.
+  if (download->IsInProgress()) {
+    MaybeCompleteDownload(download);
+  } else {
+    // TODO(rdsmith): Convert to DCHECK() when http://crbug.com/84508
+    // is fixed.
+    CHECK(download->IsCancelled())
+        << " download = " << download->DebugString(true);
+    in_progress_.erase(download_id);
+    active_downloads_.erase(download_id);
+    delegate_->UpdateItemInPersistentStore(download);
+    download->UpdateObservers();
+  }
 }
 
 void DownloadManager::ShowDownloadInBrowser(DownloadItem* download) {
   // The 'contents' may no longer exist if the user closed the tab before we
   // get this start completion event.
   DownloadRequestHandle request_handle = download->request_handle();
   TabContents* content = request_handle.GetTabContents();
 
   // If the contents no longer exists, we ask the embedder to suggest another
   // tab.
@@ skipping 18 matching lines @@
   // not its id, so we have to iterate.
   for (DownloadMap::iterator it = history_downloads_.begin();
        it != history_downloads_.end(); ++it) {
     DownloadItem* item = it->second;
     if (item->id() == download_id)
       return item;
   }
   return NULL;
 }
 
-void DownloadManager::GetInProgressDownloads(
-    std::vector<DownloadItem*>* result) {
-  DCHECK(result);
-
-  for (DownloadMap::iterator it = active_downloads_.begin();
-       it != active_downloads_.end(); ++it) {
-    result->push_back(it->second);
-  }
-}
-
 DownloadItem* DownloadManager::GetActiveDownloadItem(int download_id) {
-  if (!ContainsKey(active_downloads_, download_id))
-    return NULL;
-
+  DCHECK(ContainsKey(active_downloads_, download_id));
   DownloadItem* download = active_downloads_[download_id];
   DCHECK(download != NULL);
   return download;
 }
 
 // Confirm that everything in all maps is also in |downloads_|, and that
 // everything in |downloads_| is also in some other map.
 void DownloadManager::AssertContainersConsistent() const {
 #if !defined(NDEBUG)
   // Turn everything into sets.
@@ skipping 95 matching lines @@
     DCHECK(ContainsKey(save_page_downloads_, download->id()));
     save_page_downloads_.erase(download->id());
 
     if (download->IsComplete())
       NotificationService::current()->Notify(
           content::NOTIFICATION_SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
           Source<DownloadManager>(this),
           Details<DownloadItem>(download));
   }
 }