[Downloads] Rework how hashes are calculated for download files.

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 6a6dc7d74112e7334413b6be0d36d97355ba44f3..8500ce8a917d95d66703b412470776a93a684637 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -38,6 +38,7 @@
 #include "content/browser/download/download_file.h"
 #include "content/browser/download/download_interrupt_reasons_impl.h"
 #include "content/browser/download/download_item_impl_delegate.h"
+#include "content/browser/download/download_net_log_parameters.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
@@ -98,8 +99,6 @@ bool IsDownloadResumptionEnabled() {
 
 const uint32_t DownloadItem::kInvalidId = 0;
 
-const char DownloadItem::kEmptyFileHash[] = "";
-
 // The maximum number of attempts we will make to resume automatically.
 const int DownloadItemImpl::kMaxAutoResumeAttempts = 5;
 
@@ -118,22 +117,19 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
                                    const std::string& last_modified,
                                    int64_t received_bytes,
                                    int64_t total_bytes,
+                                   const std::string& hash,
                                    DownloadItem::DownloadState state,
                                    DownloadDangerType danger_type,
                                    DownloadInterruptReason interrupt_reason,
                                    bool opened,
                                    const net::BoundNetLog& bound_net_log)
     : download_id_(download_id),
-      current_path_(current_path),
       target_path_(target_path),
       url_chain_(url_chain),
       referrer_url_(referrer_url),
       mime_type_(mime_type),
       original_mime_type_(original_mime_type),
       total_bytes_(total_bytes),
-      received_bytes_(received_bytes),
-      last_modified_time_(last_modified),
-      etag_(etag),
       last_reason_(interrupt_reason),
       start_tick_(base::TimeTicks()),
       state_(ExternalToInternalState(state)),
@@ -141,8 +137,13 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
       start_time_(start_time),
       end_time_(end_time),
       delegate_(delegate),
-      all_data_saved_(state == COMPLETE),
       opened_(opened),
