[downloads] Move platform specific code out of DownloadTargetDeterminer.

content/browser/download/download_item_impl.cc

 // Copyright (c) 2012 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.
 
 // File method ordering: Methods in this file are in the same order as
 // in download_item_impl.h, with the following exception: The public
 // interface Start is placed in chronological order with the other
 // (private) routines that together define a DownloadItem's state
 // transitions as the download progresses.  See "Download progression
 // cascade" later in this file.
@@ skipping 98 matching lines @@
     temp_file_path.clear();
     return base::FilePath();
   }
 }
 
 static void DownloadFileCancel(std::unique_ptr<DownloadFile> download_file) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   download_file->Cancel();
 }
 
+bool IsCancellation(DownloadInterruptReason reason) {
+  return reason == DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN ||
+         reason == DOWNLOAD_INTERRUPT_REASON_USER_CANCELED;
+}
+
 }  // namespace
 
 const uint32_t DownloadItem::kInvalidId = 0;
 
 // The maximum number of attempts we will make to resume automatically.
 const int DownloadItemImpl::kMaxAutoResumeAttempts = 5;
 
 // Constructor for reading from the history service.
 DownloadItemImpl::DownloadItemImpl(
     DownloadItemImplDelegate* delegate,
@@ skipping 182 matching lines @@
 
   danger_type_ = DOWNLOAD_DANGER_TYPE_USER_VALIDATED;
 
   net_log_.AddEvent(
       net::NetLogEventType::DOWNLOAD_ITEM_SAFETY_STATE_UPDATED,
       base::Bind(&ItemCheckedNetLogCallback, GetDangerType()));
 
   UpdateObservers();  // TODO(asanka): This is potentially unsafe. The download
                       // may not be in a consistent state or around at all after
                       // invoking observers. http://crbug.com/586610
-
   MaybeCompleteDownload();
 }
 
 void DownloadItemImpl::StealDangerousDownload(
     bool delete_file_afterward,
     const AcquireFileCallback& callback) {
   DVLOG(20) << __func__ << "() download = " << DebugString(true);
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(IsDangerous());
   DCHECK(AllDataSaved());
@@ skipping 273 matching lines @@
 std::string DownloadItemImpl::GetOriginalMimeType() const {
   return original_mime_type_;
 }
 
 std::string DownloadItemImpl::GetRemoteAddress() const {
   return remote_address_;
 }
 
 bool DownloadItemImpl::HasUserGesture() const {
   return has_user_gesture_;
-};
+}
 
 ui::PageTransition DownloadItemImpl::GetTransitionType() const {
   return transition_type_;
-};
+}
 
 const std::string& DownloadItemImpl::GetLastModifiedTime() const {
   return last_modified_time_;
 }
 
 const std::string& DownloadItemImpl::GetETag() const {
   return etag_;
 }
 
 bool DownloadItemImpl::IsSavePackageDownload() const {
@@ skipping 274 matching lines @@
   // We also can't continue if we don't have some verifier to make sure
   // we're getting the same file.
   bool restart_required =
       (current_path_.empty() || (etag_.empty() && last_modified_time_.empty()));
 
   // We won't auto-restart if we've used up our attempts or the
   // download has been paused by user action.
   bool user_action_required =
       (auto_resume_count_ >= kMaxAutoResumeAttempts || IsPaused());
 
-  switch(last_reason_) {
+  switch (last_reason_) {
     case DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR:
     case DOWNLOAD_INTERRUPT_REASON_NETWORK_TIMEOUT:
       break;
 
     case DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE:
     // The server disagreed with the file offset that we sent.
 
     case DOWNLOAD_INTERRUPT_REASON_FILE_HASH_MISMATCH:
     // The file on disk was found to not match the expected hash. Discard and
     // start from beginning.
@@ skipping 40 matching lines @@
     case DOWNLOAD_INTERRUPT_REASON_NONE:
     case DOWNLOAD_INTERRUPT_REASON_NETWORK_INVALID_REQUEST:
     case DOWNLOAD_INTERRUPT_REASON_FILE_VIRUS_INFECTED:
     case DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT:
     case DOWNLOAD_INTERRUPT_REASON_USER_CANCELED:
     case DOWNLOAD_INTERRUPT_REASON_FILE_BLOCKED:
     case DOWNLOAD_INTERRUPT_REASON_FILE_SECURITY_CHECK_FAILED:
     case DOWNLOAD_INTERRUPT_REASON_SERVER_UNAUTHORIZED:
     case DOWNLOAD_INTERRUPT_REASON_SERVER_CERT_PROBLEM:
     case DOWNLOAD_INTERRUPT_REASON_SERVER_FORBIDDEN:
-      // Unhandled.
       return RESUME_MODE_INVALID;
   }
 
   if (user_action_required && restart_required)
     return RESUME_MODE_USER_RESTART;
 
   if (restart_required)
     return RESUME_MODE_IMMEDIATE_RESTART;
 
   if (user_action_required)
@@ skipping 104 matching lines @@
     int64_t bytes_per_sec,
     const std::vector<DownloadItem::ReceivedSlice>& received_slices) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // If the download is in any other state we don't expect any
   // DownloadDestinationObserver callbacks. An interruption or a cancellation
   // results in a call to ReleaseDownloadFile which invalidates the weak
   // reference held by the DownloadFile and hence cuts off any pending
   // callbacks.
   DCHECK(state_ == TARGET_PENDING_INTERNAL || state_ == IN_PROGRESS_INTERNAL);
 
-  // There must be no pending destination_error_.
-  DCHECK_EQ(destination_error_, DOWNLOAD_INTERRUPT_REASON_NONE);
+  // There must be no pending deferred_interrupt_reason_.
+  DCHECK_EQ(deferred_interrupt_reason_, DOWNLOAD_INTERRUPT_REASON_NONE);
 
   DVLOG(20) << __func__ << "() so_far=" << bytes_so_far
             << " per_sec=" << bytes_per_sec
             << " download=" << DebugString(true);
 
   UpdateProgress(bytes_so_far, bytes_per_sec);
   received_slices_ = received_slices;
   if (net_log_.IsCapturing()) {
     net_log_.AddEvent(
         net::NetLogEventType::DOWNLOAD_ITEM_UPDATED,
         net::NetLog::Int64Callback("bytes_so_far", received_bytes_));
   }
 
   UpdateObservers();
 }
 
 void DownloadItemImpl::DestinationError(
     DownloadInterruptReason reason,
     int64_t bytes_so_far,
     std::unique_ptr<crypto::SecureHash> secure_hash) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // If the download is in any other state we don't expect any
   // DownloadDestinationObserver callbacks. An interruption or a cancellation
   // results in a call to ReleaseDownloadFile which invalidates the weak
   // reference held by the DownloadFile and hence cuts off any pending
   // callbacks.
   DCHECK(state_ == TARGET_PENDING_INTERNAL || state_ == IN_PROGRESS_INTERNAL);
   DVLOG(20) << __func__
