Modified cancel and interrupt processing to avoid race with history.

chrome/browser/download/download_item.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.
 //
 // Each download is represented by a DownloadItem, and all DownloadItems
 // are owned by the DownloadManager which maintains a global list of all
 // downloads. DownloadItems are created when a user initiates a download,
 // and exist for the duration of the browser life time.
 //
 // Download observers:
@@ skipping 145 matching lines @@
 
   // Show the download via the OS shell.
   void ShowDownloadInShell();
 
   // Called when the user has validated the download of a dangerous file.
   void DangerousDownloadValidated();
 
   // 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 the download has been persisted into the history
+  // it will remove the download completely from the system.

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

Change comment to indicate both behaviors (before and after persistence) and
phrase the difference as "before all download attributes have been finalized").

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

Changed phrasing, left assymetric specification.

+  void Cancel();
 
   // Called by external code (SavePackage) using the DownloadItem interface
   // to display progress when the DownloadItem should be considered complete.
   void MarkAsComplete();
 
   // Called when all data has been saved. Only has display effects.
   void OnAllDataSaved(int64 size);
 
   // Called when the downloaded file is removed.
   void OnDownloadedFileRemoved();
 
-  // Download operation had an error.
+  // Download operation had an error; call to interrupt the processing.
+  // Note that if the download hasn't yet been persisted to the history
+  // (which implies that it hasn't yet been shown to the user in the UI),
+  // it will be completely removed.

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

Make this comment align with the Cancel comment.

Long term, we may want to go through all the hoops for a download before we
accept a network error, so that interrupted downloads can always be resumed. 
I'm not sure what this means for dealing with file errors while trying to
determine the file name.

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

Done.

   // |size| is the amount of data received so far, and |os_error| is the error
   // code that the operation received.
-  void Interrupted(int64 size, int os_error);
+  void Interrupt(int64 size, int os_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
   // the discard button, and false if it is being deleted for other reasons like
   // browser shutdown.
   void Delete(DeleteReason reason);
 
   // Removes the download from the views and history.
   void Remove();
 
@@ skipping 145 matching lines @@
 #endif
 
  private:
   // Construction common to all constructors. |active| should be true for new
   // downloads and false for downloads from the history.
   void Init(bool active);
 
   // Internal helper for maintaining consistent received and total sizes.
   void UpdateSize(int64 size);
 
+  // Internal function to do the main work of cancelling or
+  // interrupting a download.
+  void StopInternal(DownloadState target_state);
+
   // Called when the entire download operation (including renaming etc)
   // is completed.
   void Completed();
 
   // Start/stop sending periodic updates to our observers
   void StartProgressTimer();
   void StopProgressTimer();
 
   // Call to install this item as a CRX. Should only be called on
   // items which are CRXes. Use is_extension_install() to check.
@@ skipping 105 matching lines @@
   // only.
   bool open_enabled_;
 
   // DownloadItem observes CRX installs it initiates.
   NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_H_