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

Also add various tests required or enabled by this change.

This changes two aspects of Cancel semantics (for downloads, not save page):
a) Cancel can now be called anytime during the lifetime of a download, and
b) if it is called before the history callback occurs, the download will be removed from the system (no where to store it persistently while waiting)

BUG=85408

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=101510

Review URL: http://codereview.chromium.org/7796014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@102126 0039d316-1c4b-4281-b951-d872f2087c98

29 files changed, 765 insertions, 311 deletions

diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index 71f1eac..28c1f53 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -3127,7 +3127,7 @@ void TestingAutomationProvider::PerformActionOnDownload(
   } else if (action == "cancel") {
     selected_item->AddObserver(new AutomationProviderDownloadUpdatedObserver(
         this, reply_message, false));
-    selected_item->Cancel(true);
+    selected_item->Cancel();
   } else {
     AutomationJSONReply(this, reply_message)
         .SendError(StringPrintf("Invalid action '%s' given.", action.c_str()));
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index e3055bc..89d72ab 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -220,8 +220,8 @@ void ChromeDownloadManagerDelegate::UpdatePathForItemInPersistentStore(
 }
 
 void ChromeDownloadManagerDelegate::RemoveItemFromPersistentStore(
-    DownloadItem* item) {
-  download_history_->RemoveEntry(item);
+    int64 db_handle) {
+  download_history_->RemoveEntry(db_handle);
 }
 
 void ChromeDownloadManagerDelegate::RemoveItemsFromPersistentStoreBetween(
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index b006e53..e5d7197 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -68,7 +68,7 @@ class ChromeDownloadManagerDelegate
   virtual void UpdatePathForItemInPersistentStore(
       DownloadItem* item,
       const FilePath& new_path) OVERRIDE;
-  virtual void RemoveItemFromPersistentStore(DownloadItem* item) OVERRIDE;
+  virtual void RemoveItemFromPersistentStore(int64 db_handle) OVERRIDE;
   virtual void RemoveItemsFromPersistentStoreBetween(
       const base::Time remove_begin,
       const base::Time remove_end) OVERRIDE;
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index 8c5f3d1..ac39150 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -58,6 +58,7 @@ const FilePath kLargeThemePath(FILE_PATH_LITERAL("extensions/theme2.crx"));
 enum DangerousDownloadAction {
   ON_DANGEROUS_DOWNLOAD_ACCEPT,  // Accept the download
   ON_DANGEROUS_DOWNLOAD_DENY,  // Deny the download
+  ON_DANGEROUS_DOWNLOAD_IGNORE,  // Don't do anything; calling code will handle.
   ON_DANGEROUS_DOWNLOAD_FAIL  // Fail if a dangerous download is seen
 };
 
@@ -73,7 +74,7 @@ void DenyDangerousDownload(scoped_refptr<DownloadManager> download_manager,
                            int32 download_id) {
   DownloadItem* download = download_manager->GetDownloadItem(download_id);
   ASSERT_TRUE(download->IsPartialDownload());
-  download->Cancel(true);
+  download->Cancel();
   download->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
 }
 
@@ -92,8 +93,10 @@ void DenyDangerousDownload(scoped_refptr<DownloadManager> download_manager,
 class DownloadsObserver : public DownloadManager::Observer,
                           public DownloadItem::Observer {
  public:
+  typedef std::set<DownloadItem::DownloadState> StateSet;
+
   // Create an object that will be considered finished when |wait_count|
-  // download items have entered state |download_finished_state|.
+  // download items have entered any states in |download_finished_states|.
   // If |finish_on_select_file| is true, the object will also be
   // considered finished if the DownloadManager raises a
   // SelectFileDialogDisplayed() notification.
@@ -102,20 +105,21 @@ class DownloadsObserver : public DownloadManager::Observer,
   // to treat as completion events.
   DownloadsObserver(DownloadManager* download_manager,
                     size_t wait_count,
-                    DownloadItem::DownloadState download_finished_state,
+                    StateSet download_finished_states,
                     bool finish_on_select_file,
                     DangerousDownloadAction dangerous_download_action)
       : download_manager_(download_manager),
         wait_count_(wait_count),
         finished_downloads_at_construction_(0),
         waiting_(false),
-        download_finished_state_(download_finished_state),
+        download_finished_states_(download_finished_states),
         finish_on_select_file_(finish_on_select_file),
         select_file_dialog_seen_(false),
         dangerous_download_action_(dangerous_download_action) {
     download_manager_->AddObserver(this);  // Will call initial ModelChanged().
     finished_downloads_at_construction_ = finished_downloads_.size();
-    EXPECT_NE(DownloadItem::REMOVING, download_finished_state)
+    EXPECT_TRUE(download_finished_states.find(DownloadItem::REMOVING) ==
+                download_finished_states.end())
         << "Waiting for REMOVING is not supported.  Try COMPLETE.";
   }
 
@@ -194,12 +198,16 @@ class DownloadsObserver : public DownloadManager::Observer,
           ADD_FAILURE() << "Unexpected dangerous download item.";
           break;
 
+        case ON_DANGEROUS_DOWNLOAD_IGNORE:
+          break;
+
         default:
           NOTREACHED();
       }
     }
 
-    if (download->state() == download_finished_state_) {
+    if (download_finished_states_.find(download->state()) !=
+        download_finished_states_.end()) {
       DownloadInFinalState(download);
     }
   }
@@ -302,8 +310,8 @@ class DownloadsObserver : public DownloadManager::Observer,
   // all downloads completing.
   bool waiting_;
 
-  // The state on which to consider the DownloadItem finished.
-  DownloadItem::DownloadState download_finished_state_;
+  // The states on which to consider the DownloadItem finished.
+  StateSet download_finished_states_;
 
   // True if we should transition the DownloadsObserver to finished if
   // the select file dialog comes up.
@@ -321,121 +329,36 @@ class DownloadsObserver : public DownloadManager::Observer,
   DISALLOW_COPY_AND_ASSIGN(DownloadsObserver);
 };
 
-// WaitForFlush() returns after:
-//      * There are no IN_PROGRESS download items remaining on the
-//        DownloadManager.
-//      * There have been two round trip messages through the file and
-//        IO threads.
-// This almost certainly means that a Download cancel has propagated through
-// the system.
-class DownloadsFlushObserver
-    : public DownloadManager::Observer,
-      public DownloadItem::Observer,
-      public base::RefCountedThreadSafe<DownloadsFlushObserver> {
+// Ping through an arbitrary set of threads.  Must be run from the UI
+// thread.
+class ThreadPinger : public base::RefCountedThreadSafe<ThreadPinger> {
  public:
-  explicit DownloadsFlushObserver(DownloadManager* download_manager)
-      : download_manager_(download_manager),
-        waiting_for_zero_inprogress_(true) {}
+  ThreadPinger(const BrowserThread::ID ids[], size_t num_ids) :
+      next_thread_index_(0u),
+      ids_(ids, ids + num_ids) {}
 
-  void WaitForFlush() {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-    download_manager_->AddObserver(this);
+  void Ping() {
+    if (ids_.size() == 0)
+      return;
+    NextThread();
     ui_test_utils::RunMessageLoop();
   }
 
-  // DownloadsManager observer methods.
-  virtual void ModelChanged() {
-    // Model has changed, so there may be more DownloadItems to observe.
-    CheckDownloadsInProgress(true);
-  }
-
-  // DownloadItem observer methods.
-  virtual void OnDownloadUpdated(DownloadItem* download) {
-    // No change in DownloadItem set on manager.
-    CheckDownloadsInProgress(false);
-  }
-  virtual void OnDownloadOpened(DownloadItem* download) {}
-
- protected:
-  friend class base::RefCountedThreadSafe<DownloadsFlushObserver>;
-
-  virtual ~DownloadsFlushObserver() {
-    download_manager_->RemoveObserver(this);
-    for (std::set<DownloadItem*>::iterator it = downloads_observed_.begin();
-         it != downloads_observed_.end(); ++it) {
-      (*it)->RemoveObserver(this);
-    }
-  }
-
  private:
-  // If we're waiting for that flush point, check the number
-  // of downloads in the IN_PROGRESS state and take appropriate
-  // action.  If requested, also observes all downloads while iterating.
-  void CheckDownloadsInProgress(bool observe_downloads) {
-    if (waiting_for_zero_inprogress_) {
-      int count = 0;
-
-      std::vector<DownloadItem*> downloads;
-      download_manager_->SearchDownloads(string16(), &downloads);
-      std::vector<DownloadItem*>::iterator it = downloads.begin();
-      for (; it != downloads.end(); ++it) {
-        if ((*it)->state() == DownloadItem::IN_PROGRESS)
-          count++;
-        if (observe_downloads) {
-          if (downloads_observed_.find(*it) == downloads_observed_.end()) {
-            (*it)->AddObserver(this);
-          }
-          // Download items are forever, and we don't want to make
-          // assumptions about future state transitions, so once we
-          // start observing them, we don't stop until destruction.
-        }
-      }
-
-      if (count == 0) {
-        waiting_for_zero_inprogress_ = false;
-        // Stop observing DownloadItems.  We maintain the observation
-        // of DownloadManager so that we don't have to independently track
-        // whether we are observing it for conditional destruction.
-        for (std::set<DownloadItem*>::iterator it = downloads_observed_.begin();
-             it != downloads_observed_.end(); ++it) {
-          (*it)->RemoveObserver(this);
-        }
-        downloads_observed_.clear();
-
-        // Trigger next step.  We need to go past the IO thread twice, as
-        // there's a self-task posting in the IO thread cancel path.
-        BrowserThread::PostTask(
-            BrowserThread::FILE, FROM_HERE,
-            NewRunnableMethod(this,
-                              &DownloadsFlushObserver::PingFileThread, 2));
-      }
-    }
-  }
-
-  void PingFileThread(int cycle) {
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        NewRunnableMethod(this, &DownloadsFlushObserver::PingIOThread,
-                          cycle));
-  }
-
-  void PingIOThread(int cycle) {
-    if (--cycle) {
+  void NextThread() {
+    if (next_thread_index_ == ids_.size()) {
       BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          NewRunnableMethod(this, &DownloadsFlushObserver::PingFileThread,
-                            cycle));
+          BrowserThread::UI, FROM_HERE, new MessageLoop::QuitTask());
     } else {
+      BrowserThread::ID next_id(ids_[next_thread_index_++]);
       BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE, new MessageLoop::QuitTask());
+          next_id, FROM_HERE,
+          NewRunnableMethod(this, &ThreadPinger::NextThread));
     }
   }
 
-  DownloadManager* download_manager_;
-  std::set<DownloadItem*> downloads_observed_;
-  bool waiting_for_zero_inprogress_;
-
-  DISALLOW_COPY_AND_ASSIGN(DownloadsFlushObserver);
+  size_t next_thread_index_;
+  std::vector<BrowserThread::ID> ids_;
 };
 
 // Collect the information from FILE and IO threads needed for the Cancel