-            << "() reason:" << DownloadInterruptReasonToString(reason);
+            << "() reason:" << DownloadInterruptReasonToString(reason)
+            << " this:" << DebugString(true);
 
-  // Postpone recognition of this error until after file name determination
-  // has completed and the intermediate file has been renamed to simplify
-  // resumption conditions.
-  if (state_ == TARGET_PENDING_INTERNAL) {
-    received_bytes_ = bytes_so_far;
-    hash_state_ = std::move(secure_hash);
-    hash_.clear();
-    destination_error_ = reason;
-    return;
-  }
   InterruptWithPartialState(bytes_so_far, std::move(secure_hash), reason);
   UpdateObservers();
 }
 
 void DownloadItemImpl::DestinationCompleted(
     int64_t total_bytes,
     std::unique_ptr<crypto::SecureHash> secure_hash) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // If the download is in any other state we don't expect any
   // DownloadDestinationObserver callbacks. An interruption or a cancellation
@@ skipping 47 matching lines @@
     std::unique_ptr<DownloadRequestHandleInterface> req_handle,
     const DownloadCreateInfo& new_create_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!download_file_.get());
   DVLOG(20) << __func__ << "() this=" << DebugString(true);
   RecordDownloadCount(START_COUNT);
 
   download_file_ = std::move(file);
   job_ = DownloadJobFactory::CreateJob(this, std::move(req_handle),
                                        new_create_info);
