[Downloads] Update origin info after each response.

When a download is resumed, a new URL request is sent out. The response
received for this request may contain new ETag and Last-Modified
information which should be used for subsequent resumption attempts.
Otherwise if a resource changes (along with the corresponding ETag)
subsequent partial resumption attempts will all fail even if it should
have succeeded.

BUG=7648

Review URL: https://codereview.chromium.org/74523002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240773 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 112 insertions, 7 deletions

diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc
index 767819e..80c83e8 100644
--- a/content/browser/download/download_browsertest.cc
+++ b/content/browser/download/download_browsertest.cc
@@ -1125,12 +1125,12 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
       switches::kEnableDownloadResumption);
   ASSERT_TRUE(test_server()->Start());
 
-  GURL url = test_server()->GetURL(
-      base::StringPrintf(
-          // First download hits an RST, rest don't, precondition fail.
-          "rangereset?size=%d&rst_boundary=%d&"
-          "token=NoRange&rst_limit=1&fail_precondition=2",
-          GetSafeBufferChunk() * 3, GetSafeBufferChunk()));
+  GURL url = test_server()->GetURL(base::StringPrintf(
+      // First download hits an RST, rest don't, precondition fail.
+      "rangereset?size=%d&rst_boundary=%d&"
+      "token=BadPrecondition&rst_limit=1&fail_precondition=2",
+      GetSafeBufferChunk() * 3,
+      GetSafeBufferChunk()));
 
   // Start the download and wait for first data chunk.
   DownloadItem* download(StartDownloadAndReturnItem(url));
@@ -1142,6 +1142,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
   ConfirmFileStatusForResume(
       download, true, GetSafeBufferChunk(), GetSafeBufferChunk() * 3,
       base::FilePath(FILE_PATH_LITERAL("rangereset.crdownload")));
+  EXPECT_EQ("BadPrecondition2", download->GetETag());
 
   DownloadUpdatedObserver completion_observer(
       download, base::Bind(DownloadCompleteFilter));
@@ -1151,6 +1152,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest,
   ConfirmFileStatusForResume(
       download, true, GetSafeBufferChunk() * 3, GetSafeBufferChunk() * 3,
       base::FilePath(FILE_PATH_LITERAL("rangereset")));
+  EXPECT_EQ("BadPrecondition0", download->GetETag());
 
   static const RecordingDownloadObserver::RecordStruct expected_record[] = {
     // Result of RST
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index 507dfe1..eead7f6 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -896,6 +896,50 @@ DownloadItemImpl::ResumeMode DownloadItemImpl::GetResumeMode() const {
   return mode;
 }
 
+void DownloadItemImpl::MergeOriginInfoOnResume(
+    const DownloadCreateInfo& new_create_info) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(RESUMING_INTERNAL, state_);
+  DCHECK(!new_create_info.url_chain.empty());
+
+  // We are going to tack on any new redirects to our list of redirects.
+  // When a download is resumed, the URL used for the resumption request is the
+  // one at the end of the previous redirect chain. Tacking additional redirects
+  // to the end of this chain ensures that:
+  // - If the download needs to be resumed again, the ETag/Last-Modified headers
+  //   will be used with the last server that sent them to us.
+  // - The redirect chain contains all the servers that were involved in this
+  //   download since the initial request, in order.
+  std::vector<GURL>::const_iterator chain_iter =
+      new_create_info.url_chain.begin();
+  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)
+    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);
+
+  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;
+
+  // Don't update observers. This method is expected to be called just before a
+  // DownloadFile is created and Start() is called. The observers will be
+  // notified when the download transitions to the IN_PROGRESS state.
+}
+
 void DownloadItemImpl::NotifyRemoved() {
   FOR_EACH_OBSERVER(Observer, observers_, OnDownloadRemoved(this));
 }
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h
index 2086a54..804afa6 100644
--- a/content/browser/download/download_item_impl.h
+++ b/content/browser/download/download_item_impl.h
@@ -163,6 +163,11 @@ class CONTENT_EXPORT DownloadItemImpl
   // INTERRUPTED state.
   virtual ResumeMode GetResumeMode() const;
 
+  // Notify the download item that new origin information is available due to a
+  // resumption request receiving a response.
+  virtual void MergeOriginInfoOnResume(
+      const DownloadCreateInfo& new_create_info);
+
   // State transition operations on regular downloads --------------------------
 
   // Start the download.
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 35825d3..4f2e68a 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -398,6 +398,7 @@ void DownloadManagerImpl::StartDownloadWithId(
     }
     download = item_iterator->second;
     DCHECK_EQ(DownloadItem::INTERRUPTED, download->GetState());