@@ -506,6 +429,39 @@ class PickSuggestedFileDelegate : public ChromeDownloadManagerDelegate {
   }
 };
 
+// Don't respond to ChooseDownloadPath until a cancel is requested
+// out of band.  Can handle only one outstanding request at a time
+// for a download path.
+class BlockCancelFileDelegate : public ChromeDownloadManagerDelegate {
+ public:
+  explicit BlockCancelFileDelegate(Profile* profile)
+      : ChromeDownloadManagerDelegate(profile),
+        choose_download_path_called_(false),
+        choose_download_path_data_(NULL) {
+    SetDownloadManager(profile->GetDownloadManager());
+  }
+
+  virtual void ChooseDownloadPath(TabContents* tab_contents,
+                                  const FilePath& suggested_path,
+                                  void* data) OVERRIDE {
+    CHECK(!choose_download_path_called_);
+    choose_download_path_called_ = true;
+    choose_download_path_data_ = data;
+  }
+
+  void CancelOutstandingDownloadPathRequest() {
+    if (choose_download_path_called_) {
+      if (download_manager_)
+        download_manager_->FileSelectionCanceled(choose_download_path_data_);
+      choose_download_path_called_ = false;
+      choose_download_path_data_ = NULL;
+    }
+  }
+ private:
+  bool choose_download_path_called_;
+  void *choose_download_path_data_;
+};
+
 // Get History Information.
 class DownloadsHistoryDataCollector {
  public:
@@ -623,6 +579,25 @@ class MockDownloadOpeningObserver : public DownloadManager::Observer {
   DISALLOW_COPY_AND_ASSIGN(MockDownloadOpeningObserver);
 };
 
+static const DownloadItem::DownloadState kTerminalStates[] = {
+  DownloadItem::CANCELLED,
+  DownloadItem::INTERRUPTED,
+  DownloadItem::COMPLETE,
+};
+
+static const DownloadItem::DownloadState kInProgressStates[] = {
+  DownloadItem::IN_PROGRESS,
+};
+
+static const BrowserThread::ID kIOFileX2ThreadList[] = {
+  BrowserThread::IO, BrowserThread::FILE,
+  BrowserThread::IO, BrowserThread::FILE };
+
+static const BrowserThread::ID kExternalTerminationThreadList[] = {
+  BrowserThread::IO, BrowserThread::IO, BrowserThread::FILE };
+
+// Not in anonymous namespace so that friend class from DownloadManager
+// can target it.
 class DownloadTest : public InProcessBrowserTest {
  public:
   enum SelectExpectation {
@@ -699,6 +674,12 @@ class DownloadTest : public InProcessBrowserTest {
     return true;
   }
 
+  // For tests that want to test system reaction to files
+  // going away underneath them.
+  void DeleteDownloadsDirectory() {
+    EXPECT_TRUE(downloads_directory_.Delete());
+  }
+
   DownloadPrefs* GetDownloadPrefs(Browser* browser) {
     return DownloadPrefs::FromDownloadManager(
         browser->profile()->GetDownloadManager());
@@ -715,12 +696,29 @@ class DownloadTest : public InProcessBrowserTest {
         browser->profile()->GetDownloadManager();
     return new DownloadsObserver(
         download_manager, num_downloads,
-        DownloadItem::COMPLETE,  // Really done
+        DownloadsObserver::StateSet(
+            kTerminalStates, kTerminalStates + arraysize(kTerminalStates)),
         true,                    // Bail on select file
         ON_DANGEROUS_DOWNLOAD_FAIL);
   }
 
   // Create a DownloadsObserver that will wait for the
+  // specified number of downloads to finish, and is
+  // ok with dangerous downloads.  Note that use of this
+  // waiter is conditional on accepting the dangerous download.
+  DownloadsObserver* CreateDangerousWaiter(
+      Browser* browser, int num_downloads) {
+    DownloadManager* download_manager =
+        browser->profile()->GetDownloadManager();
+    return new DownloadsObserver(
+        download_manager, num_downloads,
+        DownloadsObserver::StateSet(
+            kTerminalStates, kTerminalStates + arraysize(kTerminalStates)),
+        true,                   // Bail on select file
+        ON_DANGEROUS_DOWNLOAD_IGNORE);
+  }
+
+  // Create a DownloadsObserver that will wait for the
   // specified number of downloads to start.
   DownloadsObserver* CreateInProgressWaiter(Browser* browser,
                                             int num_downloads) {
@@ -728,9 +726,11 @@ class DownloadTest : public InProcessBrowserTest {
         browser->profile()->GetDownloadManager();
     return new DownloadsObserver(
         download_manager, num_downloads,
-        DownloadItem::IN_PROGRESS,      // Has started
+        DownloadsObserver::StateSet(
+            kInProgressStates,
+            kInProgressStates + arraysize(kInProgressStates)),
         true,                           // Bail on select file
-        ON_DANGEROUS_DOWNLOAD_FAIL);
+        ON_DANGEROUS_DOWNLOAD_IGNORE);
   }
 
   // Create a DownloadsObserver that will wait for the
@@ -741,11 +741,13 @@ class DownloadTest : public InProcessBrowserTest {
       int num_downloads,
       DownloadItem::DownloadState final_state,
       DangerousDownloadAction dangerous_download_action) {
+    DownloadsObserver::StateSet states;
+    states.insert(final_state);
     DownloadManager* download_manager =
         browser->profile()->GetDownloadManager();
     return new DownloadsObserver(
         download_manager, num_downloads,
-        final_state,
+        states,
         true,                         // Bail on select file
         dangerous_download_action);
   }
@@ -900,12 +902,35 @@ class DownloadTest : public InProcessBrowserTest {
     return true;
   }
 
-  void GetDownloads(Browser* browser, std::vector<DownloadItem*>* downloads) {
+  void GetPersistentDownloads(Browser* browser,
+                              std::vector<DownloadItem*>* downloads) {
     DCHECK(downloads);
+    downloads->clear();
     DownloadManager* manager = browser->profile()->GetDownloadManager();
     manager->SearchDownloads(string16(), downloads);
   }
 
+  void GetInProgressDownloads(Browser* browser,
+                              std::vector<DownloadItem*>* downloads) {
+    downloads->clear();
+    DCHECK(downloads);
+    DownloadManager* manager = browser->profile()->GetDownloadManager();
+    manager->GetInProgressDownloads(downloads);
+  }
+
+  size_t NumberInProgressDownloads(Browser* browser) {
+    std::vector<DownloadItem*> downloads;
+    browser->profile()->GetDownloadManager()->GetInProgressDownloads(
+        &downloads);
+    return downloads.size();
+  }
+
+  void WaitForIOFileX2() {
+    scoped_refptr<ThreadPinger> pinger(
+        new ThreadPinger(kIOFileX2ThreadList, arraysize(kIOFileX2ThreadList)));
+    pinger->Ping();
+  }
+
   // Check that the download UI (shelf on non-chromeos or panel on chromeos)
   // is visible or not as expected. Additionally, check that the filename
   // is present in the UI (currently only on chromeos).
@@ -973,6 +998,39 @@ class DownloadTest : public InProcessBrowserTest {
     browser->profile()->SetDownloadManagerDelegate(new_delegate);
   }
 
+  // Arrange for select file calls on the given browser from the
+  // download manager to block until explicitly released.
+  // Note that the returned pointer is non-owning; the lifetime
+  // of the object will be that of the profile.
+  BlockCancelFileDelegate* BlockSelectFile(Browser* browser) {
+    BlockCancelFileDelegate* new_delegate =
+        new BlockCancelFileDelegate(browser->profile());
+
+    DownloadManager* manager = browser->profile()->GetDownloadManager();
+
+    new_delegate->SetDownloadManager(manager);
+    manager->set_delegate(new_delegate);
+
+    // Gives ownership to Profile.
+    browser->profile()->SetDownloadManagerDelegate(new_delegate);
+
+    return new_delegate;
+  }
+
+  // Wait for an external termination (e.g. signalling
+  // URLRequestSlowDownloadJob to return an error) to propagate through
+  // this sytem.  This involves hopping over to the IO thread (twice,
+  // because of possible self-posts from URLRequestJob) then
+  // chasing the (presumed) notification through the download
+  // system (FILE->UI).
+  void WaitForExternalTermination() {
+    scoped_refptr<ThreadPinger> pinger(
+        new ThreadPinger(
+            kExternalTerminationThreadList,
+            arraysize(kExternalTerminationThreadList)));
+    pinger->Ping();
+  }
+
  private:
   // Location of the test data.
   FilePath test_dir_;
@@ -1039,7 +1097,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadMimeTypeSelect) {
       new DownloadsObserver(
           browser()->profile()->GetDownloadManager(),
           1,
-          DownloadItem::COMPLETE,  // Really done
+          DownloadsObserver::StateSet(
+              kTerminalStates, kTerminalStates + arraysize(kTerminalStates)),
           false,                   // Continue on select file.
           ON_DANGEROUS_DOWNLOAD_FAIL));
   ui_test_utils::NavigateToURLWithDisposition(
@@ -1048,12 +1107,168 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadMimeTypeSelect) {
   observer->WaitForFinished();
   EXPECT_TRUE(observer->select_file_dialog_seen());
 
-  // Check state.
   EXPECT_EQ(1, browser()->tab_count());
   CheckDownload(browser(), file, file);
   CheckDownloadUI(browser(), true, true, file);
 }
 
+// Put up a Select File dialog when the file is downloaded, due to
+// prompt_for_download==true argument to InitialSetup().
+// Confirm that we can cancel the download in that state.
+IN_PROC_BROWSER_TEST_F(DownloadTest, CancelAtFileSelection) {
+  ASSERT_TRUE(InitialSetup(true));
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  BlockCancelFileDelegate* delegate = BlockSelectFile(browser());
+
+  // Download the file and wait.  We expect the Select File dialog to appear.
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG);
+
+  std::vector<DownloadItem*> active_downloads, history_downloads;
+  GetInProgressDownloads(browser(), &active_downloads);
+  ASSERT_EQ(1u, active_downloads.size());
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, active_downloads[0]->state());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  // This should remove the download as it hasn't yet been entered into
+  // the history.
+  active_downloads[0]->Cancel();
+  MessageLoopForUI::current()->RunAllPending();
+
+  GetInProgressDownloads(browser(), &active_downloads);
+  EXPECT_EQ(0u, active_downloads.size());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  // Check state.
+  EXPECT_EQ(1, browser()->tab_count());
+  // Since we exited while the Select File dialog was visible, there should not
+  // be anything in the download shelf and so it should not be visible.
+  CheckDownloadUI(browser(), false, false, FilePath());
+
+  // Return from file selection to release allocated data.
+  delegate->CancelOutstandingDownloadPathRequest();
+}
+
+// Put up a Select File dialog when the file is downloaded, due to
+// prompt_for_download==true argument to InitialSetup().
+// Confirm that we can cancel the download as if the user had hit cancel.
+IN_PROC_BROWSER_TEST_F(DownloadTest, CancelFromFileSelection) {
+  ASSERT_TRUE(InitialSetup(true));
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  BlockCancelFileDelegate* delegate = BlockSelectFile(browser());
+
+  // Download the file and wait.  We expect the Select File dialog to appear.
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG);
+
+  std::vector<DownloadItem*> active_downloads, history_downloads;
+  GetInProgressDownloads(browser(), &active_downloads);
+  ASSERT_EQ(1u, active_downloads.size());
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, active_downloads[0]->state());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  // (Effectively) Click the Cancel button.  This should remove the download
+  // as it hasn't yet been entered into the history.
+  delegate->CancelOutstandingDownloadPathRequest();
+  MessageLoopForUI::current()->RunAllPending();
+
+  GetInProgressDownloads(browser(), &active_downloads);
+  EXPECT_EQ(0u, active_downloads.size());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  // Check state.
+  EXPECT_EQ(1, browser()->tab_count());
+  // Since we exited while the Select File dialog was visible, there should not
+  // be anything in the download shelf and so it should not be visible.
+  CheckDownloadUI(browser(), false, false, FilePath());
+}
+
+// Put up a Select File dialog when the file is downloaded, due to
+// prompt_for_download==true argument to InitialSetup().
+// Confirm that we can remove the download in that state.
+IN_PROC_BROWSER_TEST_F(DownloadTest, RemoveFromFileSelection) {
+  ASSERT_TRUE(InitialSetup(true));
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  BlockCancelFileDelegate* delegate = BlockSelectFile(browser());
+
+  // Download the file and wait.  We expect the Select File dialog to appear.
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG);
+
+  std::vector<DownloadItem*> active_downloads, history_downloads;
+  GetInProgressDownloads(browser(), &active_downloads);
+  ASSERT_EQ(1u, active_downloads.size());
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, active_downloads[0]->state());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  // Confirm the file can be successfully removed from the select file
+  // dialog blocked state.
+  active_downloads[0]->Remove();
+  MessageLoopForUI::current()->RunAllPending();
+
+  GetInProgressDownloads(browser(), &active_downloads);
+  EXPECT_EQ(0u, active_downloads.size());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  EXPECT_EQ(1, browser()->tab_count());
+  // Since we exited while the Select File dialog was visible, there should not
+  // be anything in the download shelf and so it should not be visible.
+  CheckDownloadUI(browser(), false, false, FilePath());
+
+  // Return from file selection to release allocated data.
+  delegate->CancelOutstandingDownloadPathRequest();
+}
+
+// Put up a Select File dialog when the file is downloaded, due to
+// prompt_for_download==true argument to InitialSetup().
+// Confirm that an error coming in from the network works properly
+// when in that state.
+IN_PROC_BROWSER_TEST_F(DownloadTest, InterruptFromFileSelection) {
+  ASSERT_TRUE(InitialSetup(true));
+  GURL url(URLRequestSlowDownloadJob::kKnownSizeUrl);
+
+  BlockCancelFileDelegate* delegate = BlockSelectFile(browser());
+
+  // Download the file and wait.  We expect the Select File dialog to appear.
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG);
+
+  std::vector<DownloadItem*> active_downloads, history_downloads;
+  GetInProgressDownloads(browser(), &active_downloads);
+  ASSERT_EQ(1u, active_downloads.size());
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, active_downloads[0]->state());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  // Complete the download with error.
+  GURL error_url(URLRequestSlowDownloadJob::kErrorFinishDownloadUrl);
+  ui_test_utils::NavigateToURL(browser(), error_url);
+  MessageLoopForUI::current()->RunAllPending();
+  WaitForExternalTermination();
+
+  // Confirm that a download error before entry into history
+  // deletes the download.
+  GetInProgressDownloads(browser(), &active_downloads);
+  EXPECT_EQ(0u, active_downloads.size());
+  GetPersistentDownloads(browser(), &history_downloads);
+  EXPECT_EQ(0u, history_downloads.size());
+
+  // Since we exited while the Select File dialog was visible, there should not
+  // be anything in the download shelf and so it should not be visible.
+  CheckDownloadUI(browser(), false, false, FilePath());
+
+  // Return from file selection to release allocated data.
+  delegate->CancelOutstandingDownloadPathRequest();
+}
+
 // Access a file with a viewable mime-type, verify that a download
 // did not initiate.
 IN_PROC_BROWSER_TEST_F(DownloadTest, NoDownload) {
@@ -1453,25 +1668,37 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, NewWindow) {
   CheckDownload(browser(), file, file);
 }
 