-  destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
+  deferred_interrupt_reason_ = DOWNLOAD_INTERRUPT_REASON_NONE;
 
   if (state_ == CANCELLED_INTERNAL) {
     // The download was in the process of resuming when it was cancelled. Don't
     // proceed.
     ReleaseDownloadFile(true);
     job_->Cancel(true);
     return;
   }
 
   // The state could be one of the following:
@@ skipping 15 matching lines @@
     // Download requests that are interrupted by Start() should result in a
     // DownloadCreateInfo with an intact DownloadSaveInfo.
     DCHECK(new_create_info.save_info);
 
     int64_t offset = new_create_info.save_info->offset;
     std::unique_ptr<crypto::SecureHash> hash_state =
         new_create_info.save_info->hash_state
             ? new_create_info.save_info->hash_state->Clone()
             : nullptr;
 
-    // Interrupted downloads also need a target path.
-    if (target_path_.empty()) {
-      received_bytes_ = offset;
-      hash_state_ = std::move(hash_state);
-      hash_.clear();
-      destination_error_ = new_create_info.result;
-      received_slices_.clear();
-      TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
-      DetermineDownloadTarget();
-      return;
-    }
-
-    // Otherwise, this was a resumption attempt which ended with an
-    // interruption. Continue with current target path.
-    TransitionTo(TARGET_RESOLVED_INTERNAL);
-    InterruptWithPartialState(
-        offset, std::move(hash_state), new_create_info.result);
-    UpdateObservers();
+    current_path_ = new_create_info.save_info->file_path;
+    received_bytes_ = offset;
+    hash_state_ = std::move(hash_state);
+    hash_.clear();
+    deferred_interrupt_reason_ = new_create_info.result;
+    TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
+    DetermineDownloadTarget();
     return;
   }
 
   if (state_ == INITIAL_INTERNAL) {
     RecordDownloadCount(NEW_DOWNLOAD_COUNT);
     RecordDownloadMimeType(mime_type_);
     if (!GetBrowserContext()->IsOffTheRecord()) {
       RecordDownloadCount(NEW_DOWNLOAD_COUNT_NORMAL_PROFILE);
       RecordDownloadMimeTypeForNormalProfile(mime_type_);
     }
@@ skipping 20 matching lines @@
       base::Bind(&DownloadFile::Initialize,
                  // Safe because we control download file lifetime.
                  base::Unretained(download_file_.get()),
                  base::Bind(&DownloadItemImpl::OnDownloadFileInitialized,
                             weak_ptr_factory_.GetWeakPtr())));
 }
 
 void DownloadItemImpl::OnDownloadFileInitialized(
     DownloadInterruptReason result) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK_EQ(state_, TARGET_PENDING_INTERNAL);
+  DCHECK(state_ == TARGET_PENDING_INTERNAL ||
+         state_ == INTERRUPTED_TARGET_PENDING_INTERNAL)
+      << "Unexpected state: " << DebugDownloadStateString(state_);
+
   DVLOG(20) << __func__
             << "() result:" << DownloadInterruptReasonToString(result);
   if (result != DOWNLOAD_INTERRUPT_REASON_NONE) {
-    // Whoops. That didn't work. Proceed as an interrupted download, but reset
-    // the partial state. Currently, the partial stub cannot be recovered if the
-    // download file initialization fails.
-    received_bytes_ = 0;
-    hash_state_.reset();
-    hash_.clear();
-    destination_error_ = result;
-    received_slices_.clear();
-    TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
+    ReleaseDownloadFile(true);
+    InterruptAndDiscardPartialState(result);
   }
 
   DetermineDownloadTarget();
 }
 
 void DownloadItemImpl::DetermineDownloadTarget() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DVLOG(20) << __func__ << "() " << DebugString(true);
 
   delegate_->DetermineDownloadTarget(
       this, base::Bind(&DownloadItemImpl::OnDownloadTargetDetermined,
                        weak_ptr_factory_.GetWeakPtr()));
 }
 
 // Called by delegate_ when the download target path has been determined.
 void DownloadItemImpl::OnDownloadTargetDetermined(
     const base::FilePath& target_path,
     TargetDisposition disposition,
     DownloadDangerType danger_type,
-    const base::FilePath& intermediate_path) {
+    const base::FilePath& intermediate_path,
+    DownloadInterruptReason interrupt_reason) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(state_ == TARGET_PENDING_INTERNAL ||
          state_ == INTERRUPTED_TARGET_PENDING_INTERNAL);