+    download->MergeOriginInfoOnResume(*info);
   }
 
   base::FilePath default_download_directory;
diff --git a/content/browser/download/download_stats.cc b/content/browser/download/download_stats.cc
index df3a059..b6e3ba8 100644
--- a/content/browser/download/download_stats.cc
+++ b/content/browser/download/download_stats.cc
@@ -611,4 +611,14 @@ void RecordSavePackageEvent(SavePackageEvent event) {
                             SAVE_PACKAGE_LAST_ENTRY);
 }
 
+void RecordOriginStateOnResumption(bool is_partial,
+                                   int state) {
+  if (is_partial)
+    UMA_HISTOGRAM_ENUMERATION("Download.OriginStateOnPartialResumption", state,
+                              ORIGIN_STATE_ON_RESUMPTION_MAX);
+  else
+    UMA_HISTOGRAM_ENUMERATION("Download.OriginStateOnFullResumption", state,
+                              ORIGIN_STATE_ON_RESUMPTION_MAX);
+}
+
 }  // namespace content
diff --git a/content/browser/download/download_stats.h b/content/browser/download/download_stats.h
index b1307d3..0e59884 100644
--- a/content/browser/download/download_stats.h
+++ b/content/browser/download/download_stats.h
@@ -218,6 +218,19 @@ enum SavePackageEvent {
 
 void RecordSavePackageEvent(SavePackageEvent event);
 
+enum OriginStateOnResumption {
+  ORIGIN_STATE_ON_RESUMPTION_ADDITIONAL_REDIRECTS = 1<<0,
+  ORIGIN_STATE_ON_RESUMPTION_VALIDATORS_CHANGED = 1<<1,
+  ORIGIN_STATE_ON_RESUMPTION_CONTENT_DISPOSITION_CHANGED = 1<<2,
+  ORIGIN_STATE_ON_RESUMPTION_MAX = 1<<3
+};
+
+// Record the state of the origin information across a download resumption
+// request. |state| is a combination of values from OriginStateOnResumption
+// enum.
+void RecordOriginStateOnResumption(bool is_partial,
+                                   int state);
+
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_STATS_H_
diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py
index f50dd0c..085619f 100755
--- a/net/tools/testserver/testserver.py
+++ b/net/tools/testserver/testserver.py
@@ -1573,7 +1573,10 @@ class TestPageHandler(testserver_base.BasePageHandler):
     self.send_header('Content-Type', 'application/octet-stream')
     self.send_header('Content-Length', last_byte - first_byte + 1)
     if send_verifiers:
-      self.send_header('Etag', '"XYZZY"')
+      # If fail_precondition is non-zero, then the ETag for each request will be
+      # different.
+      etag = "%s%d" % (token, fail_precondition)
+      self.send_header('ETag', etag)
       self.send_header('Last-Modified', 'Tue, 19 Feb 2013 14:32 EST')
     self.end_headers()
 
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 1a2c6d1..53c78d1 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -3209,6 +3209,22 @@ other types of suffix sets.
   </summary>
 </histogram>
 
+<histogram name="Download.OriginStateOnFullResumption"
+    enum="DownloadOriginStateOnResumption">
+  <summary>
+    Changes observed when a response is received for a full download resumption
+    request.
+  </summary>
+</histogram>
+
+<histogram name="Download.OriginStateOnPartialResumption"
+    enum="DownloadOriginStateOnResumption">
+  <summary>
+    Changes observed when a response is received for a partial (byte-range)
+    download resumption request.
+  </summary>
+</histogram>
+
 <histogram name="Download.PotentialBandwidth" units="Bytes/second">
   <summary>
     The maximum bandwidth (per read) that Chrome could have provided for the
@@ -22794,6 +22810,17 @@ other types of suffix sets.
   <int value="2" label="Opened with plaform handler by user choice"/>
 </enum>
 
+<enum name="DownloadOriginStateOnResumption" type="int">
+  <int value="0" label="No changes"/>
+  <int value="1" label="New redirects"/>
+  <int value="2" label="New validators"/>
+  <int value="3" label="New redirects + validators"/>
+  <int value="4" label="New Content-Disposition"/>
+  <int value="5" label="New redirects + Content-Disposition"/>
+  <int value="6" label="New validators + Content-Disposition"/>
+  <int value="7" label="New redirects + validators + Content-Disposition"/>
+</enum>
+
 <enum name="DownloadSavePackageEvent" type="int">
   <int value="0" label="Started"/>
   <int value="1" label="Cancelled"/>