+// Create a download, wait until it's visible on the DownloadManager,
+// then cancel it.
 IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadCancelled) {
   ASSERT_TRUE(InitialSetup(false));
   EXPECT_EQ(1, browser()->tab_count());
 
-  // TODO(rdsmith): Fragile code warning!  The code below relies on the
-  // DownloadsObserver only finishing when the new download has reached
-  // the state of being entered into the history and being user-visible
-  // (that's what's required for the Remove to be valid and for the
-  // download shelf to be visible).  By the pure semantics of
+  // The code below is slightly fragile.  Currently,
+  // the DownloadsObserver will only finish when the new download has
+  // reached the state of being entered into the history and being
+  // user-visible.  However, by the pure semantics of
   // DownloadsObserver, that's not guaranteed; DownloadItems are created
   // in the IN_PROGRESS state and made known to the DownloadManager
   // immediately, so any ModelChanged event on the DownloadManager after
-  // navigation would allow the observer to return.  However, the only
-  // ModelChanged() event the code will currently fire is in
-  // OnCreateDownloadEntryComplete, at which point the download item will
-  // be in the state we need.
-  // The right way to fix this is to create finer grained states on the
-  // DownloadItem, and wait for the state that indicates the item has been
-  // entered in the history and made visible in the UI.
+  // navigation would allow the observer to return.  At some point we'll
+  // probably change the DownloadManager to fire a ModelChanged event
+  // earlier in download processing.  This test should continue to work,
+  // as a cancel is valid at any point during download process.  However,
+  // at that point it'll be testing two different things, depending on
+  // what state the download item has reached when it is cancelled:
+  // a) cancelling from a pre-history entry when the only record of a
+  // download item is on the active_downloads_ queue, and b) cancelling
+  // from a post-history entry when the download is present on the
+  // history_downloads_ list.
+  //
+  // Note that the above is why we don't do any UI testing here--we don't
+  // know whether or not the download item has been made visible to the user.
+
+  // TODO(rdsmith): After we fire ModelChanged events at finer granularity,
+  // split this test into subtests that tests canceling from each separate
+  // download item state.  Also include UI testing as appropriate in those
+  // split tests.
 
   // Create a download, wait until it's started, and confirm
   // we're in the expected state.
@@ -1482,27 +1709,199 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadCancelled) {
   observer->WaitForFinished();
 
   std::vector<DownloadItem*> downloads;
-  browser()->profile()->GetDownloadManager()->SearchDownloads(
-      string16(), &downloads);
+  GetPersistentDownloads(browser(), &downloads);
   ASSERT_EQ(1u, downloads.size());
   ASSERT_EQ(DownloadItem::IN_PROGRESS, downloads[0]->state());
-  CheckDownloadUI(browser(), true, true, FilePath());
 
   // Cancel the download and wait for download system quiesce.
   downloads[0]->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
-  scoped_refptr<DownloadsFlushObserver> flush_observer(
-      new DownloadsFlushObserver(browser()->profile()->GetDownloadManager()));
-  flush_observer->WaitForFlush();
+  ASSERT_EQ(0u, NumberInProgressDownloads(browser()));
+  WaitForIOFileX2();
 
   // Get the important info from other threads and check it.
   scoped_refptr<CancelTestDataCollector> info(new CancelTestDataCollector());
   info->WaitForDataCollected();
   EXPECT_EQ(0, info->rdh_pending_requests());
   EXPECT_EQ(0, info->dfm_pending_downloads());
+}
+
+// Do a dangerous download and confirm that the download does
+// not complete until user accept, and that all states are
+// correct along the way.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadDangerous) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-dangerous.jar"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  EXPECT_EQ(1, browser()->tab_count());
+
+  scoped_ptr<DownloadsObserver> observer(
+      CreateInProgressWaiter(browser(), 1));
+  ui_test_utils::NavigateToURL(browser(), url);
+  observer->WaitForFinished();
+
+  // We should have one download, in history, and it should
+  // still be dangerous.
+  std::vector<DownloadItem*> downloads;
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  DownloadItem* download = downloads[0];
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
+  EXPECT_EQ(DownloadItem::DANGEROUS, download->safety_state());
+  EXPECT_EQ(DownloadItem::DANGEROUS_FILE, download->GetDangerType());
+  // In ChromeOS, popup will be up, but file name will be unrecognizable.
+  CheckDownloadUI(browser(), true, true, FilePath());
+
+  // See if accepting completes the download and changes the safety
+  // state.
+  scoped_ptr<DownloadsObserver> completion_observer(
+      CreateDangerousWaiter(browser(), 1));
+  AcceptDangerousDownload(browser()->profile()->GetDownloadManager(),
+                          download->id());
+  completion_observer->WaitForFinished();
+
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  ASSERT_EQ(downloads[0], download);
+  EXPECT_EQ(DownloadItem::COMPLETE, download->state());
+  EXPECT_EQ(DownloadItem::DANGEROUS_BUT_VALIDATED, download->safety_state());
+  CheckDownloadUI(browser(), true, true, file);
+}
+
+// Confirm that a dangerous download that gets a file error before
+// completion ends in the right state (currently cancelled because file
+// errors are non-resumable).  Note that this is really testing
+// to make sure errors from the final rename are propagated properly.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadDangerousFileError) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-dangerous.jar"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  EXPECT_EQ(1, browser()->tab_count());
+
+  scoped_ptr<DownloadsObserver> observer(
+      CreateInProgressWaiter(browser(), 1));
+  ui_test_utils::NavigateToURL(browser(), url);
+  observer->WaitForFinished();
+
+  // We should have one download, in history, and it should
+  // still be dangerous.
+  std::vector<DownloadItem*> downloads;
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  DownloadItem* download = downloads[0];
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
+  EXPECT_EQ(DownloadItem::DANGEROUS, download->safety_state());
+  EXPECT_EQ(DownloadItem::DANGEROUS_FILE, download->GetDangerType());
+  // In ChromeOS, popup will be up, but file name will be unrecognizable.
+  CheckDownloadUI(browser(), true, true, FilePath());
+
+  // Accept it after nuking the directory into which it's being downloaded;
+  // that should complete the download with an error.
+  DeleteDownloadsDirectory();
+  scoped_ptr<DownloadsObserver> completion_observer(
+      CreateDangerousWaiter(browser(), 1));
+  AcceptDangerousDownload(browser()->profile()->GetDownloadManager(),
+                          download->id());
+  completion_observer->WaitForFinished();
+
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  ASSERT_EQ(downloads[0], download);
+  EXPECT_EQ(DownloadItem::INTERRUPTED, download->state());
+  EXPECT_EQ(DownloadItem::DANGEROUS_BUT_VALIDATED, download->safety_state());
+  // In ChromeOS, popup will still be up, but the file will have been
+  // deleted.
+  CheckDownloadUI(browser(), true, true, FilePath());
+}
+
+// Confirm that declining a dangerous download erases it from living memory.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadDangerousDecline) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-dangerous.jar"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  EXPECT_EQ(1, browser()->tab_count());
+
+  scoped_ptr<DownloadsObserver> observer(
+      CreateInProgressWaiter(browser(), 1));
+  ui_test_utils::NavigateToURL(browser(), url);
+  observer->WaitForFinished();
+
+  // We should have one download, in history, and it should
+  // still be dangerous.
+  std::vector<DownloadItem*> downloads;
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  DownloadItem* download = downloads[0];
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
+  EXPECT_EQ(DownloadItem::DANGEROUS, download->safety_state());
+  EXPECT_EQ(DownloadItem::DANGEROUS_FILE, download->GetDangerType());
+  CheckDownloadUI(browser(), true, true, FilePath());
+
+  DenyDangerousDownload(browser()->profile()->GetDownloadManager(),
+                        download->id());
+
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(0u, downloads.size());
+  CheckDownloadUI(browser(), false, true, FilePath());
+}
+
+// Fail a download with a network error partway through, and make sure the
+// state is INTERRUPTED and the error is propagated.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadInterrupted) {
+  ASSERT_TRUE(InitialSetup(false));
+  GURL url(URLRequestSlowDownloadJob::kKnownSizeUrl);
+
+  scoped_ptr<DownloadsObserver> observer(
+      CreateInProgressWaiter(browser(), 1));
+  ui_test_utils::NavigateToURL(browser(), url);
+  observer->WaitForFinished();
+
+  DownloadItem* download1;
+  std::vector<DownloadItem*> downloads;
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  download1 = downloads[0];
+  ASSERT_EQ(DownloadItem::IN_PROGRESS, download1->state());
+  FilePath filename;
+  net::FileURLToFilePath(url, &filename);
+  CheckDownloadUI(browser(), true, true,
+                  download_util::GetCrDownloadPath(filename.BaseName()));
+
+  // Fail the download
+  GURL error_url(URLRequestSlowDownloadJob::kErrorFinishDownloadUrl);
+  ui_test_utils::NavigateToURL(browser(), error_url);
+  MessageLoopForUI::current()->RunAllPending();
+  WaitForExternalTermination();
+
+  // Should still be visible, with INTERRUPTED state.
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  DownloadItem* download = downloads[0];
+  ASSERT_EQ(download, download1);
+  ASSERT_EQ(DownloadItem::INTERRUPTED, download->state());
+  // TODO(rdsmith): Confirm error provided by URLRequest is shown
+  // in DownloadItem.
+  CheckDownloadUI(browser(), true, true, FilePath());
+
+  // Confirm cancel does nothing.
+  download->Cancel();
+  MessageLoopForUI::current()->RunAllPending();
+
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(1u, downloads.size());
+  ASSERT_EQ(download, downloads[0]);
+  ASSERT_EQ(DownloadItem::INTERRUPTED, download->state());
+  CheckDownloadUI(browser(), true, true, FilePath());
+
+  // Confirm remove gets rid of it.
+  download->Remove();
+  download = NULL;
+  MessageLoopForUI::current()->RunAllPending();
 