+  DVLOG(20) << __func__ << "() target_path:" << target_path.value()
+            << " intermediate_path:" << intermediate_path.value()
+            << " disposition:" << disposition << " danger_type:" << danger_type
+            << " interrupt_reason:"
+            << DownloadInterruptReasonToString(interrupt_reason)
+            << " this:" << DebugString(true);
 
-  // If the |target_path| is empty, then we consider this download to be
-  // canceled.
-  if (target_path.empty()) {
+  if (IsCancellation(interrupt_reason) || target_path.empty()) {
     Cancel(true);
     return;
   }
 
-  DVLOG(20) << __func__ << "() target_path:" << target_path.value()
-            << " disposition:" << disposition << " danger_type:" << danger_type
-            << " this:" << DebugString(true);
-
   target_path_ = target_path;
   target_disposition_ = disposition;
   SetDangerType(danger_type);
 
-  // This was an interrupted download that was looking for a filename. Now that
-  // it has one, transition to interrupted.
-  if (state_ == INTERRUPTED_TARGET_PENDING_INTERNAL) {
-    InterruptWithPartialState(
-        received_bytes_, std::move(hash_state_), destination_error_);
-    destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
-    UpdateObservers();
+  // There were no other pending errors, and we just failed to determined the
+  // download target.
+  if (state_ == TARGET_PENDING_INTERNAL &&
+      interrupt_reason != DOWNLOAD_INTERRUPT_REASON_NONE) {
+    deferred_interrupt_reason_ = interrupt_reason;
+    TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
+  }
+
+  // This was an interrupted download that was looking for a filename. Resolve
+  // early without performing the intermediate rename. If there is a
+  // DownloadFile, then that should be renamed to the intermediate name before
+  // we can interrupt the download. Otherwise we may lose intermediate state.
+  if (state_ == INTERRUPTED_TARGET_PENDING_INTERNAL && !download_file_) {
+    OnTargetResolved();
     return;
   }
 
   // We want the intermediate and target paths to refer to the same directory so
   // that they are both on the same device and subject to same
   // space/permission/availability constraints.
   DCHECK(intermediate_path.DirName() == target_path.DirName());
 
   // During resumption, we may choose to proceed with the same intermediate
   // file. No rename is necessary if our intermediate file already has the
   // correct name.
   //
   // The intermediate name may change from its original value during filename
   // determination on resumption, for example if the reason for the interruption
   // was the download target running out space, resulting in a user prompt.
   if (intermediate_path == current_path_) {
     OnDownloadRenamedToIntermediateName(DOWNLOAD_INTERRUPT_REASON_NONE,
                                         intermediate_path);
     return;
   }
 
   // Rename to intermediate name.
   // TODO(asanka): Skip this rename if AllDataSaved() is true. This avoids a
   //               spurious rename when we can just rename to the final
   //               filename. Unnecessary renames may cause bugs like
   //               http://crbug.com/74187.
   DCHECK(!is_save_package_download_);
-  DCHECK(download_file_.get());
   DownloadFile::RenameCompletionCallback callback =
       base::Bind(&DownloadItemImpl::OnDownloadRenamedToIntermediateName,
                  weak_ptr_factory_.GetWeakPtr());
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFile::RenameAndUniquify,
                  // Safe because we control download file lifetime.
                  base::Unretained(download_file_.get()),
                  intermediate_path, callback));
 }
 
 void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
     DownloadInterruptReason reason,
     const base::FilePath& full_path) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK_EQ(state_, TARGET_PENDING_INTERNAL);
