Clear the received slices in DownloadItemImpl when etag changed.

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 1010 matching lines @@
   if (*chain_iter == url_chain_.back())
     ++chain_iter;
 
   // Record some stats. If the precondition failed (the server returned
   // HTTP_PRECONDITION_FAILED), then the download will automatically retried as
   // a full request rather than a partial. Full restarts clobber validators.
   int origin_state = 0;
   if (chain_iter != new_create_info.url_chain.end())
     origin_state |= ORIGIN_STATE_ON_RESUMPTION_ADDITIONAL_REDIRECTS;
   if (etag_ != new_create_info.etag ||
-      last_modified_time_ != new_create_info.last_modified)
+      last_modified_time_ != new_create_info.last_modified) {
+    received_slices_.clear();

[qinmin, 2017/04/07 18:03:49]

should we also clear the received_bytes_ after the
RecordOriginStateOnResumption().

The received_bytes_ will get cleared when DownloadFile updates its state through
a posted task. but if Chrome crashes before the update, then the etag_ and
last_modified_time_ has been updated to new values, but the received_bytes_
hasn't. If the db happens to commit the etag_ and last_modified_time_ change,
then if user restarts chrome and resumes the download, the download will not
start from offset 0.

[xingliu, 2017/04/07 20:50:08]

Done.

Make sense, it's good to clear received slices before the new
content_disposition is set in memory. So we have a lower chance to get
inconsistency in db records.

[xingliu, 2017/04/07 22:18:03]

Done,

Sorry that I somehow misunderstood the comment, set the received_bytes_ to 0
only when validators changed.

Also we probably shouldn't set the received_bytes_ to 0 after
RecordOriginStateOnResumption(), since when validators are not changed and
crashed before DownloadFile sets the received_bytes_, we probably should not
restart from 0.

     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(received_bytes_ != 0, origin_state);
 
   url_chain_.insert(
       url_chain_.end(), chain_iter, new_create_info.url_chain.end());
   etag_ = new_create_info.etag;
   last_modified_time_ = new_create_info.last_modified;
   content_disposition_ = new_create_info.content_disposition;
   // It is possible that the previous download attempt failed right before the
@@ skipping 239 matching lines @@
   job_->Start();
 }
 
 void DownloadItemImpl::StartDownload() {
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFile::Initialize,
                  // Safe because we control download file lifetime.
                  base::Unretained(download_file_.get()),
                  base::Bind(&DownloadItemImpl::OnDownloadFileInitialized,
-                            weak_ptr_factory_.GetWeakPtr())));
+                            weak_ptr_factory_.GetWeakPtr()),
+                 received_slices_));
 }
 
 void DownloadItemImpl::OnDownloadFileInitialized(
     DownloadInterruptReason result) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(state_ == TARGET_PENDING_INTERNAL ||
          state_ == INTERRUPTED_TARGET_PENDING_INTERNAL)
       << "Unexpected state: " << DebugDownloadStateString(state_);
 
   DVLOG(20) << __func__
@@ skipping 937 matching lines @@
     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