-  // Using "DownloadItem::Remove" follows the discard dangerous download path,
-  // which completely removes the browser from the shelf and closes the shelf
-  // if it was there. Download panel stays open on ChromeOS.
+  GetPersistentDownloads(browser(), &downloads);
+  ASSERT_EQ(0u, downloads.size());
   CheckDownloadUI(browser(), false, true, FilePath());
 }
 
@@ -1520,7 +1919,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadHistoryCheck) {
 
   // Get details of what downloads have just happened.
   std::vector<DownloadItem*> downloads;
-  GetDownloads(browser(), &downloads);
+  GetPersistentDownloads(browser(), &downloads);
   ASSERT_EQ(1u, downloads.size());
   int64 db_handle = downloads[0]->db_handle();
 
@@ -1632,8 +2031,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, AutoOpen) {
 
   // Find the download and confirm it was opened.
   std::vector<DownloadItem*> downloads;
-  browser()->profile()->GetDownloadManager()->SearchDownloads(
-      string16(), &downloads);
+  GetPersistentDownloads(browser(), &downloads);
   ASSERT_EQ(1u, downloads.size());
   EXPECT_EQ(DownloadItem::COMPLETE, downloads[0]->state());
   EXPECT_TRUE(downloads[0]->opened());
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc
index 3da198b..9c9a26e 100644
--- a/chrome/browser/download/download_history.cc
+++ b/chrome/browser/download/download_history.cc
@@ -116,14 +116,14 @@ void DownloadHistory::UpdateDownloadPath(DownloadItem* download_item,
     hs->UpdateDownloadPath(new_path, download_item->db_handle());
 }
 
-void DownloadHistory::RemoveEntry(DownloadItem* download_item) {
+void DownloadHistory::RemoveEntry(int64 db_handle) {
   // No update necessary if the download was initiated while in incognito mode.
-  if (download_item->db_handle() <= DownloadItem::kUninitializedHandle)
+  if (db_handle <= DownloadItem::kUninitializedHandle)
     return;
 
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   if (hs)
-    hs->RemoveDownload(download_item->db_handle());
+    hs->RemoveDownload(db_handle);
 }
 
 void DownloadHistory::RemoveEntriesBetween(const base::Time remove_begin,
diff --git a/chrome/browser/download/download_history.h b/chrome/browser/download/download_history.h
index 6d6b237..9ef445c 100644
--- a/chrome/browser/download/download_history.h
+++ b/chrome/browser/download/download_history.h
@@ -47,8 +47,10 @@ class DownloadHistory {
   void UpdateDownloadPath(DownloadItem* download_item,
                           const FilePath& new_path);
 
-  // Removes |download_item| from the history database.
-  void RemoveEntry(DownloadItem* download_item);
+  // Removes the download identified by |db_handle| from the history database.
+  // |db_handle| is used instead of the DownloadItem pointer to allow
+  // removal after deletion of the download item.
+  void RemoveEntry(int64 db_handle);
 
   // Removes download-related history entries in the given time range.
   void RemoveEntriesBetween(const base::Time remove_begin,
diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc
index 891a5ed..f5fec1b 100644
--- a/chrome/browser/download/download_item_model.cc
+++ b/chrome/browser/download/download_item_model.cc
@@ -26,7 +26,7 @@ DownloadItemModel::DownloadItemModel(DownloadItem* download)
 }
 
 void DownloadItemModel::CancelTask() {
-  download_->Cancel(true /* update history service */);
+  download_->Cancel();
 }
 
 string16 DownloadItemModel::GetStatusText() {
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index 3380046..6a2974b 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -534,7 +534,7 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) {
                                        simple_size,
                                        simple_total));
 
-  download->Cancel(true);
+  download->Cancel();
 
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::INTERRUPTED));
@@ -630,7 +630,7 @@ TEST_F(DownloadManagerTest, DownloadFileErrorTest) {
             download_item_model->GetStatusText());
 
   // Clean up.
-  download->Cancel(true);
+  download->Cancel();
   message_loop_.RunAllPending();
 }
 
