Made the cancel from CancelDownloadOnRename go through the full cancel path,

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 34 matching lines @@
 }
 
 }  // namespace
 
 DownloadManager::DownloadManager(DownloadManagerDelegate* delegate,
                                  DownloadStatusUpdater* status_updater)
     : shutdown_needed_(false),
       browser_context_(NULL),
       file_manager_(NULL),
       status_updater_(status_updater->AsWeakPtr()),
-      delegate_(delegate) {
+      delegate_(delegate),
+      largest_db_handle_in_history_(DownloadItem::kUninitializedHandle) {
   if (status_updater_)
     status_updater_->AddDelegate(this);
 }
 
 DownloadManager::~DownloadManager() {
   DCHECK(!shutdown_needed_);
   if (status_updater_)
     status_updater_->RemoveDelegate(this);
 }
 
@@ skipping 215 matching lines @@
   }
 }
 
 void DownloadManager::CreateDownloadItem(DownloadCreateInfo* info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download = new DownloadItem(this, *info,
                                             browser_context_->IsOffTheRecord());
   int32 download_id = info->download_id;
   DCHECK(!ContainsKey(in_progress_, download_id));
+
+  // TODO(rdsmith): Remove after http://crbug.com/85408 resolved.
   CHECK(!ContainsKey(active_downloads_, download_id));
   downloads_.insert(download);
   active_downloads_[download_id] = download;
 }
 
 void DownloadManager::ContinueDownloadWithPath(DownloadItem* download,
                                                const FilePath& chosen_file) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download);
 
@@ skipping 198 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::DownloadCancelled(int32 download_id) {
+void DownloadManager::CancelDownload(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DownloadMap::iterator it = in_progress_.find(download_id);
-  if (it == in_progress_.end())
+  DownloadItem* download = GetDownloadItem(download_id);
+  if (!download)
     return;
-  DownloadItem* download = it->second;
 
-  VLOG(20) << __FUNCTION__ << "()" << " download_id = " << download_id
+  download->Cancel(true);
+}
+
+void DownloadManager::DownloadCancelledInternal(DownloadItem* download) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  int download_id = download->id();
+
+  VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
   // 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(it);
+    in_progress_.erase(download_id);
     active_downloads_.erase(download_id);
     UpdateDownloadProgress();  // Reflect removal from in_progress_.
     delegate_->UpdateItemInPersistentStore(download);
+
+    // This function is called from the DownloadItem, so DI state
+    // should already have been updated.
     AssertQueueStateConsistent(download);
   }
 
-  DownloadCancelledInternal(download_id, download->request_handle());
-}
-
-void DownloadManager::DownloadCancelledInternal(
-    int download_id, const DownloadRequestHandle& request_handle) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  request_handle.CancelRequest();
-
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          file_manager_, &DownloadFileManager::CancelDownload, download_id));
+  download->OffThreadCancel(file_manager_);
 }
 
 void DownloadManager::OnDownloadError(int32 download_id,
                                       int64 size,
                                       int os_error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = active_downloads_.find(download_id);
   // A cancel at the right time could remove the download from the
   // |active_downloads_| map before we get here.
   if (it == active_downloads_.end())
@@ skipping 204 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);
 
-  DownloadCancelledInternal(download_id, download->request_handle());
+  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.
+  int64 largest_handle = 0;
+
   for (size_t i = 0; i < entries->size(); ++i) {
     DownloadItem* download = new DownloadItem(this, entries->at(i));
+    // TODO(rdsmith): Remove after http://crbug.com/85408 resolved.
     CHECK(!ContainsKey(history_downloads_, download->db_handle()));
     downloads_.insert(download);
     history_downloads_[download->db_handle()] = download;
     VLOG(20) << __FUNCTION__ << "()" << i << ">"
              << " download = " << download->DebugString(true);
+
+    if (download->db_handle() > largest_handle)

[benjhayden, 2011/08/25 20:41:40]

Why not use largest_db_handle_in_history_ here and kill largest_handle?

[Randy Smith (Not in Mondays), 2011/08/25 21:19:55]

Because I'm being over-paranoid; I'll fix that.  (More specifically, because I
wanted to change ldhih_ atomically, but it's only read on the UI thread and I'm
on the UI thread, and I don't need to worry about that.)  Done.

+      largest_handle = download->db_handle();
   }
+  largest_db_handle_in_history_ = largest_handle;
   NotifyModelChanged();
   CheckForHistoryFilesRemoval();
 }
 
 void DownloadManager::AddDownloadItemToHistory(DownloadItem* download,
                                                int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // TODO(rdsmith): Convert to DCHECK() when http://crbug.com/84508
   // is fixed.
   CHECK_NE(DownloadItem::kUninitializedHandle, db_handle);
 
   DCHECK(download->db_handle() == DownloadItem::kUninitializedHandle);
   download->set_db_handle(db_handle);
 
+  // TODO(rdsmith): Convert to DCHECK() when http://crbug.com/84508
+  // is fixed.
   CHECK(!ContainsKey(history_downloads_, download->db_handle()));
   history_downloads_[download->db_handle()] = download;
 
   // Show in the appropriate browser UI.
   // This includes buttons to save or cancel, for a dangerous download.
   ShowDownloadInBrowser(download);
 
   // Inform interested objects about the new download.
   NotifyModelChanged();
 }
@@ skipping 16 matching lines @@
                                                            int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download = GetActiveDownloadItem(download_id);
   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.
+  CHECK(!ContainsKey(history_downloads_, download->db_handle()));
+  int64 largest_handle = largest_db_handle_in_history_;
+  base::debug::Alias(&largest_handle);
+
   AddDownloadItemToHistory(download, db_handle);
 
   // 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) {
@@ skipping 117 matching lines @@
   // have been cleared.
   if (it == save_page_downloads_.end())
     return;
 
   DownloadItem* download = it->second;
   if (!download) {
     NOTREACHED();
     return;
   }
 
+  // TODO(rdsmith): Remove after http://crbug.com/85408 resolved.
+  CHECK(!ContainsKey(history_downloads_, download->db_handle()));
+  int64 largest_handle = largest_db_handle_in_history_;
+  base::debug::Alias(&largest_handle);
+
   AddDownloadItemToHistory(download, db_handle);
 
   // Finalize this download if it finished before the history callback.
   if (!download->IsInProgress())
     SavePageDownloadFinished(download);
 }
 
 void DownloadManager::SavePageDownloadFinished(DownloadItem* download) {
   if (download->db_handle() != DownloadItem::kUninitializedHandle) {
     delegate_->UpdateItemInPersistentStore(download);
     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));
   }
 }