Range request support for parallel download in DownloadRequestCore.

content/browser/download/download_request_core.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.
 
 #include "content/browser/download/download_request_core.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 140 matching lines @@
     else
       load_flags |= net::LOAD_SKIP_CACHE_VALIDATION;
   } else {
     load_flags |= net::LOAD_DISABLE_CACHE;
   }
   request->SetLoadFlags(load_flags);
 
   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);
+  // Strong validator is required for download resumption.
+  DCHECK(params->offset() == 0 || params->length() > 0 || has_etag ||

[qinmin, 2017/01/31 19:25:56]

I don't think this DCHECK is right.

The original DCHECK checks is purely for resumption: if the download is a
resumption, then has_etag || has_last_modified is true. Otherwise,
params->offset() == 0.

When dealing with parallel download, you should have a separate DCHECK:
DCHECK(param->length() == DownloadSaveInfo::kLengthUnknown || has_etag ||
has_last_modified);

[xingliu, 2017/01/31 21:32:25]

Done, this is something I didn't consider that strong validator ensures requests
are working on the same file.

Merged the two DCHECK into one.

+         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)) {
+  // Add "Range" and "If-Range" request header fields for download resumption
+  // with strong validator.
+  if (params->offset() > 0 &&
+      params->length() == DownloadSaveInfo::kLengthUnknown &&
+      (has_etag || has_last_modified)) {
     request->SetExtraRequestHeaderByName(
         "Range", base::StringPrintf("bytes=%" PRId64 "-", params->offset()),
         true);
 
     // In accordance with RFC 2616 Section 14.27, 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.
     //
     // This function assumes that HasStrongValidators() was true and that the
     // ETag and Last-Modified header values supplied are valid.
     request->SetExtraRequestHeaderByName(
         "If-Range", has_etag ? params->etag() : params->last_modified(), true);
   }
 
+  // Add "Range" request header for new download if length is specified.
+  if (params->length() > 0) {

[qinmin, 2017/01/31 19:25:56]

you need to add has_etag || has_last_modified condition here. If there is no
strong validator, we cannot ensure parallel download jobs are downloading the
same content

[xingliu, 2017/01/31 21:32:25]

Done.

+    request->SetExtraRequestHeaderByName(
+        "Range",
+        base::StringPrintf("bytes=%" PRId64 "-%" PRId64, params->offset(),
+                           params->offset() + params->length() - 1),
+        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.
   request->set_first_party_for_cookies(params->url());
   request->set_first_party_url_policy(
       net::URLRequest::UPDATE_FIRST_PARTY_URL_ON_REDIRECT);
   request->set_initiator(params->initiator());
 
   for (const auto& header : params->request_headers())
     request->SetExtraRequestHeaderByName(header.first, header.second,
@@ skipping 385 matching lines @@
       return DOWNLOAD_INTERRUPT_REASON_SERVER_FORBIDDEN;
       break;
     default:  // All other errors.
       // Redirection and informational codes should have been handled earlier
       // in the stack.
       DCHECK_NE(3, http_headers.response_code() / 100);
       DCHECK_NE(1, http_headers.response_code() / 100);
       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.
       save_info->offset = 0;
+      save_info->length = DownloadSaveInfo::kLengthUnknown;
       save_info->hash_of_partial_file.clear();
       save_info->hash_state.reset();
       return DOWNLOAD_INTERRUPT_REASON_NONE;
     }
 
     int64_t first_byte = -1;
     int64_t last_byte = -1;
     int64_t length = -1;
     if (!http_headers.GetContentRangeFor206(&first_byte, &last_byte, &length))
       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.
       //
       // In the future we should consider allowing offsets that are less than
       // the offset we've requested, since in theory we can truncate the partial
       // file at the offset and continue.
       return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
     }
 
     return DOWNLOAD_INTERRUPT_REASON_NONE;
   }
 
   if (http_headers.response_code() == net::HTTP_PARTIAL_CONTENT)
     return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
 
   return DOWNLOAD_INTERRUPT_REASON_NONE;
 }
 
 }  // namespace content