@@ -676,10 +676,10 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {
 
   download_file->AppendDataToFile(kTestData, kTestDataLen);
 
-  download->Cancel(false);
+  download->Cancel();
   message_loop_.RunAllPending();
 
-  EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
+  EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
diff --git a/chrome/browser/ui/cocoa/download/download_item_controller.mm b/chrome/browser/ui/cocoa/download/download_item_controller.mm
index eb854ba..d89cfdc 100644
--- a/chrome/browser/ui/cocoa/download/download_item_controller.mm
+++ b/chrome/browser/ui/cocoa/download/download_item_controller.mm
@@ -353,7 +353,7 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu {
                            base::Time::Now() - creationTime_);
   DownloadItem* download = bridge_->download_model()->download();
   if (download->IsPartialDownload())
-    download->Cancel(true);
+    download->Cancel();
   download->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
   // WARNING: we are deleted at this point.  Don't access 'this'.
 }
diff --git a/chrome/browser/ui/gtk/download/download_item_gtk.cc b/chrome/browser/ui/gtk/download/download_item_gtk.cc
index 5f9218e..b0d61a1 100644
--- a/chrome/browser/ui/gtk/download/download_item_gtk.cc
+++ b/chrome/browser/ui/gtk/download/download_item_gtk.cc
@@ -878,6 +878,6 @@ void DownloadItemGtk::OnDangerousDecline(GtkWidget* button) {
   UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download",
                            base::Time::Now() - creation_time_);
   if (get_download()->IsPartialDownload())
-    get_download()->Cancel(true);
+    get_download()->Cancel();
   get_download()->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
 }