+  DCHECK(state_ == TARGET_PENDING_INTERNAL ||
+         state_ == INTERRUPTED_TARGET_PENDING_INTERNAL);
+  DCHECK(download_file_);
   DVLOG(20) << __func__ << "() download=" << DebugString(true);
 
+  if (DOWNLOAD_INTERRUPT_REASON_NONE == reason) {
+    SetFullPath(full_path);
+  } else {
+    // TODO(asanka): Even though the rename failed, it may still be possible to
+    // recover the partial state from the 'before' name.
+    deferred_interrupt_reason_ = reason;
+    TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
+  }
+  OnTargetResolved();
+}
+
+void DownloadItemImpl::OnTargetResolved() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DVLOG(20) << __func__ << "() download=" << DebugString(true);
+  DCHECK((state_ == TARGET_PENDING_INTERNAL &&
+          deferred_interrupt_reason_ == DOWNLOAD_INTERRUPT_REASON_NONE) ||
+         (state_ == INTERRUPTED_TARGET_PENDING_INTERNAL &&
+          deferred_interrupt_reason_ != DOWNLOAD_INTERRUPT_REASON_NONE))
+      << " deferred_interrupt_reason_:"
+      << DownloadInterruptReasonToString(deferred_interrupt_reason_)
+      << " this:" << DebugString(true);
+
+  // This transition is here to ensure that the DownloadItemImpl state machines
+  // doesn't transition to INTERRUPTED or IN_PROGRESS from
+  // TARGET_PENDING_INTERNAL directly. Doing so without passing through
+  // OnTargetResolved() can result in an externally visible state where the
+  // download is interrupted but doesn't have a target path associated with it.
+  //
+  // While not terrible, this complicates the DownloadItem<->Observer
+  // relationship since an observer that needs a target path in order to respond
+  // properly to an interruption will need to wait for another OnDownloadUpdated
+  // notification.  This requirement currently affects all of our UIs.
   TransitionTo(TARGET_RESOLVED_INTERNAL);
 
-  // If the intermediate rename fails while there's also a destination_error_,
-  // then the former is considered the critical error since it requires
-  // discarding the partial state.
-  if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
-    // TODO(asanka): Even though the rename failed, it may still be possible to
-    // recover the partial state from the 'before' name.
-    InterruptAndDiscardPartialState(reason);
+  if (DOWNLOAD_INTERRUPT_REASON_NONE != deferred_interrupt_reason_) {
+    InterruptWithPartialState(received_bytes_, std::move(hash_state_),
+                              deferred_interrupt_reason_);
+    deferred_interrupt_reason_ = DOWNLOAD_INTERRUPT_REASON_NONE;
     UpdateObservers();
     return;
   }
 
-  if (DOWNLOAD_INTERRUPT_REASON_NONE != destination_error_) {
-    SetFullPath(full_path);
-    InterruptWithPartialState(
-        received_bytes_, std::move(hash_state_), destination_error_);
-    destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
-    UpdateObservers();
-    return;
-  }
-
-  SetFullPath(full_path);
   TransitionTo(IN_PROGRESS_INTERNAL);
   // TODO(asanka): Calling UpdateObservers() prior to MaybeCompleteDownload() is
   // not safe. The download could be in an underminate state after invoking
   // observers. http://crbug.com/586610
   UpdateObservers();
   MaybeCompleteDownload();
 }
 
 // When SavePackage downloads MHTML to GData (see
 // SavePackageFilePickerChromeOS), GData calls MaybeCompleteDownload() like it
@@ skipping 183 matching lines @@
     case COMPLETING_INTERNAL:
     case COMPLETE_INTERNAL:
       // Already complete.
       return;
 
     case INITIAL_INTERNAL:
     case MAX_DOWNLOAD_INTERNAL_STATE:
       NOTREACHED();
       return;
 
+    case TARGET_PENDING_INTERNAL:
     case INTERRUPTED_TARGET_PENDING_INTERNAL:
