Modified cancel and interrupt processing to avoid race with history.

chrome/browser/download/download_manager.h

 // 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.
 //
 // The DownloadManager object manages the process of downloading, including
 // updates to the history system and providing the information for displaying
 // the downloads view in the Destinations tab. There is one DownloadManager per
 // active profile in Chrome.
 //
 // Download observers:
@@ skipping 104 matching lines @@
   bool Init(Profile* profile);
 
   // Notifications sent from the download thread to the UI thread
   void StartDownload(int32 id);
   void UpdateDownload(int32 download_id, int64 size);
   // |hash| is sha256 hash for the downloaded file. It is empty when the hash
   // is not available.
   void OnResponseCompleted(int32 download_id, int64 size, int os_error,
                            const std::string& hash);
 
-  // Called from a view when a user clicks a UI button or link.
-  void DownloadCancelled(int32 download_id);
-  void RemoveDownload(int64 download_handle);
+  // Called to initiate download cancel on behalf of an off-thread portion
+  // of the download system; redirects to DownloadItem.
+  void CancelDownload(int32 download_handle);
+
+  // This routine is called from the DownloadItem when a
+  // request is cancelled or interrupted.  It removes the download
+  // from all internal queues, and takes care of the off-thread aspects

[Randy Smith (Not in Mondays), 2011/06/30 23:05:13]

Not all internal queues, just the ones representing activity.

[Randy Smith (Not in Mondays), 2011/07/05 20:28:44]

Done.

+  // of the cancel (stopping the request, cancelling the download on
+  // the file thread).
+  void DownloadStopped(int32 download_id);
+
+  // Called from DownloadItem when the download is being removed.

[Randy Smith (Not in Mondays), 2011/06/30 23:05:13]

A bit more detail as to what it does.

[Randy Smith (Not in Mondays), 2011/07/05 20:28:44]

Done.

+  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
   // history insertion.
   bool IsDownloadReadyForCompletion(DownloadItem* download);
 
   // If all pre-requisites have been met, complete download processing, i.e.
   // do internal cleanup, file rename, and potentially auto-open.
   // (Dangerous downloads still may block on user acceptance after this
   // point.)
@@ skipping 121 matching lines @@
 
   // Assert the named download item is on the correct queues
   // in the DownloadManager.  For debugging.
   void AssertQueueStateConsistent(DownloadItem* download);
 
   // Get the download item from the history map.  Useful after the item has
   // been removed from the active map, or was retrieved from the history DB.
   DownloadItem* GetDownloadItem(int id);
 
  private:
-  // For testing.
-  friend class DownloadManagerTest;
-  friend class MockDownloadManager;
-
   // This class is used to let an incognito DownloadManager observe changes to
   // a normal DownloadManager, to propagate ModelChanged() calls from the parent
   // DownloadManager to the observers of the incognito DownloadManager.
   class OtherDownloadManagerObserver : public Observer {
    public:
     explicit OtherDownloadManagerObserver(
         DownloadManager* observing_download_manager);
     virtual ~OtherDownloadManagerObserver();
 
     // Observer interface.
     virtual void ModelChanged();
     virtual void ManagerGoingDown();
 
    private:
     // The incognito download manager.
     DownloadManager* observing_download_manager_;
 
     // The original profile's download manager.
     DownloadManager* observed_download_manager_;
   };
 
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class DeleteTask<DownloadManager>;
   friend class OtherDownloadManagerObserver;
 
+  // For testing.
+  friend class DownloadManagerTest;
+  friend class MockDownloadManager;
+  FRIEND_TEST_ALL_PREFIXES(DownloadTest, CancelFromFileSelection);
+  FRIEND_TEST_ALL_PREFIXES(DownloadTest, RemoveFromFileSelection);
+  FRIEND_TEST_ALL_PREFIXES(DownloadTest, InterruptFromFileSelection);
+
   virtual ~DownloadManager();
 
   // Called on the FILE thread to check the existence of a downloaded file.
   void CheckForFileRemovalOnFileThread(int64 db_handle, const FilePath& path);
 
   // Called on the UI thread if the FILE thread detects the removal of
   // the downloaded file. The UI thread updates the state of the file
   // and then notifies this update to the file's observer.
   void OnFileRemovalDetected(int64 db_handle);
 
   // Called on the download thread to check whether the suggested file path
   // exists.  We don't check if the file exists on the UI thread to avoid UI
   // stalls from interacting with the file system.
   void CheckIfSuggestedPathExists(int32 download_id,
                                   DownloadStateInfo state,
                                   const FilePath& default_path);
 
   // Called on the UI thread once the DownloadManager has determined whether the
   // suggested file path exists.
   void OnPathExistenceAvailable(int32 download_id,
                                 DownloadStateInfo new_state);
 
   // Called back after a target path for the file to be downloaded to has been
   // determined, either automatically based on the suggested file name, or by
   // the user in a Save As dialog box.
   void ContinueDownloadWithPath(DownloadItem* download,
                                 const FilePath& chosen_file);
 
-  // Download cancel helper function.
-  // |request_handle| is passed by value because it is ultimately passed to
-  // other threads, and this way we don't have to worry about object lifetimes.
-  void DownloadCancelledInternal(int download_id,
-                                 DownloadRequestHandle request_handle);
-
   // All data has been downloaded.
   // |hash| is sha256 hash for the downloaded file. It is empty when the hash
   // is not available.
   void OnAllDataSaved(int32 download_id, int64 size, const std::string& hash);
 
   // An error occurred in the download.
   void OnDownloadError(int32 download_id, int64 size, int os_error);
 
   // Updates the app icon about the overall download progress.
   void UpdateAppIcon();
 
   // 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);
+
   // Get the download item from the active map.  Useful when the item is not
   // yet in the history map.
   DownloadItem* GetActiveDownloadItem(int id);
 
   // Debugging routine to confirm relationship between below
   // containers; no-op if NDEBUG.
   void AssertContainersConsistent() const;
 
   // |downloads_| is the owning set for all downloads known to the
   // DownloadManager.  This includes downloads started by the user in
@@ skipping 68 matching lines @@
   // The "Save As" dialog box used to ask the user where a file should be
   // saved.
   scoped_refptr<SelectFileDialog> select_file_dialog_;
 
   scoped_ptr<OtherDownloadManagerObserver> other_download_manager_observer_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadManager);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_