+      current_path_(current_path),
+      received_bytes_(received_bytes),
+      all_data_saved_(state == COMPLETE),
+      hash_(hash),
+      last_modified_time_(last_modified),
+      etag_(etag),
       bound_net_log_(bound_net_log),
       weak_ptr_factory_(this) {
   delegate_->Attach();
@@ -173,14 +174,14 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
       original_mime_type_(info.original_mime_type),
       remote_address_(info.remote_address),
       total_bytes_(info.total_bytes),
-      last_modified_time_(info.last_modified),
-      etag_(info.etag),
       last_reason_(info.result),
       start_tick_(base::TimeTicks::Now()),
       state_(INITIAL_INTERNAL),
       start_time_(info.start_time),
       delegate_(delegate),
       is_temporary_(!info.save_info->file_path.empty()),
+      last_modified_time_(info.last_modified),
+      etag_(info.etag),
       bound_net_log_(bound_net_log),
       weak_ptr_factory_(this) {
   delegate_->Attach();
@@ -208,7 +209,6 @@ DownloadItemImpl::DownloadItemImpl(
     : is_save_package_download_(true),
       request_handle_(std::move(request_handle)),
       download_id_(download_id),
-      current_path_(path),
       target_path_(path),
       url_chain_(1, url),
       mime_type_(mime_type),
@@ -217,6 +217,7 @@ DownloadItemImpl::DownloadItemImpl(
       state_(IN_PROGRESS_INTERNAL),
       start_time_(base::Time::Now()),
       delegate_(delegate),
+      current_path_(path),
       bound_net_log_(bound_net_log),
       weak_ptr_factory_(this) {
   delegate_->Attach();
@@ -371,8 +372,9 @@ void DownloadItemImpl::Resume() {
 void DownloadItemImpl::Cancel(bool user_cancel) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DVLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
-  Interrupt(user_cancel ? DOWNLOAD_INTERRUPT_REASON_USER_CANCELED
-                        : DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN);
+  InterruptAndDiscardPartialState(
+      user_cancel ? DOWNLOAD_INTERRUPT_REASON_USER_CANCELED
+                  : DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN);
   UpdateObservers();
 }
 
@@ -381,7 +383,7 @@ void DownloadItemImpl::Remove() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   delegate_->AssertStateConsistent(this);
-  Interrupt(DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
+  InterruptAndDiscardPartialState(DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
   UpdateObservers();
   delegate_->AssertStateConsistent(this);
 
@@ -590,10 +592,6 @@ const std::string& DownloadItemImpl::GetHash() const {
   return hash_;
 }
 
-const std::string& DownloadItemImpl::GetHashState() const {
-  return hash_state_;
-}
-
 bool DownloadItemImpl::GetFileExternallyRemoved() const {
   return file_externally_removed_;
 }
@@ -785,8 +783,7 @@ std::string DownloadItemImpl::DebugString(bool verbose) const {
 
   if (verbose) {
     description += base::StringPrintf(
-        " total = %" PRId64
-        " received = %" PRId64
+        " total = %" PRId64 " received = %" PRId64
         " reason = %s"
         " paused = %c"
         " resume_mode = %s"
@@ -797,22 +794,16 @@ std::string DownloadItemImpl::DebugString(bool verbose) const {
         " etag = '%s'"
         " has_download_file = %s"
         " url_chain = \n\t\"%s\"\n\t"
-        " full_path = \"%" PRFilePath "\"\n\t"
+        " current_path = \"%" PRFilePath
+        "\"\n\t"
         " target_path = \"%" PRFilePath "\"",
-        GetTotalBytes(),
-        GetReceivedBytes(),
+        GetTotalBytes(), GetReceivedBytes(),
         DownloadInterruptReasonToString(last_reason_).c_str(),
-        IsPaused() ? 'T' : 'F',
-        DebugResumeModeString(GetResumeMode()),
-        auto_resume_count_,
-        GetDangerType(),
-        AllDataSaved() ? 'T' : 'F',
-        GetLastModifiedTime().c_str(),
-        GetETag().c_str(),
-        download_file_.get() ? "true" : "false",
-        url_list.c_str(),
-        GetFullPath().value().c_str(),
-        GetTargetFilePath().value().c_str());
+        IsPaused() ? 'T' : 'F', DebugResumeModeString(GetResumeMode()),
+        auto_resume_count_, GetDangerType(), AllDataSaved() ? 'T' : 'F',
+        GetLastModifiedTime().c_str(), GetETag().c_str(),
+        download_file_.get() ? "true" : "false", url_list.c_str(),
+        GetFullPath().value().c_str(), GetTargetFilePath().value().c_str());
   } else {
     description += base::StringPrintf(" url = \"%s\"", url_list.c_str());
   }
@@ -851,10 +842,14 @@ DownloadItemImpl::ResumeMode DownloadItemImpl::GetResumeMode() const {
     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.
+
     case DOWNLOAD_INTERRUPT_REASON_FILE_TOO_SHORT:
       // The [possibly persisted] file offset disagreed with the file on disk.
 
-      // The intermediate stub is not usable and the server is resonding. Hence
+      // The intermediate stub is not usable and the server is responding. Hence
       // retrying the request from the beginning is likely to work.
       restart_required = true;
       break;
@@ -946,8 +941,7 @@ void DownloadItemImpl::UpdateValidatorsOnResumption(
     origin_state |= ORIGIN_STATE_ON_RESUMPTION_VALIDATORS_CHANGED;
   if (content_disposition_ != new_create_info.content_disposition)
     origin_state |= ORIGIN_STATE_ON_RESUMPTION_CONTENT_DISPOSITION_CHANGED;
-  RecordOriginStateOnResumption(new_create_info.save_info->offset != 0,
-                                origin_state);
+  RecordOriginStateOnResumption(received_bytes_ != 0, origin_state);
 
   url_chain_.insert(
       url_chain_.end(), chain_iter, new_create_info.url_chain.end());
@@ -983,16 +977,19 @@ void DownloadItemImpl::SetTotalBytes(int64_t total_bytes) {
   total_bytes_ = total_bytes;
 }
 
-void DownloadItemImpl::OnAllDataSaved(const std::string& final_hash) {
+void DownloadItemImpl::OnAllDataSaved(
+    int64_t total_bytes,
+    scoped_ptr<crypto::SecureHash> hash_state) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!all_data_saved_);
   all_data_saved_ = true;
-  DVLOG(20) << __FUNCTION__ << " download=" << DebugString(true);
-
-  // Store final hash and null out intermediate serialized hash state.
-  hash_ = final_hash;
-  hash_state_ = "";
+  SetTotalBytes(total_bytes);
+  UpdateProgress(total_bytes, 0);
+  hash_state_ = std::move(hash_state);
+  UpdatePrefixHash();
+  hash_state_.reset();
 
+  DVLOG(20) << __FUNCTION__ << " download=" << DebugString(true);
   UpdateObservers();
 }
 
@@ -1006,8 +1003,7 @@ void DownloadItemImpl::MarkAsComplete() {
 }
 
 void DownloadItemImpl::DestinationUpdate(int64_t bytes_so_far,
-                                         int64_t bytes_per_sec,
-                                         const std::string& hash_state) {
+                                         int64_t bytes_per_sec) {
   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
@@ -1015,19 +1011,15 @@ void DownloadItemImpl::DestinationUpdate(int64_t bytes_so_far,
   // 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);
+
   DVLOG(20) << __FUNCTION__ << " so_far=" << bytes_so_far
             << " per_sec=" << bytes_per_sec
             << " download=" << DebugString(true);
 
-  bytes_per_sec_ = bytes_per_sec;
-  hash_state_ = hash_state;
-  received_bytes_ = bytes_so_far;
-
-  // If we've received more data than we were expecting (bad server info?),
-  // revert to 'unknown size mode'.
-  if (received_bytes_ > total_bytes_)
-    total_bytes_ = 0;
-
+  UpdateProgress(bytes_so_far, bytes_per_sec);
   if (bound_net_log_.IsCapturing()) {
     bound_net_log_.AddEvent(
         net::NetLog::TYPE_DOWNLOAD_ITEM_UPDATED,
@@ -1037,7 +1029,10 @@ void DownloadItemImpl::DestinationUpdate(int64_t bytes_so_far,
   UpdateObservers();
 }
 
-void DownloadItemImpl::DestinationError(DownloadInterruptReason reason) {
+void DownloadItemImpl::DestinationError(
+    DownloadInterruptReason reason,
+    int64_t bytes_so_far,
+    scoped_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
@@ -1052,14 +1047,19 @@ void DownloadItemImpl::DestinationError(DownloadInterruptReason reason) {
   // 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;
   }
-  Interrupt(reason);
+  InterruptWithPartialState(bytes_so_far, std::move(secure_hash), reason);
   UpdateObservers();
 }
 
-void DownloadItemImpl::DestinationCompleted(const std::string& final_hash) {
+void DownloadItemImpl::DestinationCompleted(
+    int64_t total_bytes,
+    scoped_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
@@ -1069,7 +1069,7 @@ void DownloadItemImpl::DestinationCompleted(const std::string& final_hash) {
   DCHECK(state_ == TARGET_PENDING_INTERNAL || state_ == IN_PROGRESS_INTERNAL);
   DVLOG(20) << __FUNCTION__ << " download=" << DebugString(true);
 
-  OnAllDataSaved(final_hash);
+  OnAllDataSaved(total_bytes, std::move(secure_hash));
   MaybeCompleteDownload();
 }
 
@@ -1148,10 +1148,23 @@ void DownloadItemImpl::Start(
   if (new_create_info.result != DOWNLOAD_INTERRUPT_REASON_NONE) {
     DCHECK(!download_file_.get());
 
+    // 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;
+    scoped_ptr<crypto::SecureHash> hash_state =
+        make_scoped_ptr(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()) {
-      TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
+      received_bytes_ = offset;
+      hash_state_ = std::move(hash_state);
+      hash_.clear();
       destination_error_ = new_create_info.result;
+      TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
       DetermineDownloadTarget();
       return;
     }
@@ -1159,7 +1172,8 @@ void DownloadItemImpl::Start(
     // Otherwise, this was a resumption attempt which ended with an
     // interruption. Continue with current target path.
     TransitionTo(TARGET_RESOLVED_INTERNAL);
-    Interrupt(new_create_info.result);
+    InterruptWithPartialState(offset, std::move(hash_state),
+                              new_create_info.result);
     UpdateObservers();
     return;
   }
@@ -1189,7 +1203,12 @@ void DownloadItemImpl::OnDownloadFileInitialized(
   DVLOG(20) << __FUNCTION__
             << "() result:" << DownloadInterruptReasonToString(result);
   if (result != DOWNLOAD_INTERRUPT_REASON_NONE) {
-    // Whoops. That didn't work. Proceed as an interrupted download.
+    // 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;
     TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
   }
@@ -1234,7 +1253,8 @@ void DownloadItemImpl::OnDownloadTargetDetermined(
   // 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) {
-    Interrupt(destination_error_);
+    InterruptWithPartialState(received_bytes_, std::move(hash_state_),
+                              destination_error_);
     destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
     UpdateObservers();
     return;
@@ -1282,33 +1302,36 @@ void DownloadItemImpl::OnDownloadRenamedToIntermediateName(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK_EQ(state_, TARGET_PENDING_INTERNAL);
   DVLOG(20) << __FUNCTION__ << " download=" << DebugString(true);
+
   TransitionTo(TARGET_RESOLVED_INTERNAL);
 
-  if (DOWNLOAD_INTERRUPT_REASON_NONE != destination_error_) {
-    // Process destination error.  If both |reason| and |destination_error_|
-    // refer to actual errors, we want to use the |destination_error_| as the
-    // argument to the Interrupt() routine, as it happened first.
-    if (reason == DOWNLOAD_INTERRUPT_REASON_NONE)
-      SetFullPath(full_path);
-    Interrupt(destination_error_);
-    destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
-    UpdateObservers();
-  } else if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
-    Interrupt(reason);
-    // All file errors result in file deletion above; no need to cleanup.  The
-    // current_path_ should be empty. Resuming this download will force a
-    // restart and a re-doing of filename determination.
-    DCHECK(current_path_.empty());
+  // 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);
     UpdateObservers();
-  } else {
+    return;
+  }
+
+  if (DOWNLOAD_INTERRUPT_REASON_NONE != destination_error_) {
     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
+    InterruptWithPartialState(received_bytes_, std::move(hash_state_),
+                              destination_error_);
+    destination_error_ = DOWNLOAD_INTERRUPT_REASON_NONE;
     UpdateObservers();
-    MaybeCompleteDownload();
+    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
@@ -1370,8 +1393,9 @@ void DownloadItemImpl::OnDownloadCompleting() {
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFile::RenameAndAnnotate,
-                 base::Unretained(download_file_.get()),
-                 GetTargetFilePath(), callback));
+                 base::Unretained(download_file_.get()), GetTargetFilePath(),
+                 delegate_->GetApplicationClientIdForFileScanning(), GetURL(),
+                 GetReferrerUrl(), callback));
 }
 
 void DownloadItemImpl::OnDownloadRenamedToFinalName(
@@ -1391,11 +1415,10 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName(
             << " " << DebugString(false);
 
   if (DOWNLOAD_INTERRUPT_REASON_NONE != reason) {
-    Interrupt(reason);
-
-    // All file errors should have resulted in in file deletion above. On
-    // resumption we will need to re-do filename determination.
-    DCHECK(current_path_.empty());
+    // Failure to perform the final rename is considered fatal. TODO(asanka): It
+    // may not be, in which case we should figure out whether we can recover the
+    // state.
+    InterruptAndDiscardPartialState(reason);
     UpdateObservers();
     return;
   }
@@ -1463,12 +1486,21 @@ void DownloadItemImpl::Completed() {
 
 // **** End of Download progression cascade
 
-// An error occurred somewhere.
-void DownloadItemImpl::Interrupt(DownloadInterruptReason reason) {
+void DownloadItemImpl::InterruptAndDiscardPartialState(
+    DownloadInterruptReason reason) {
+  InterruptWithPartialState(0, scoped_ptr<crypto::SecureHash>(), reason);
+}
+
+void DownloadItemImpl::InterruptWithPartialState(
+    int64_t bytes_so_far,
+    scoped_ptr<crypto::SecureHash> hash_state,
+    DownloadInterruptReason reason) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK_NE(DOWNLOAD_INTERRUPT_REASON_NONE, reason);
   DVLOG(20) << __FUNCTION__
             << "() reason:" << DownloadInterruptReasonToString(reason)
+            << " bytes_so_far:" << bytes_so_far
+            << " hash_state:" << (hash_state ? "Valid" : "Invalid")
             << " this=" << DebugString(true);
 
   // Somewhat counter-intuitively, it is possible for us to receive an
@@ -1509,6 +1541,7 @@ void DownloadItemImpl::Interrupt(DownloadInterruptReason reason) {
 
     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 &&
@@ -1540,6 +1573,16 @@ void DownloadItemImpl::Interrupt(DownloadInterruptReason reason) {
   // behave properly with setting all_data_saved_ to false here.
   all_data_saved_ = false;
 
+  if (current_path_.empty()) {
+    hash_state_.reset();
+    hash_.clear();
+    received_bytes_ = 0;
+  } else {
+    UpdateProgress(bytes_so_far, 0);
+    hash_state_ = std::move(hash_state);
+    UpdatePrefixHash();
+  }
+
   if (request_handle_)
     request_handle_->CancelRequest();
 
@@ -1566,6 +1609,29 @@ void DownloadItemImpl::Interrupt(DownloadInterruptReason reason) {
   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;
+
+  // If we've received more data than we were expecting (bad server info?),
+  // revert to 'unknown size mode'.
+  if (received_bytes_ > total_bytes_)
+    total_bytes_ = 0;
+}
+
+void DownloadItemImpl::UpdatePrefixHash() {

[svaldez, 2016/03/10 17:45:34]

Is it possible to get rid of hash_ and make GetHash do this calculation? It
seems easy for someone updating hash_state_ to forget to call UpdatePrefixHash.

[asanka, 2016/03/11 16:25:09]

GetHash() may be called at a time where there's no partial hash_state_ (e.g.
right after the download was restored from history, but before resumption).
Instead I added a SetHashState() method that handles the work of setting
hash_state_ and hash_.

I also renamed prefix_hash to hash_of_partial_file to better reflect what it is.

+  if (!hash_state_) {
+    hash_.clear();
+    return;
+  }
+
+  scoped_ptr<crypto::SecureHash> clone(hash_state_->Clone());
+  std::vector<char> hash_value(clone->GetHashLength());
+  clone->Finish(&hash_value.front(), hash_value.size());
+  hash_.assign(hash_value.begin(), hash_value.end());
+}
+
 void DownloadItemImpl::ReleaseDownloadFile(bool destroy_file) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DVLOG(20) << __FUNCTION__ << "() destroy_file:" << destroy_file;
@@ -1681,24 +1747,21 @@ void DownloadItemImpl::TransitionTo(DownloadInternalState new_state) {
       break;
 
     case INTERRUPTED_INTERNAL:
-      bound_net_log_.AddEvent(
-          net::NetLog::TYPE_DOWNLOAD_ITEM_INTERRUPTED,
-          base::Bind(&ItemInterruptedNetLogCallback, last_reason_,
-                     received_bytes_, &hash_state_));
+      bound_net_log_.AddEvent(net::NetLog::TYPE_DOWNLOAD_ITEM_INTERRUPTED,
+                              base::Bind(&ItemInterruptedNetLogCallback,
+                                         last_reason_, received_bytes_));
       break;
 
     case RESUMING_INTERNAL:
-      bound_net_log_.AddEvent(
-          net::NetLog::TYPE_DOWNLOAD_ITEM_RESUMED,
-          base::Bind(&ItemResumingNetLogCallback, false, last_reason_,
-                     received_bytes_, &hash_state_));
+      bound_net_log_.AddEvent(net::NetLog::TYPE_DOWNLOAD_ITEM_RESUMED,
+                              base::Bind(&ItemResumingNetLogCallback, false,
+                                         last_reason_, received_bytes_));
       break;
 
     case CANCELLED_INTERNAL:
       bound_net_log_.AddEvent(
           net::NetLog::TYPE_DOWNLOAD_ITEM_CANCELED,
-          base::Bind(&ItemCanceledNetLogCallback, received_bytes_,
-                     &hash_state_));
+          base::Bind(&ItemCanceledNetLogCallback, received_bytes_));
       break;
 
     case MAX_DOWNLOAD_INTERNAL_STATE:
@@ -1799,9 +1862,10 @@ void DownloadItemImpl::ResumeInterruptedDownload() {
   if (mode == RESUME_MODE_IMMEDIATE_RESTART ||
       mode == RESUME_MODE_USER_RESTART) {
     received_bytes_ = 0;
-    hash_state_ = "";
-    last_modified_time_ = "";
-    etag_ = "";
+    last_modified_time_.clear();
+    etag_.clear();
+    hash_.clear();
+    hash_state_.reset();
   }
 
   // Avoid using the WebContents even if it's still around. Resumption requests
@@ -1812,9 +1876,10 @@ void DownloadItemImpl::ResumeInterruptedDownload() {
       GetURL(), -1, -1, -1, GetBrowserContext()->GetResourceContext()));
   download_params->set_file_path(GetFullPath());
   download_params->set_offset(GetReceivedBytes());
-  download_params->set_hash_state(GetHashState());
   download_params->set_last_modified(GetLastModifiedTime());
   download_params->set_etag(GetETag());
+  download_params->set_prefix_hash(hash_);
+  download_params->set_hash_state(std::move(hash_state_));
 
   TransitionTo(RESUMING_INTERNAL);
   delegate_->ResumeInterruptedDownload(std::move(download_params), GetId());