Download resumption - Preliminary

content/browser/download/download_item_impl.h

Index: content/browser/download/download_item_impl.h
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h
index 72d8c266951df272b96117d9f9e745eb73b34f24..4ddbc136121105c9103d987139b62e3bc78653c4 100644
--- a/content/browser/download/download_item_impl.h
+++ b/content/browser/download/download_item_impl.h
@@ -28,6 +28,9 @@ class DownloadItemImplDelegate;
 // See download_item.h for usage.
 class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
  public:
+  // The maximum number of attempts we will make to resume automatically.
+  static const int kMaxAutoResumeAttempts;
+
   // Note that it is the responsibility of the caller to ensure that a
   // DownloadItemImplDelegate passed to a DownloadItemImpl constructor
   // outlives the DownloadItemImpl.
@@ -43,7 +46,6 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   // |bound_net_log| is constructed externally for our use.
   DownloadItemImpl(DownloadItemImplDelegate* delegate,
                    const DownloadCreateInfo& info,
-                   scoped_ptr<DownloadRequestHandleInterface> request_handle,

[Randy Smith (Not in Mondays), 2012/10/15 00:36:08]

Why?  (This question applies to DownloadItemFactory::CreateActiveItem as well). 
It looks as if Resume() takes a request_handle, so I don't see the reason to
remove this from the constructor (and thus require the addition of
SetRequest()).  

One possible design would be to create the DownloadItemImpl in (effectively) an
already cancelled/interrupted state, and use the same accessor on it (presumably
Start() or something like it) to start it, passing in a request_handle.  I'm
open to such designs, but they're a little tricky, as the set of state
transitions that the DII would go through would look weird to an outsider
observer (CANCELLED -> IN_PROGRESS).  That might be soluble by an
internal/external state dichotomy (I recently split the DII internal state away
from the external state, so this wouldn't be hard to do), but I'm not sure it's
worth picking up that particular can of worms right now.  So I'd be more
inclined to have the creation pathway be as it currently is, and have the
resumption pathway pass in the request handle.

                    const net::BoundNetLog& bound_net_log);
 
   // Constructing for the "Save Page As..." feature:
@@ -74,6 +76,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual DownloadState GetState() const OVERRIDE;
   virtual content::DownloadInterruptReason GetLastReason() const OVERRIDE;
   virtual bool IsPaused() const OVERRIDE;
+  virtual ResumeMode CanResumeInterrupted() const OVERRIDE;
   virtual bool IsTemporary() const OVERRIDE;
   virtual bool IsPersisted() const OVERRIDE;
   virtual bool IsPartialDownload() const OVERRIDE;
@@ -117,6 +120,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual base::Time GetEndTime() const OVERRIDE;
   virtual bool CanShowInFolder() OVERRIDE;
   virtual bool CanOpenDownload() OVERRIDE;
+  virtual bool CanResumeDownload() const OVERRIDE;
   virtual bool ShouldOpenFileBasedOnExtension() OVERRIDE;
   virtual bool GetOpenWhenComplete() const OVERRIDE;
   virtual bool GetAutoOpened() OVERRIDE;
@@ -134,6 +138,10 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual void SetDisplayName(const FilePath& name) OVERRIDE;
   virtual std::string DebugString(bool verbose) const OVERRIDE;
   virtual void MockDownloadOpenForTesting() OVERRIDE;
+  virtual const net::BoundNetLog& GetBoundNetLog() const OVERRIDE;
+
+  void SetRequest(scoped_ptr<DownloadRequestHandleInterface> request_handle);
+  bool IsResuming() const;

[Randy Smith (Not in Mondays), 2012/10/15 00:36:08]

This (including the underlying state variable) doesn't appear to be used
anywhere; can we delete it?

 
   // All remaining public interfaces virtual to allow for DownloadItemImpl
   // mocks.
@@ -179,9 +187,16 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   // rewrites of the DownloadManager queues.
   virtual void OffThreadCancel();
 
+  // Completes the off-thread aspects of the download interruption.
+  virtual void OffThreadInterrupt();
+
   // Indicate that an error has occurred on the download.
   virtual void Interrupt(content::DownloadInterruptReason reason);
 
+  // User has resumed a download after an interruption.
+  // |req_handle| is the new request handle associated with the download.
+  virtual void Resume(scoped_ptr<DownloadRequestHandleInterface> req_handle);
+
   // DownloadItemImpl routines only needed by SavePackage ----------------------
 
   // Called by SavePackage to set the total number of bytes on the item.
@@ -286,7 +301,8 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
 
   void SetFullPath(const FilePath& new_path);
 
-  // Mapping between internal and external states.
+  void AutoResumeIfValid();
+
   static DownloadState InternalToExternalState(
       DownloadInternalState internal_state);
   static DownloadInternalState ExternalToInternalState(
@@ -294,6 +310,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
 
   // Debugging routines --------------------------------------------------------
   static const char* DebugDownloadStateString(DownloadInternalState state);
+  static const char* DebugResumeModeString(DownloadItem::ResumeMode mode);
 
   // The handle to the request information.  Used for operations outside the
   // download system.
@@ -415,6 +432,12 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   // In progress downloads may be paused by the user, we note it here.
   bool is_paused_;
 
+  // A download has been resumed.
+  bool is_resuming_;
+
+  // The number of times this download has been resumed automatically.
+  int auto_resume_count_;
+
   // A flag for indicating if the download should be opened at completion.
   bool open_when_complete_;
 
@@ -430,6 +453,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   // before the observer is added.
   bool auto_opened_;
 
+  // The download has been added to the history DB.
   bool is_persisted_;
 
   // True if the item was downloaded temporarily.