Download resumption - Preliminary

content/browser/download/download_item_impl.cc

Index: content/browser/download/download_item_impl.cc
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index d0984add840f7819230ee1b52fbbf8977952da43..582aa8517a045ba4ad266134102b32793cea03db 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -46,6 +46,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/download_persistent_store_info.h"
+#include "content/public/browser/download_url_parameters.h"
 #include "net/base/net_util.h"
 
 using content::BrowserThread;
@@ -58,7 +59,7 @@ using content::WebContents;
 
 namespace {
 
-static void DeleteDownloadedFile(const FilePath& path) {
+void DeleteDownloadedFile(const FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   // Make sure we only delete files.
@@ -99,6 +100,8 @@ class NullDownloadRequestHandle : public DownloadRequestHandleInterface {
   virtual void PauseRequest() const OVERRIDE {}
   virtual void ResumeRequest() const OVERRIDE {}
   virtual void CancelRequest() const OVERRIDE {}
+  virtual void SetRequestId(int new_request_id) OVERRIDE {}
+  virtual int RequestId() const OVERRIDE { return -1; }
   virtual std::string DebugString() const OVERRIDE {
     return "Null DownloadRequestHandle";
   }
@@ -119,6 +122,9 @@ const char DownloadItem::kEmptyFileHash[] = "";
 
 }
 
+// The maximum number of attempts we will make to resume automatically.
+const int DownloadItemImpl::kMaxAutoResumeAttempts = 5;
+
 // Our download table ID starts at 1, so we use 0 to represent a download that
 // has started, but has not yet had its data persisted in the table. We use fake
 // database handles in incognito mode starting at -1 and progressively getting
@@ -149,6 +155,8 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
       db_handle_(info.db_handle),
       delegate_(delegate),
       is_paused_(false),
+      is_resuming_(false),
+      auto_resume_count_(0),
       open_when_complete_(false),
       file_externally_removed_(false),
       safety_state_(SAFE),
@@ -174,10 +182,8 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
 DownloadItemImpl::DownloadItemImpl(
     DownloadItemImplDelegate* delegate,
     const DownloadCreateInfo& info,
-    scoped_ptr<DownloadRequestHandleInterface> request_handle,
     const net::BoundNetLog& bound_net_log)
-    : request_handle_(request_handle.Pass()),
-      download_id_(info.download_id),
+    : download_id_(info.download_id),
       target_disposition_(
           (info.prompt_user_for_save_location) ?
               TARGET_DISPOSITION_PROMPT : TARGET_DISPOSITION_OVERWRITE),
@@ -203,6 +209,8 @@ DownloadItemImpl::DownloadItemImpl(
       db_handle_(DownloadItem::kUninitializedHandle),
       delegate_(delegate),
       is_paused_(false),
+      is_resuming_(false),
+      auto_resume_count_(0),
       open_when_complete_(false),
       file_externally_removed_(false),
       safety_state_(SAFE),
@@ -258,6 +266,8 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
       db_handle_(DownloadItem::kUninitializedHandle),
       delegate_(delegate),
       is_paused_(false),
+      is_resuming_(false),
+      auto_resume_count_(0),
       open_when_complete_(false),
       file_externally_removed_(false),
       safety_state_(SAFE),
@@ -321,35 +331,42 @@ void DownloadItemImpl::DangerousDownloadValidated() {
 
   UpdateObservers();
 
-  delegate_->MaybeCompleteDownload(this);
+  delegate_->MaybeCompleteDownload(download_id_.local());
 }
 
 void DownloadItemImpl::TogglePause() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DCHECK(state_ == IN_PROGRESS_INTERNAL || state_ == COMPLETING_INTERNAL);
-  if (is_paused_)
-    request_handle_->ResumeRequest();
-  else
-    request_handle_->PauseRequest();
-  is_paused_ = !is_paused_;
+  DCHECK(IsPartialDownload());
+  if (IsInProgress()) {
+    if (is_paused_)
+      request_handle_->ResumeRequest();
+    else
+      request_handle_->PauseRequest();
+    is_paused_ = !is_paused_;
+  } else if (IsInterrupted()) {
+    auto_resume_count_ = 0;  // User input resets the counter.
+    delegate_->RestartInterruptedDownload(
+        this, content::DownloadUrlParameters::OnStartedCallback());
+  }
+
   UpdateObservers();
 }
 
 void DownloadItemImpl::Cancel(bool user_cancel) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  last_reason_ = user_cancel ?
-      content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED :
-      content::DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN;
-
   VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
-  if (state_ != IN_PROGRESS_INTERNAL) {
+  if (state_ != IN_PROGRESS_INTERNAL && state_ != INTERRUPTED_INTERNAL) {
     // Small downloads might be complete before this method has
     // a chance to run.
     return;
   }
 
+  last_reason_ = user_cancel ?
+      content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED :
+      content::DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN;
+
   download_stats::RecordDownloadCount(download_stats::CANCELLED_COUNT);
 
   TransitionTo(CANCELLED_INTERNAL);
@@ -456,6 +473,87 @@ bool DownloadItemImpl::IsPaused() const {
   return is_paused_;
 }
 
+bool DownloadItemImpl::IsResuming() const {
+  return is_resuming_;
+}
+
+void DownloadItemImpl::SetRequest(
+    scoped_ptr<DownloadRequestHandleInterface> request_handle) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  request_handle_ = request_handle.Pass();
+}
+
+DownloadItem::ResumeMode DownloadItemImpl::CanResumeInterrupted() const {
+  if (!IsInterrupted() || !is_persisted_)
+    return RESUME_MODE_INVALID;
+
+  ResumeMode mode = RESUME_MODE_INVALID;
+
+  switch(last_reason_) {
+    case content::DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR:
+    case content::DOWNLOAD_INTERRUPT_REASON_NETWORK_TIMEOUT:
+      mode = RESUME_MODE_IMMEDIATE_CONTINUE;  // Continue immediately.
+      break;
+
+    case content::DOWNLOAD_INTERRUPT_REASON_SERVER_PRECONDITION:
+    case content::DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE:
+     mode = RESUME_MODE_IMMEDIATE_RESTART;  // Restart immediately.
+      break;
+
+    case content::DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED:
+    case content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED:
+    case content::DOWNLOAD_INTERRUPT_REASON_NETWORK_SERVER_DOWN:
+    case content::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED:
+    case content::DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN:
+    case content::DOWNLOAD_INTERRUPT_REASON_CRASH:
+      mode = RESUME_MODE_USER_CONTINUE;  // Continue via user input.
+      break;
+
+    case content::DOWNLOAD_INTERRUPT_REASON_FILE_FAILED:
+    case content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED:
+    case content::DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE:
+    case content::DOWNLOAD_INTERRUPT_REASON_FILE_NAME_TOO_LONG:
+    case content::DOWNLOAD_INTERRUPT_REASON_FILE_TOO_LARGE:
+      mode = RESUME_MODE_USER_RESTART;  // Restart via user input.
+      break;
+
+    case content::DOWNLOAD_INTERRUPT_REASON_NONE:
+    case content::DOWNLOAD_INTERRUPT_REASON_FILE_VIRUS_INFECTED:
+    case content::DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT:
+    case content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED:
+      break;
+  }
+
+  // Check if we have exhausted the number of automatic retry attempts.
+  if (auto_resume_count_ >= kMaxAutoResumeAttempts) {
+    if (mode == RESUME_MODE_IMMEDIATE_CONTINUE)
+      mode = RESUME_MODE_USER_CONTINUE;
+    else if (mode == RESUME_MODE_IMMEDIATE_RESTART)
+      mode = RESUME_MODE_USER_RESTART;
+  }
+
+  return mode;
+}
+
+void DownloadItemImpl::AutoResumeIfValid() {
+  ResumeMode mode = CanResumeInterrupted();
+
+  if ((mode == RESUME_MODE_IMMEDIATE_RESTART) ||
+      (mode == RESUME_MODE_IMMEDIATE_CONTINUE)) {
+    if (mode == RESUME_MODE_IMMEDIATE_RESTART) {
+      received_bytes_ = 0;  // Restart instead of continuing.
+      hash_state_ = "";
+      last_modified_time_ = "";
+      etag_ = "";
+    }
+
+    auto_resume_count_++;
+
+    delegate_->RestartInterruptedDownload(
+        this, content::DownloadUrlParameters::OnStartedCallback());
+  }
+}
+
 bool DownloadItemImpl::IsTemporary() const {
   return is_temporary_;
 }
@@ -464,10 +562,9 @@ bool DownloadItemImpl::IsPersisted() const {
   return is_persisted_;
 }
 
-// TODO(ahendrickson) -- Move |INTERRUPTED| from |IsCancelled()| to
-// |IsPartialDownload()|, when resuming interrupted downloads is implemented.
 bool DownloadItemImpl::IsPartialDownload() const {
-  return InternalToExternalState(state_) == IN_PROGRESS;
+  DownloadState state = InternalToExternalState(state_);
+  return (state == IN_PROGRESS) || (state == INTERRUPTED);

[benjhayden, 2012/10/15 17:59:37]

Have you audited all the callers of IsPartialDownload() to see if any of them
actually mean to call IsInProgress()?
Would you mind splitting this out into a separate CL?

 }
 
 bool DownloadItemImpl::IsInProgress() const {
@@ -475,8 +572,7 @@ bool DownloadItemImpl::IsInProgress() const {
 }
 
 bool DownloadItemImpl::IsCancelled() const {
-  DownloadState external_state = InternalToExternalState(state_);
-  return  external_state == CANCELLED || external_state == INTERRUPTED;

[benjhayden, 2012/10/15 17:59:37]

Have you audited all the callers of IsCancelled() to see if any of them want to
also call IsInterrupted()?
Would you mind splitting this out into a separate CL?
I'm sensitive to subtle changes like this because I've just had a run of subtle
CLs that broke things.

+  return  InternalToExternalState(state_) == CANCELLED;
 }
 
 bool DownloadItemImpl::IsInterrupted() const {
@@ -664,6 +760,21 @@ bool DownloadItemImpl::CanOpenDownload() {
   return !file_externally_removed_;
 }
 
+bool DownloadItemImpl::CanResumeDownload() const {
+  // Do not allow interrupted downloads to be resumed until the target name
+  // has been determined, and the user has validated any dangerous items.
+  // TODO(rdsmith) -- Add a state indicating that filename determination has
+  // occurred.
+  if (GetTargetFilePath().empty())
+    return false;
+
+  if (GetSafetyState() == DANGEROUS)
+    return false;
+
+  return IsInProgress() ||
+         (CanResumeInterrupted() != DownloadItem::RESUME_MODE_INVALID);
+}
+
 bool DownloadItemImpl::ShouldOpenFileBasedOnExtension() {
   return delegate_->ShouldOpenFileBasedOnExtension(GetUserVerifiedFilePath());
 }
@@ -760,7 +871,11 @@ std::string DownloadItemImpl::DebugString(bool verbose) const {
         " received = %" PRId64
         " reason = %s"
         " paused = %c"
+        " resuming = %c"
+        " resume_mode = %s"
         " safety = %s"
+        " all_data_saved = %c"
+        " persisted = %c"
         " last_modified = '%s'"
         " etag = '%s'"
         " url_chain = \n\t\"%s\"\n\t"
@@ -771,7 +886,11 @@ std::string DownloadItemImpl::DebugString(bool verbose) const {
         GetReceivedBytes(),
         InterruptReasonDebugString(last_reason_).c_str(),
         IsPaused() ? 'T' : 'F',
+        IsResuming() ? 'T' : 'F',
+        DebugResumeModeString(CanResumeInterrupted()),
         DebugSafetyStateString(GetSafetyState()),
+        AllDataSaved() ? 'T' : 'F',
+        IsPersisted() ? 'T' : 'F',
         GetLastModifiedTime().c_str(),
         GetETag().c_str(),
         url_list.c_str(),
@@ -797,6 +916,7 @@ void DownloadItemImpl::NotifyRemoved() {
 void DownloadItemImpl::OnDownloadedFileRemoved() {
   file_externally_removed_ = true;
   UpdateObservers();
+  delegate_->MaybeCompleteDownload(download_id_.local());

[benjhayden, 2012/10/15 17:59:37]

Sorry, what's this doing? I looked in download_manager_impl.cc and didn't see an
explanation there, either.

 }
 
 void DownloadItemImpl::OffThreadCancel() {
@@ -809,6 +929,19 @@ void DownloadItemImpl::OffThreadCancel() {
                  delegate_->GetDownloadFileManager(), download_id_));
 }
 
+void DownloadItemImpl::OffThreadInterrupt() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  request_handle_->CancelRequest();
+
+  BrowserThread::PostTask(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&DownloadFileManager::InterruptDownload,
+                 delegate_->GetDownloadFileManager(),
+                 download_id_,
+                 base::Closure()));
+}
+
 // An error occurred somewhere.
 void DownloadItemImpl::Interrupt(content::DownloadInterruptReason reason) {
   // Somewhat counter-intuitively, it is possible for us to receive an
@@ -825,10 +958,25 @@ void DownloadItemImpl::Interrupt(content::DownloadInterruptReason reason) {
     return;
 
   last_reason_ = reason;
+  is_resuming_ = false;
   TransitionTo(INTERRUPTED_INTERNAL);
   download_stats::RecordDownloadInterrupted(
       reason, received_bytes_, total_bytes_);
   delegate_->DownloadStopped(this);
+
+  AutoResumeIfValid();
+}
+
+void DownloadItemImpl::Resume(
+    scoped_ptr<DownloadRequestHandleInterface> req_handle) {
+  DVLOG(20) << __FUNCTION__ << "() " << DebugString(true);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (state_ != INTERRUPTED_INTERNAL)
+    return;
+  request_handle_ = req_handle.Pass();
+  is_resuming_ = true;
+  TransitionTo(IN_PROGRESS_INTERNAL);
 }
 
 void DownloadItemImpl::SetTotalBytes(int64 total_bytes) {
@@ -1014,7 +1162,7 @@ void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
 
 void DownloadItemImpl::MaybeCompleteDownload() {
   // TODO(rdsmith): Move logic for this function here.
-  delegate_->MaybeCompleteDownload(this);
+  delegate_->MaybeCompleteDownload(download_id_.local());
 }
 
 // Called by DownloadManagerImpl::MaybeCompleteDownload() when it has
@@ -1269,6 +1417,10 @@ DownloadItemImpl::ExternalToInternalState(
    return MAX_DOWNLOAD_INTERNAL_STATE;
  }
 
+const net::BoundNetLog& DownloadItemImpl::GetBoundNetLog() const {
+  return bound_net_log_;
+}
+
 const char* DownloadItemImpl::DebugDownloadStateString(
     DownloadInternalState state) {
   switch (state) {
@@ -1287,3 +1439,22 @@ const char* DownloadItemImpl::DebugDownloadStateString(
       return "unknown";
   };
 }
+
+const char* DownloadItemImpl::DebugResumeModeString(
+    DownloadItem::ResumeMode mode) {
+  switch (mode) {
+    case DownloadItem::RESUME_MODE_INVALID:
+      return "INVALID";
+    case DownloadItem::RESUME_MODE_IMMEDIATE_CONTINUE:
+      return "IMMEDIATE_CONTINUE";
+    case DownloadItem::RESUME_MODE_IMMEDIATE_RESTART:
+      return "IMMEDIATE_RESTART";
+    case DownloadItem::RESUME_MODE_USER_CONTINUE:
+      return "USER_CONTINUE";
+    case DownloadItem::RESUME_MODE_USER_RESTART:
+      return "USER_RESTART";
+    default:
+      NOTREACHED() << "Unknown resume mode " << mode;
+      return "unknown";
+  }
+}