+      // Postpone recognition of this error until after file name determination
+      // has completed and the intermediate file has been renamed to simplify
+      // resumption conditions. The target determination logic is much simpler
+      // if the state of the download remains constant until that stage
+      // completes.
+      //
+      // current_path_ may be empty because it is possible for DownloadItem to
+      // receive a DestinationError prior to the download file initialization
+      // complete callback.
+      if (!IsCancellation(reason)) {
+        UpdateProgress(bytes_so_far, 0);
+        SetHashState(std::move(hash_state));
+        deferred_interrupt_reason_ = reason;
+        TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
+        return;
+      }
+    // else - Fallthrough for cancellation handling which is equivalent to the
+    // IN_PROGRESS state.
+
     case IN_PROGRESS_INTERNAL:
-    case TARGET_PENDING_INTERNAL:
     case TARGET_RESOLVED_INTERNAL:
       // last_reason_ needs to be set for GetResumeMode() to work.
       last_reason_ = reason;
 
       if (download_file_) {
         ResumeMode resume_mode = GetResumeMode();
         ReleaseDownloadFile(resume_mode != RESUME_MODE_IMMEDIATE_CONTINUE &&
                             resume_mode != RESUME_MODE_USER_CONTINUE);
       }
       break;
 
     case RESUMING_INTERNAL:
     case INTERRUPTED_INTERNAL:
       DCHECK(!download_file_);
       // The first non-cancel interrupt reason wins in cases where multiple
       // things go wrong.