diff --git a/chrome/browser/ui/panels/panel_browsertest.cc b/chrome/browser/ui/panels/panel_browsertest.cc
index f3b29b1..ec8c538 100644
--- a/chrome/browser/ui/panels/panel_browsertest.cc
+++ b/chrome/browser/ui/panels/panel_browsertest.cc
@@ -890,7 +890,7 @@ class DownloadObserver : public DownloadManager::Observer {
       return;
 
     EXPECT_EQ(1U, downloads.size());
-    downloads.front()->Cancel(false);  // Don't actually need to download it.
+    downloads.front()->Cancel();  // Don't actually need to download it.
 
     saw_download_ = true;
     EXPECT_TRUE(waiting_);
diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc
index 02010f7..79ecc01 100644
--- a/chrome/browser/ui/views/download/download_item_view.cc
+++ b/chrome/browser/ui/views/download/download_item_view.cc
@@ -654,7 +654,7 @@ void DownloadItemView::ButtonPressed(
     UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download",
                              base::Time::Now() - creation_time_);
     if (download_->IsPartialDownload())
-      download_->Cancel(true);
+      download_->Cancel();
     download_->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
     // WARNING: we are deleted at this point.  Don't access 'this'.
   } else if (sender == save_button_) {
diff --git a/chrome/browser/ui/webui/active_downloads_ui.cc b/chrome/browser/ui/webui/active_downloads_ui.cc
index 3d5af17..5114b9f 100644
--- a/chrome/browser/ui/webui/active_downloads_ui.cc
+++ b/chrome/browser/ui/webui/active_downloads_ui.cc
@@ -270,7 +270,7 @@ void ActiveDownloadsHandler::HandleCancelDownload(const ListValue* args) {
   DownloadItem* item = GetDownloadById(args);
   if (item) {
     if (item->IsPartialDownload())
-      item->Cancel(true);
+      item->Cancel();
     item->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
   }
 }
diff --git a/chrome/browser/ui/webui/chromeos/imageburner/imageburner_ui.cc b/chrome/browser/ui/webui/chromeos/imageburner/imageburner_ui.cc
index 9e54398..1130256 100644
--- a/chrome/browser/ui/webui/chromeos/imageburner/imageburner_ui.cc
+++ b/chrome/browser/ui/webui/chromeos/imageburner/imageburner_ui.cc
@@ -509,7 +509,7 @@ void WebUIHandler::ProcessError(int message_id) {
       // We don't want to process Download Cancelled signal.
       active_download_item_->RemoveObserver(this);
       if (active_download_item_->IsPartialDownload())
-        active_download_item_->Cancel(true);
+        active_download_item_->Cancel();
       active_download_item_->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
       active_download_item_ = NULL;
       CleanupDownloadObjects();
diff --git a/chrome/browser/ui/webui/downloads_dom_handler.cc b/chrome/browser/ui/webui/downloads_dom_handler.cc
index 52e986c..5f74dd2 100644
--- a/chrome/browser/ui/webui/downloads_dom_handler.cc
+++ b/chrome/browser/ui/webui/downloads_dom_handler.cc
@@ -314,7 +314,7 @@ void DownloadsDOMHandler::HandleCancel(const ListValue* args) {
   CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_CANCEL);
   DownloadItem* file = GetDownloadByValue(args);
   if (file)
-    file->Cancel(true);
+    file->Cancel();
 }
 
 void DownloadsDOMHandler::HandleClearAll(const ListValue* args) {
diff --git a/chrome/test/data/download-dangerous.jar b/chrome/test/data/download-dangerous.jar
new file mode 100644
index 0000000..28f58bc
--- /dev/null
+++ b/chrome/test/data/download-dangerous.jar
@@ -0,0 +1,2 @@
+Contents of file not important; will be marked as dangerous based on
+content-type and extension.
diff --git a/chrome/test/data/download-dangerous.jar.mock-http-headers b/chrome/test/data/download-dangerous.jar.mock-http-headers
new file mode 100644
index 0000000..e675398
--- /dev/null
+++ b/chrome/test/data/download-dangerous.jar.mock-http-headers
@@ -0,0 +1,6 @@
+HTTP/1.1 200 OK
+Content-Type: application/x-jar
+Content-Disposition: attachment; filename="download-dangerous.jar"
+Content-Length: 97
+Date: Mon, 13 Nov 2006 21:38:09 GMT
+Expires: Tue, 14 Nov 2006 19:23:58 GMT
diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc
index d37cd51..f319af6 100644
--- a/content/browser/download/download_item.cc
+++ b/content/browser/download/download_item.cc
@@ -353,24 +353,27 @@ void DownloadItem::Update(int64 bytes_so_far) {
   UpdateObservers();
 }
 
-// Triggered by a user action.
-void DownloadItem::Cancel(bool update_history) {
+void DownloadItem::Cancel() {
   // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
   VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
-  if (!IsPartialDownload()) {
-    // Small downloads might be complete before this method has
-    // a chance to run.
+
+  // Small downloads might be complete before we have a chance to run.
+  if (!IsInProgress())
     return;
-  }
+
+  TransitionTo(CANCELLED);
 
   download_stats::RecordDownloadCount(download_stats::CANCELLED_COUNT);
 
-  TransitionTo(CANCELLED);
-  StopProgressTimer();
-  if (update_history)
-    download_manager_->DownloadCancelledInternal(this);
+  // History insertion is the point at which we have finalized download
+  // details and persist them if something goes wrong.  Before history
+  // insertion, interrupt or cancel results in download removal.
+  if (db_handle() == DownloadItem::kUninitializedHandle) {
+    download_manager_->RemoveDownload(this);
+    // We are now deleted; no further code should be executed on this
+    // object.
+  }
 }
 
 void DownloadItem::MarkAsComplete() {
@@ -429,10 +432,22 @@ void DownloadItem::Completed() {
 }
 
 void DownloadItem::TransitionTo(DownloadState new_state) {
-  if (state_ == new_state)
+  DownloadState old_state = state_;
+  if (old_state == new_state)
     return;
 
+  // Check for disallowed state transitions.
+  CHECK(!(old_state == IN_PROGRESS && new_state == REMOVING));
+
   state_ = new_state;
+
+  // Do special operations for transitions from an active state.
+  if (old_state == IN_PROGRESS &&
+      (new_state == CANCELLED || new_state == INTERRUPTED)) {
+    download_manager_->DownloadStopped(this);
+    StopProgressTimer();
+  }
+
   UpdateObservers();
 }
 
@@ -454,20 +469,32 @@ void DownloadItem::UpdateTarget() {
     state_info_.target_name = full_path_.BaseName();
 }
 
-void DownloadItem::Interrupted(int64 size, net::Error net_error) {
+void DownloadItem::Interrupt(int64 size, net::Error net_error) {
   // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
 
+  // Small downloads might be complete before we have a chance to run.
   if (!IsInProgress())
     return;
 
-  last_error_ = net_error;
   UpdateSize(size);
-  StopProgressTimer();
+  last_error_ = net_error;
+
+  TransitionTo(INTERRUPTED);
+
   download_stats::RecordDownloadInterrupted(net_error,
                                             received_bytes_,
                                             total_bytes_);
-  TransitionTo(INTERRUPTED);
+
+  // History insertion is the point at which we have finalized download
+  // details and persist them if something goes wrong.  Before history
+  // insertion, interrupt or cancel results in download removal.
+  if (db_handle() == DownloadItem::kUninitializedHandle) {
+    download_manager_->RemoveDownload(this);
+    // We are now deleted; no further code should be executed on this
+    // object.
+  }
 }
 
 void DownloadItem::Delete(DeleteReason reason) {
@@ -498,11 +525,14 @@ void DownloadItem::Remove() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   download_manager_->AssertQueueStateConsistent(this);
-  Cancel(true);
+  if (IsInProgress()) {
+    TransitionTo(CANCELLED);
+    download_stats::RecordDownloadCount(download_stats::CANCELLED_COUNT);
+  }
   download_manager_->AssertQueueStateConsistent(this);
+  download_stats::RecordDownloadCount(download_stats::REMOVED_COUNT);
 
-  TransitionTo(REMOVING);
-  download_manager_->RemoveDownload(db_handle_);
+  download_manager_->RemoveDownload(this);
   // We have now been deleted.
 }
 
diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h
index 07c43fe..7a81aa0 100644
--- a/content/browser/download/download_item.h
+++ b/content/browser/download/download_item.h
@@ -157,16 +157,11 @@ class CONTENT_EXPORT DownloadItem {
   // Received a new chunk of data
   void Update(int64 bytes_so_far);
 
-  // Cancel the download operation. We need to distinguish between cancels at
-  // exit (DownloadManager destructor) from user interface initiated cancels
-  // because at exit, the history system may not exist, and any updates to it
-  // require AddRef'ing the DownloadManager in the destructor which results in
-  // a DCHECK failure. Set 'update_history' to false when canceling from at
-  // exit to prevent this crash. This may result in a difference between the
-  // downloaded file's size on disk, and what the history system's last record
-  // of it is. At worst, we'll end up re-downloading a small portion of the file
-  // when resuming a download (assuming the server supports byte ranges).
-  void Cancel(bool update_history);
+  // Cancel the download operation.  This may be called at any time; if
+  // it is called before all download attributes have been finalized and
+  // the download entered into the history, it will remove the download from
+  // the system.
+  void Cancel();
 
   // Called by external code (SavePackage) using the DownloadItem interface
   // to display progress when the DownloadItem should be considered complete.
@@ -182,10 +177,14 @@ class CONTENT_EXPORT DownloadItem {
   // Called when the downloaded file is removed.
   void OnDownloadedFileRemoved();
 
-  // Download operation had an error.
-  // |size| is the amount of data received at interruption.
-  // |error| is the network error code that the operation received.
-  void Interrupted(int64 size, net::Error error);
+  // Download operation had an error; call to interrupt the processing.
+  // Note that if the download attributes haven't yet been finalized and
+  // the download entered into the history (which implies that it hasn't
+  // yet been made visible in the UI), this call will remove the
+  // download from the system.
+  // |size| is the amount of data received so far, and |net_error| is the
+  // error code that the operation received.
+  void Interrupt(int64 size, net::Error net_error);
 
   // Deletes the file from disk and removes the download from the views and
   // history.  |user| should be true if this is the result of the user clicking
@@ -357,7 +356,8 @@ class CONTENT_EXPORT DownloadItem {
   void StartProgressTimer();
   void StopProgressTimer();
 
-  // Call to transition state; all state transitions should go through this.
+  // Does the actual work of transition state; all state
+  // transitions should go through this.
   void TransitionTo(DownloadState new_state);
 
   // Called when safety_state_ should be recomputed from is_dangerous_file
diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc
index f1c52d4..55ba657 100644
--- a/content/browser/download/download_manager.cc
+++ b/content/browser/download/download_manager.cc
@@ -112,8 +112,7 @@ void DownloadManager::Shutdown() {
       // from all queues.
       download->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
     } else if (download->IsPartialDownload()) {
-      download->Cancel(false);
-      delegate_->UpdateItemInPersistentStore(download);
+      download->Cancel();
     }
   }
 
@@ -275,7 +274,7 @@ void DownloadManager::RestartDownload(
     TabContents* contents = request_handle.GetTabContents();
 
     // |id_ptr| will be deleted in either FileSelected() or
-    // FileSelectionCancelled().
+    // FileSelectionCanceled().
     int32* id_ptr = new int32;
     *id_ptr = download_id;
 
@@ -505,23 +504,19 @@ void DownloadManager::OnDownloadRenamedToFinalName(int download_id,
   delegate_->UpdatePathForItemInPersistentStore(item, full_path);
 }
 
-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) {
+void DownloadManager::DownloadStopped(DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(ContainsKey(active_downloads_, download->id()));
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
-  RemoveFromActiveList(download);
+  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);
+
   // This function is called from the DownloadItem, so DI state
   // should already have been updated.
   AssertQueueStateConsistent(download);
@@ -543,9 +538,7 @@ void DownloadManager::OnDownloadError(int32 download_id,
            << " size = " << size
            << " download = " << download->DebugString(true);
 
-  RemoveFromActiveList(download);
-  download->Interrupted(size, error);
-  download->OffThreadCancel(file_manager_);
+  download->Interrupt(size, error);
 }
 
 DownloadItem* DownloadManager::GetActiveDownload(int32 download_id) {
@@ -562,20 +555,6 @@ DownloadItem* DownloadManager::GetActiveDownload(int32 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();
 }
@@ -605,14 +584,9 @@ int DownloadManager::RemoveDownloadItems(
   return num_deleted;
 }
 
-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);
+void DownloadManager::RemoveDownload(DownloadItem* download) {
+  // Make history update.  Ignores if db_handle isn't in history.
+  delegate_->RemoveItemFromPersistentStore(download->db_handle());
 
   // Remove from our tables and delete.
   int downloads_count = RemoveDownloadItems(DownloadVector(1, download));
@@ -762,12 +736,7 @@ void DownloadManager::FileSelectionCanceled(void* params) {
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
-  // 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_);
+  download->Cancel();
 }
 
 // Operations posted to us from the history service ----------------------------
@@ -838,8 +807,22 @@ void DownloadManager::OnDownloadItemAddedToPersistentStore(int32 download_id,
                                                            int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
+  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);
     return;
+  }
 
   VLOG(20) << __FUNCTION__ << "()" << " db_handle = " << db_handle
            << " download_id = " << download_id
@@ -855,27 +838,10 @@ void DownloadManager::OnDownloadItemAddedToPersistentStore(int32 download_id,
 
   CHECK(!ContainsKey(history_downloads_, db_handle));
 
+  CHECK(download->IsInProgress());
   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();
-  }
+  MaybeCompleteDownload(download);
 }
 
 void DownloadManager::ShowDownloadInBrowser(DownloadItem* download) {
@@ -914,8 +880,20 @@ DownloadItem* DownloadManager::GetDownloadItem(int download_id) {
   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) {
-  DCHECK(ContainsKey(active_downloads_, download_id));
+  if (!ContainsKey(active_downloads_, download_id))
+    return NULL;
+
   DownloadItem* download = active_downloads_[download_id];
   DCHECK(download != NULL);
   return download;
diff --git a/content/browser/download/download_manager.h b/content/browser/download/download_manager.h
index 06803ce..dcf4ab2 100644
--- a/content/browser/download/download_manager.h
+++ b/content/browser/download/download_manager.h
@@ -122,22 +122,24 @@ class CONTENT_EXPORT DownloadManager
   void OnResponseCompleted(int32 download_id, int64 size,
                            const std::string& hash);
 
-  // Offthread target for cancelling a particular download.  Will be a no-op
-  // if the download has already been cancelled.
-  void CancelDownload(int32 download_id);
-
   // Called when there is an error in the download.
   // |download_id| is the ID of the download.
   // |size| is the number of bytes that are currently downloaded.
   // |error| is a download error code.  Indicates what caused the interruption.
   void OnDownloadError(int32 download_id, int64 size, net::Error error);
 
-  // Called from DownloadItem to handle the DownloadManager portion of a
-  // Cancel; should not be called from other locations.
-  void DownloadCancelledInternal(DownloadItem* download);
+  // This routine is called from the DownloadItem when a
+  // request is cancelled or interrupted.  It removes the download
+  // from all internal queues holding in-progress work, and takes care
+  // of the off-thread aspects of the cancel (stopping the request,
+  // cancelling the download on the file thread).
+  void DownloadStopped(DownloadItem* download);
 
-  // Called from a view when a user clicks a UI button or link.
-  void RemoveDownload(int64 download_handle);
+  // Called from DownloadItem when the download is being removed.
+  // Takes care of all history operations, modifying internal queues,
+  // and notifying DownloadManager observers, and actually deletes
+  // the DownloadItem.
+  void RemoveDownload(DownloadItem* download);
 
   // Determine if the download is ready for completion, i.e. has had
   // all data saved, and completed the filename determination and
@@ -264,7 +266,7 @@ class CONTENT_EXPORT DownloadManager
   void SavePageDownloadFinished(DownloadItem* download);
 
   // Get the download item from the active map.  Useful when the item is not
-  // yet in the history map.
+  // yet in the history map.  Returns NULL if no such active download.
   DownloadItem* GetActiveDownloadItem(int id);
 
   DownloadManagerDelegate* delegate() const { return delegate_; }
@@ -273,16 +275,16 @@ class CONTENT_EXPORT DownloadManager
   typedef std::set<DownloadItem*> DownloadSet;
   typedef base::hash_map<int64, DownloadItem*> DownloadMap;
 
+  friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
+  friend class DeleteTask<DownloadManager>;
+  friend class base::RefCountedThreadSafe<DownloadManager,
+                                          BrowserThread::DeleteOnUIThread>;
+
   // For testing.
   friend class DownloadManagerTest;
   friend class MockDownloadManager;
   friend class DownloadTest;
 
-  friend class base::RefCountedThreadSafe<DownloadManager,
-                                          BrowserThread::DeleteOnUIThread>;
-  friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
-  friend class DeleteTask<DownloadManager>;
-
   void set_delegate(DownloadManagerDelegate* delegate) { delegate_ = delegate; }
 
   virtual ~DownloadManager();
@@ -305,17 +307,18 @@ class CONTENT_EXPORT DownloadManager
   // Returns NULL if the download is not active.
   DownloadItem* GetActiveDownload(int32 download_id);
 
-  // Removes |download| from the active and in progress maps.
-  // Called when the download is cancelled or has an error.
-  // Does nothing if the download is not in the history DB.
-  void RemoveFromActiveList(DownloadItem* download);
-
   // Updates the delegate about the overall download progress.
   void UpdateDownloadProgress();
 
   // Inform observers that the model has changed.
   void NotifyModelChanged();
 
+  // Return all in progress downloads.  This includes downloads that
+  // have not yet been entered into the history (all other accessors
+  // only return downloads that have been entered into the history).
+  // This is intended to be used for testing only.
+  void GetInProgressDownloads(std::vector<DownloadItem*>* result);
+
   // Debugging routine to confirm relationship between below
   // containers; no-op if NDEBUG.
   void AssertContainersConsistent() const;
diff --git a/content/browser/download/download_manager_delegate.h b/content/browser/download/download_manager_delegate.h
index 55d25f6..be72f11 100644
--- a/content/browser/download/download_manager_delegate.h
+++ b/content/browser/download/download_manager_delegate.h
@@ -84,7 +84,7 @@ class DownloadManagerDelegate {
 
   // Notifies the delegate that it should remove the download item from its
   // persistent store.
-  virtual void RemoveItemFromPersistentStore(DownloadItem* item) = 0;
+  virtual void RemoveItemFromPersistentStore(int64 db_handle) = 0;
 
   // Notifies the delegate to remove downloads from the given time range.
   virtual void RemoveItemsFromPersistentStoreBetween(
diff --git a/content/browser/download/download_request_handle.h b/content/browser/download/download_request_handle.h
index c65ebe6..aba8dc1 100644
--- a/content/browser/download/download_request_handle.h
+++ b/content/browser/download/download_request_handle.h
@@ -37,11 +37,15 @@ class DownloadRequestHandle {
   TabContents* GetTabContents() const;
   DownloadManager* GetDownloadManager() const;
 
-  // Pause or resume the matching URL request.
+  // Pause or resume the matching URL request.  Note that while these
+  // do not modify the DownloadRequestHandle, they do modify the
+  // request the handle refers to.
   void PauseRequest() const;
   void ResumeRequest() const;
 
-  // Cancel the request
+  // Cancel the request.  Note that while this does not
+  // modify the DownloadRequestHandle, it does modify the
+  // request the handle refers to.
   void CancelRequest() const;
 
   std::string DebugString() const;
diff --git a/content/browser/download/download_stats.h b/content/browser/download/download_stats.h
index 7ca7a23..d80cba2 100644
--- a/content/browser/download/download_stats.h
+++ b/content/browser/download/download_stats.h
@@ -64,6 +64,9 @@ enum DownloadCountTypes {
   // Counts iterations of the BaseFile::AppendDataToFile() loop.
   WRITE_LOOP_COUNT,
 
+  // Downloads that were removed by the user.
+  REMOVED_COUNT,
+
   DOWNLOAD_COUNT_TYPES_LAST_ENTRY
 };
 
diff --git a/content/browser/download/mock_download_manager_delegate.cc b/content/browser/download/mock_download_manager_delegate.cc
index e5b8b66..dba6b8c 100644
--- a/content/browser/download/mock_download_manager_delegate.cc
+++ b/content/browser/download/mock_download_manager_delegate.cc
@@ -65,7 +65,7 @@ void MockDownloadManagerDelegate::UpdatePathForItemInPersistentStore(
 }
 
 void MockDownloadManagerDelegate::RemoveItemFromPersistentStore(
-    DownloadItem* item) {
+    int64 db_handle) {
 }
 
 void MockDownloadManagerDelegate::RemoveItemsFromPersistentStoreBetween(
diff --git a/content/browser/download/mock_download_manager_delegate.h b/content/browser/download/mock_download_manager_delegate.h
index fc3f8f2..453f3449 100644
--- a/content/browser/download/mock_download_manager_delegate.h
+++ b/content/browser/download/mock_download_manager_delegate.h
@@ -31,7 +31,7 @@ class MockDownloadManagerDelegate : public DownloadManagerDelegate {
   virtual void UpdatePathForItemInPersistentStore(
       DownloadItem* item,
       const FilePath& new_path) OVERRIDE;
-  virtual void RemoveItemFromPersistentStore(DownloadItem* item) OVERRIDE;
+  virtual void RemoveItemFromPersistentStore(int64 db_handle) OVERRIDE;
   virtual void RemoveItemsFromPersistentStoreBetween(
       const base::Time remove_begin,
       const base::Time remove_end) OVERRIDE;
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index 1560dd0..f0c2be1 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -626,7 +626,7 @@ void SavePackage::Stop() {
 
   // Inform the DownloadItem we have canceled whole save page job.
   if (download_) {
-    download_->Cancel(false);
+    download_->Cancel();
     FinalizeDownloadEntry();
   }
 }
diff --git a/content/browser/net/url_request_slow_download_job.cc b/content/browser/net/url_request_slow_download_job.cc
index ef1370f..4cb152c 100644
--- a/content/browser/net/url_request_slow_download_job.cc
+++ b/content/browser/net/url_request_slow_download_job.cc
@@ -10,9 +10,11 @@
 #include "base/string_util.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_status.h"
 
 const int kFirstDownloadSize = 1024 * 35;
 const int kSecondDownloadSize = 1024 * 10;
@@ -23,9 +25,18 @@ const char URLRequestSlowDownloadJob::kKnownSizeUrl[] =
   "http://url.handled.by.slow.download/download-known-size";
 const char URLRequestSlowDownloadJob::kFinishDownloadUrl[] =
   "http://url.handled.by.slow.download/download-finish";
+const char URLRequestSlowDownloadJob::kErrorFinishDownloadUrl[] =
+  "http://url.handled.by.slow.download/download-error";
 
 std::vector<URLRequestSlowDownloadJob*>
-    URLRequestSlowDownloadJob::kPendingRequests;
+    URLRequestSlowDownloadJob::pending_requests_;
+
+// Return whether this is the finish or error URL.
+static bool IsCompletionUrl(const GURL& url) {
+  if (url.spec() == URLRequestSlowDownloadJob::kFinishDownloadUrl)
+    return true;
+  return (url.spec() == URLRequestSlowDownloadJob::kErrorFinishDownloadUrl);
+}
 
 void URLRequestSlowDownloadJob::Start() {
   MessageLoop::current()->PostTask(
@@ -43,6 +54,8 @@ void URLRequestSlowDownloadJob::AddUrlHandler() {
                         &URLRequestSlowDownloadJob::Factory);
   filter->AddUrlHandler(GURL(kFinishDownloadUrl),
                         &URLRequestSlowDownloadJob::Factory);
+  filter->AddUrlHandler(GURL(kErrorFinishDownloadUrl),
+                        &URLRequestSlowDownloadJob::Factory);
 }
 
 /*static */
@@ -50,19 +63,23 @@ net::URLRequestJob* URLRequestSlowDownloadJob::Factory(
     net::URLRequest* request,
     const std::string& scheme) {
   URLRequestSlowDownloadJob* job = new URLRequestSlowDownloadJob(request);
-  if (request->url().spec() != kFinishDownloadUrl)
-    URLRequestSlowDownloadJob::kPendingRequests.push_back(job);
+  if (!IsCompletionUrl(request->url()))
+    URLRequestSlowDownloadJob::pending_requests_.push_back(job);
   return job;
 }
 
 /* static */
-void URLRequestSlowDownloadJob::FinishPendingRequests() {
+void URLRequestSlowDownloadJob::FinishPendingRequests(bool error) {
   typedef std::vector<URLRequestSlowDownloadJob*> JobList;
-  for (JobList::iterator it = kPendingRequests.begin(); it !=
-       kPendingRequests.end(); ++it) {
-    (*it)->set_should_finish_download();
+  for (JobList::iterator it = pending_requests_.begin(); it !=
+       pending_requests_.end(); ++it) {
+    if (error) {
+      (*it)->set_should_error_download();
+    } else {
+      (*it)->set_should_finish_download();
+    }
   }
-  kPendingRequests.clear();
+  pending_requests_.clear();
 }
 
 URLRequestSlowDownloadJob::URLRequestSlowDownloadJob(net::URLRequest* request)
@@ -70,19 +87,21 @@ URLRequestSlowDownloadJob::URLRequestSlowDownloadJob(net::URLRequest* request)
       first_download_size_remaining_(kFirstDownloadSize),
       should_finish_download_(false),
       should_send_second_chunk_(false),
+      should_error_download_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {}
 
 void URLRequestSlowDownloadJob::StartAsync() {
-  if (LowerCaseEqualsASCII(kFinishDownloadUrl, request_->url().spec().c_str()))
-    URLRequestSlowDownloadJob::FinishPendingRequests();
+  if (IsCompletionUrl(request_->url())) {
+    URLRequestSlowDownloadJob::FinishPendingRequests(
+        request_->url().spec() == kErrorFinishDownloadUrl);
+  }
 
   NotifyHeadersComplete();
 }
 
 bool URLRequestSlowDownloadJob::ReadRawData(net::IOBuffer* buf, int buf_size,
                                             int *bytes_read) {
-  if (LowerCaseEqualsASCII(kFinishDownloadUrl,
-                           request_->url().spec().c_str())) {
+  if (IsCompletionUrl(request_->url())) {
     *bytes_read = 0;
     return true;
   }
@@ -132,6 +151,9 @@ void URLRequestSlowDownloadJob::CheckDoneStatus() {
     should_send_second_chunk_ = true;
     SetStatus(net::URLRequestStatus());
     NotifyReadComplete(kSecondDownloadSize);
+  } else if (should_error_download_) {
+    NotifyDone(
+        net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED));
   } else {
     MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
@@ -154,8 +176,7 @@ void URLRequestSlowDownloadJob::GetResponseInfoConst(
     net::HttpResponseInfo* info) const {
   // Send back mock headers.
   std::string raw_headers;
-  if (LowerCaseEqualsASCII(kFinishDownloadUrl,
-                           request_->url().spec().c_str())) {
+  if (IsCompletionUrl(request_->url())) {
     raw_headers.append(
       "HTTP/1.1 200 OK\n"
       "Content-type: text/plain\n");
@@ -165,7 +186,7 @@ void URLRequestSlowDownloadJob::GetResponseInfoConst(
       "Content-type: application/octet-stream\n"
       "Cache-Control: max-age=0\n");
 
-    if (LowerCaseEqualsASCII(kKnownSizeUrl, request_->url().spec().c_str())) {
+    if (request_->url().spec() == kKnownSizeUrl) {
       raw_headers.append(base::StringPrintf(
           "Content-Length: %d\n",
           kFirstDownloadSize + kSecondDownloadSize));
diff --git a/content/browser/net/url_request_slow_download_job.h b/content/browser/net/url_request_slow_download_job.h
index 9cbde81..66eff9a6 100644
--- a/content/browser/net/url_request_slow_download_job.h
+++ b/content/browser/net/url_request_slow_download_job.h
@@ -1,9 +1,13 @@
 // 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.
-// This class simulates a slow download.  This used in a UI test to test the
-// download manager.  Requests to |kUnknownSizeUrl| and |kKnownSizeUrl| start
-// downloads that pause after the first
+// This class simulates a slow download.  This is used in browser tests
+// to test the download manager.  Requests to |kUnknownSizeUrl| and
+// |kKnownSizeUrl| start downloads that pause after the first chunk of
+// data is delivered.  A later request to |kFinishDownloadUrl| will finish
+// these downloads.  A later request to |kErrorFinishDownloadUrl| will
+// cause these downloads to error with |net::ERR_FAILED|.
+// TODO(rdsmith): Update to allow control of returned error.
 
 #ifndef CONTENT_BROWSER_NET_URL_REQUEST_SLOW_DOWNLOAD_JOB_H_
 #define CONTENT_BROWSER_NET_URL_REQUEST_SLOW_DOWNLOAD_JOB_H_
@@ -37,6 +41,7 @@ class URLRequestSlowDownloadJob : public net::URLRequestJob {
   static const char kUnknownSizeUrl[];
   static const char kKnownSizeUrl[];
   static const char kFinishDownloadUrl[];
+  static const char kErrorFinishDownloadUrl[];
 
   // Adds the testing URLs to the net::URLRequestFilter.
   CONTENT_EXPORT static void AddUrlHandler();
@@ -47,17 +52,19 @@ class URLRequestSlowDownloadJob : public net::URLRequestJob {
   void GetResponseInfoConst(net::HttpResponseInfo* info) const;
 
   // Mark all pending requests to be finished.  We keep track of pending
-  // requests in |kPendingRequests|.
-  static void FinishPendingRequests();
-  static std::vector<URLRequestSlowDownloadJob*> kPendingRequests;
+  // requests in |pending_requests_|.
+  static void FinishPendingRequests(bool error);
+  static std::vector<URLRequestSlowDownloadJob*> pending_requests_;
 
   void StartAsync();
 
   void set_should_finish_download() { should_finish_download_ = true; }
+  void set_should_error_download() { should_error_download_ = true; }
 
   int first_download_size_remaining_;
   bool should_finish_download_;
   bool should_send_second_chunk_;
+  bool should_error_download_;
 
   ScopedRunnableMethodFactory<URLRequestSlowDownloadJob> method_factory_;
 };