Range request support for parallel download in DownloadRequestCore.

content/browser/download/download_request_core.cc

Index: content/browser/download/download_request_core.cc
diff --git a/content/browser/download/download_request_core.cc b/content/browser/download/download_request_core.cc
index 0fec3f3b8e5730fd3f2b16884f2ca87a9d4e819c..f1b690e35824865d8865f3711de537f965147198 100644
--- a/content/browser/download/download_request_core.cc
+++ b/content/browser/download/download_request_core.cc
@@ -37,6 +37,7 @@
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/upload_bytes_element_reader.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_request_context.h"
@@ -158,19 +159,23 @@ std::unique_ptr<net::URLRequest> DownloadRequestCore::CreateRequestOnIOThread(
   bool has_last_modified = !params->last_modified().empty();
   bool has_etag = !params->etag().empty();
 
-  // If we've asked for a range, we want to make sure that we only get that
-  // range if our current copy of the information is good.  We shouldn't be
-  // asked to continue if we don't have a verifier.
-  DCHECK(params->offset() == 0 || has_etag || has_last_modified);
-
-  // If we're not at the beginning of the file, retrieve only the remaining
-  // portion.
-  if (params->offset() > 0 && (has_etag || has_last_modified)) {
+  // Strong validator(i.e. etag or last modified) is required in range requests
+  // for download resumption and parallel download.
+  DCHECK((params->offset() == 0 &&
+          params->length() == DownloadSaveInfo::kLengthFullContent) ||
+         has_etag || has_last_modified);
+
+  // Add "Range" and "If-Range" request header fields if the range request is to
+  // retrieve bytes from {offset} to the end of the file.
+  // E.g. "Range:bytes=50-".
+  if (params->offset() > 0 &&
+      params->length() == DownloadSaveInfo::kLengthFullContent &&
+      (has_etag || has_last_modified)) {
     request->SetExtraRequestHeaderByName(
-        "Range", base::StringPrintf("bytes=%" PRId64 "-", params->offset()),
-        true);
+        net::HttpRequestHeaders::kRange,
+        base::StringPrintf("bytes=%" PRId64 "-", params->offset()), true);
 
-    // In accordance with RFC 2616 Section 14.27, use If-Range to specify that
+    // In accordance with RFC 7233 Section 3.2, use If-Range to specify that
     // the server return the entire entity if the validator doesn't match.
     // Last-Modified can be used in the absence of ETag as a validator if the
     // response headers satisfied the HttpUtil::HasStrongValidators() predicate.
@@ -181,6 +186,26 @@ std::unique_ptr<net::URLRequest> DownloadRequestCore::CreateRequestOnIOThread(
         "If-Range", has_etag ? params->etag() : params->last_modified(), true);
   }
 
+  // Add "Range", "If-Match", and "If-Unmodified-Since" for range requests with
+  // last byte position specified. e.g. "Range:bytes=50-59". If the file is
+  // updated on the server, we should get http 412 response code.
+  if (params->length() != DownloadSaveInfo::kLengthFullContent &&
+      (has_etag || has_last_modified)) {
+    request->SetExtraRequestHeaderByName(
+        net::HttpRequestHeaders::kRange,
+        base::StringPrintf("bytes=%" PRId64 "-%" PRId64, params->offset(),
+                           params->offset() + params->length() - 1),
+        true);
+    if (has_etag)
+      request->SetExtraRequestHeaderByName("If-Match", params->etag(), true);

[asanka, 2017/02/03 22:16:22]

Could you add "If-Match"/"If-Unmodified-Since" to HttpRequestHeaders? I can
stamp that change along with this one.

Then you can use the new constants here and in tests.

[xingliu, 2017/02/06 19:31:39]

Done.

+    // According to RFC 7232 section 3.4, "If-Unmodified-Since" is mainly for
+    // old servers that didn't implement "If-Match" and must be ignored when
+    // "If-Match" presents.
+    if (has_last_modified)
+      request->SetExtraRequestHeaderByName("If-Unmodified-Since",
+                                           params->last_modified(), true);
+  }
+
   // Downloads are treated as top level navigations. Hence the first-party
   // origin for cookies is always based on the target URL and is updated on
   // redirects.
@@ -304,7 +329,7 @@ bool DownloadRequestCore::OnResponseStarted(
   const net::HttpResponseHeaders* headers = request()->response_headers();
   if (headers) {
     if (headers->HasStrongValidators()) {
-      // If we don't have strong validators as per RFC 2616 section 13.3.3, then
+      // If we don't have strong validators as per RFC 7232 section 2, then
       // we neither store nor use them for range requests.
       if (!headers->EnumerateHeader(nullptr, "Last-Modified",
                                     &create_info->last_modified))
@@ -546,7 +571,7 @@ DownloadInterruptReason DownloadRequestCore::HandleSuccessfulServerResponse(
 
     case net::HTTP_CREATED:
     case net::HTTP_ACCEPTED:
-      // Per RFC 2616 the entity being transferred is metadata about the
+      // Per RFC 7231 the entity being transferred is metadata about the
       // resource at the target URL and not the resource at that URL (or the
       // resource that would be at the URL once processing is completed in the
       // case of HTTP_ACCEPTED). However, we currently don't have special
@@ -556,7 +581,7 @@ DownloadInterruptReason DownloadRequestCore::HandleSuccessfulServerResponse(
 
     case net::HTTP_NO_CONTENT:
     case net::HTTP_RESET_CONTENT:
-    // These two status codes don't have an entity (or rather RFC 2616
+    // These two status codes don't have an entity (or rather RFC 7231
     // requires that there be no entity). They are treated the same as the
     // resource not being found since there is no entity to download.
 
@@ -586,11 +611,16 @@ DownloadInterruptReason DownloadRequestCore::HandleSuccessfulServerResponse(
       return DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED;
   }
 
-  if (save_info && save_info->offset > 0) {
-    // The caller is expecting a partial response.
-
+  // The caller is expecting a partial response.
+  if (save_info && (save_info->offset > 0 || save_info->length > 0)) {
     if (http_headers.response_code() != net::HTTP_PARTIAL_CONTENT) {
-      // Requested a partial range, but received the entire response.
+      // Server should send partial content when we ask for a range with last
+      // byte position specified. e.g. "Range:bytes=50-99".

[asanka, 2017/02/03 22:16:22]

Nit: This is a result of the If-Match/If-Unmodified-Since + Range headers.

[xingliu, 2017/02/06 19:31:40]

Done.

+      if (save_info->length != DownloadSaveInfo::kLengthFullContent)
+        return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
+
+      // Requested a partial range, but received the entire response, when
+      // the range header is "Range:bytes={offset}-".
       save_info->offset = 0;
       save_info->hash_of_partial_file.clear();
       save_info->hash_state.reset();
@@ -604,7 +634,9 @@ DownloadInterruptReason DownloadRequestCore::HandleSuccessfulServerResponse(
       return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
     DCHECK_GE(first_byte, 0);
 
-    if (first_byte != save_info->offset) {
+    if (first_byte != save_info->offset ||
+        (save_info->length > 0 &&
+         last_byte != save_info->offset + save_info->length - 1)) {
       // The server returned a different range than the one we requested. Assume
       // the response is bad.
       //