-      if (reason != DOWNLOAD_INTERRUPT_REASON_USER_CANCELED &&
-          reason != DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN)
+      if (!IsCancellation(reason))
         return;
 
       last_reason_ = reason;
       if (!current_path_.empty()) {
         // There is no download file and this is transitioning from INTERRUPTED
         // to CANCELLED. The intermediate file is no longer usable, and should
         // be deleted.
         BrowserThread::PostTask(
             BrowserThread::FILE, FROM_HERE,
             base::Bind(base::IgnoreResult(&DeleteDownloadedFile),
@@ skipping 20 matching lines @@
     received_bytes_ = 0;
     received_slices_.clear();
   } else {
     UpdateProgress(bytes_so_far, 0);
     SetHashState(std::move(hash_state));
   }
 
   if (job_)
     job_->Cancel(false);
 
-  if (reason == DOWNLOAD_INTERRUPT_REASON_USER_CANCELED ||
-      reason == DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN) {
+  if (IsCancellation(reason)) {
     if (IsDangerous()) {
       RecordDangerousDownloadDiscard(
           reason == DOWNLOAD_INTERRUPT_REASON_USER_CANCELED
               ? DOWNLOAD_DISCARD_DUE_TO_USER_ACTION
               : DOWNLOAD_DISCARD_DUE_TO_SHUTDOWN,
           GetDangerType(), GetTargetFilePath());
     }
 
     RecordDownloadCount(CANCELLED_COUNT);
+    DCHECK_EQ(last_reason_, reason);
     TransitionTo(CANCELLED_INTERNAL);
     return;
   }
 
   RecordDownloadInterrupted(reason, received_bytes_, total_bytes_);
   if (!GetWebContents())
     RecordDownloadCount(INTERRUPTED_WITHOUT_WEBCONTENTS);
 
+  // TODO(asanka): This is not good. We can transition to interrupted from
+  // target-pending, which is something we don't want to do. Perhaps we should
+  // explicitly transition to target-resolved prior to switching to interrupted.
+  DCHECK_EQ(last_reason_, reason);
   TransitionTo(INTERRUPTED_INTERNAL);
   AutoResumeIfValid();
 }
 
 void DownloadItemImpl::UpdateProgress(
     int64_t bytes_so_far,
     int64_t bytes_per_sec) {
   received_bytes_ = bytes_so_far;
   bytes_per_sec_ = bytes_per_sec;
 
@@ skipping 126 matching lines @@
           base::Bind(&ItemCompletingNetLogCallback, received_bytes_, &hash_));
       break;
 
     case COMPLETE_INTERNAL:
       net_log_.AddEvent(
           net::NetLogEventType::DOWNLOAD_ITEM_FINISHED,
           base::Bind(&ItemFinishedNetLogCallback, auto_opened_));
       break;
 
     case INTERRUPTED_INTERNAL:
+      DCHECK(!download_file_)
+          << "Download file must be released prior to interruption.";
+      DCHECK_NE(last_reason_, DOWNLOAD_INTERRUPT_REASON_NONE);
       net_log_.AddEvent(net::NetLogEventType::DOWNLOAD_ITEM_INTERRUPTED,
                               base::Bind(&ItemInterruptedNetLogCallback,
                                          last_reason_, received_bytes_));
       break;
 
     case RESUMING_INTERNAL:
       net_log_.AddEvent(net::NetLogEventType::DOWNLOAD_ITEM_RESUMED,
                               base::Bind(&ItemResumingNetLogCallback, false,
                                          last_reason_, received_bytes_));
       break;
@@ skipping 89 matching lines @@
     return;
 
   // We are starting a new request. Shake off all pending operations.
   DCHECK(!download_file_);
   weak_ptr_factory_.InvalidateWeakPtrs();
 
   // Reset the appropriate state if restarting.
   ResumeMode mode = GetResumeMode();
   if (mode == RESUME_MODE_IMMEDIATE_RESTART ||
       mode == RESUME_MODE_USER_RESTART) {
+    DCHECK(current_path_.empty());
     received_bytes_ = 0;
     last_modified_time_.clear();
     etag_.clear();
     hash_.clear();
     hash_state_.reset();
     received_slices_.clear();
   }
 
   StoragePartition* storage_partition =
       BrowserContext::GetStoragePartitionForSite(GetBrowserContext(),
@@ skipping 121 matching lines @@
   switch (from) {
     case INITIAL_INTERNAL:
       return to == TARGET_PENDING_INTERNAL ||
              to == INTERRUPTED_TARGET_PENDING_INTERNAL;
 
     case TARGET_PENDING_INTERNAL:
       return to == INTERRUPTED_TARGET_PENDING_INTERNAL ||
              to == TARGET_RESOLVED_INTERNAL || to == CANCELLED_INTERNAL;
 
     case INTERRUPTED_TARGET_PENDING_INTERNAL:
-      return to == INTERRUPTED_INTERNAL || to == CANCELLED_INTERNAL;
+      return to == TARGET_RESOLVED_INTERNAL || to == CANCELLED_INTERNAL;
 
     case TARGET_RESOLVED_INTERNAL:
       return to == IN_PROGRESS_INTERNAL || to == INTERRUPTED_INTERNAL ||
              to == CANCELLED_INTERNAL;
 
     case IN_PROGRESS_INTERNAL:
       return to == COMPLETING_INTERNAL || to == CANCELLED_INTERNAL ||
              to == INTERRUPTED_INTERNAL;
 
     case COMPLETING_INTERNAL:
@@ skipping 40 matching lines @@
     case COMPLETE_INTERNAL:
       return "COMPLETE";
     case CANCELLED_INTERNAL:
       return "CANCELLED";
     case INTERRUPTED_INTERNAL:
       return "INTERRUPTED";
     case RESUMING_INTERNAL:
       return "RESUMING";
     case MAX_DOWNLOAD_INTERNAL_STATE:
       break;
-  };
+  }
   NOTREACHED() << "Unknown download state " << state;
   return "unknown";
 }
 
 const char* DownloadItemImpl::DebugResumeModeString(ResumeMode mode) {
   switch (mode) {
     case RESUME_MODE_INVALID:
       return "INVALID";
     case RESUME_MODE_IMMEDIATE_CONTINUE:
       return "IMMEDIATE_CONTINUE";
     case RESUME_MODE_IMMEDIATE_RESTART:
       return "IMMEDIATE_RESTART";
     case RESUME_MODE_USER_CONTINUE:
       return "USER_CONTINUE";
     case RESUME_MODE_USER_RESTART:
       return "USER_RESTART";
   }
   NOTREACHED() << "Unknown resume mode " << mode;
   return "unknown";
 }
 
 }  // namespace content