Range request support for parallel download in DownloadRequestCore.

content/browser/download/download_request_core_unittest.cc

Index: content/browser/download/download_request_core_unittest.cc
diff --git a/content/browser/download/download_request_core_unittest.cc b/content/browser/download/download_request_core_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..84c2594c1b32720341f9a0393e6d9d0ec17ec24d
--- /dev/null
+++ b/content/browser/download/download_request_core_unittest.cc
@@ -0,0 +1,141 @@
+// Copyright 2017 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 <memory>
+
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
+#include "content/browser/download/download_item_impl.h"
+#include "content/browser/download/download_request_core.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/download_url_parameters.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "net/http/http_request_headers.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+class DownloadRequestCoreTest : public testing::Test {
+ public:
+  DownloadRequestCoreTest() = default;
+  ~DownloadRequestCoreTest() override = default;

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

Nit: you should be able to leave these out and achieve the same effect.

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

Done.

+
+  std::unique_ptr<DownloadUrlParameters> BuildDownloadParameters(
+      const std::string& url) const {
+    GURL gurl(url);
+    return base::MakeUnique<DownloadUrlParameters>(
+        gurl, request_context_getter_.get());
+  }
+
+  void CheckRequestHeaders(const std::string& name,
+                           const std::string& expected_header_value) const {

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

Nit/suggestion: Introduce a way to verify that the header doesn't exist at all
rather than existing with an empty value.

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

Done.

+    DCHECK(url_request_.get());
+    std::string header_value;
+    url_request_->extra_request_headers().GetHeader(name, &header_value);
+    EXPECT_EQ(expected_header_value, header_value);
+  }
+
+  void CreateRequestOnIOThread(DownloadUrlParameters* params) {

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

Since you are using TestBrowserThreadBundle without requesting real threads, you
should be able to call DownloadRequestCore::CreateRequestOnIOThread directly
without doing a post task.

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

This is awesome, thanks.

+    run_loop_.reset(new base::RunLoop());
+
+    // Run on Browser IO thread since there is a thread check in
+    // DownloadRequestCore.
+    BrowserThread::PostTaskAndReplyWithResult(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&DownloadRequestCore::CreateRequestOnIOThread,
+                   DownloadItem::kInvalidId, params),
+        base::Bind(&DownloadRequestCoreTest::OnRequestCreated,
+                   base::Unretained(this)));
+
+    // RunLoop blocks the main test thread (currently Browser UI thread) until
+    // the |url_request_| is created and reply back to the main test thread.
+    run_loop_->Run();
+  }
+
+  void OnRequestCreated(std::unique_ptr<net::URLRequest> url_request) {
+    url_request_ = std::move(url_request);
+    DCHECK(url_request_.get());
+    run_loop_->Quit();
+  }
+
+  void SetUp() override {
+    request_context_getter_ = new net::TestURLRequestContextGetter(
+        content::BrowserThread::GetTaskRunnerForThread(
+            content::BrowserThread::UI));
+  }
+
+  void TearDown() override {
+    // URLRequest must be released before |request_context_getter_| gets
+    // destroyed.
+    url_request_.reset();
+  }
+
+  std::unique_ptr<base::RunLoop> run_loop_;
+  std::unique_ptr<net::URLRequest> url_request_;
+
+  // Used to test functions run on particular browser thread.
+  content::TestBrowserThreadBundle browser_threads_;
+  scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
+};
+
+// Ensure "Range" header is built correctly for normal download.
+TEST_F(DownloadRequestCoreTest, BuildRangeRequest) {
+  std::unique_ptr<DownloadUrlParameters> params =
+      BuildDownloadParameters("something.com");

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

Nit: Use example.com for examples and tests.

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

Done.

+
+  // Check initial states.
+  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent, params->length());
+  EXPECT_EQ(0, params->offset());
+
+  // Non-range request.
+  CreateRequestOnIOThread(params.get());
+  CheckRequestHeaders(net::HttpRequestHeaders::kRange, "");
+  CheckRequestHeaders(net::HttpRequestHeaders::kIfRange, "");
+  CheckRequestHeaders("If-Match", "");
+  CheckRequestHeaders("If-Unmodified-Since", "");
+  url_request_.reset();
+
+  // Range request with header "Range:bytes=50-99".
+  // Requires strong validator.
+  params->set_etag("123");
+  params->set_offset(50);
+  params->set_length(50);
+  CreateRequestOnIOThread(params.get());
+  CheckRequestHeaders(net::HttpRequestHeaders::kRange, "bytes=50-99");
+  CheckRequestHeaders(net::HttpRequestHeaders::kIfRange, "");
+  CheckRequestHeaders("If-Match", "123");
+  CheckRequestHeaders("If-Unmodified-Since", "");
+  url_request_.reset();
+
+  // Range request with header "Range:bytes=0-49".
+  // Requires strong validator.
+  params->set_etag("123");
+  params->set_offset(0);
+  params->set_length(50);
+  CreateRequestOnIOThread(params.get());
+  CheckRequestHeaders(net::HttpRequestHeaders::kRange, "bytes=0-49");
+  CheckRequestHeaders(net::HttpRequestHeaders::kIfRange, "");
+  CheckRequestHeaders("If-Match", "123");
+  CheckRequestHeaders("If-Unmodified-Since", "");
+  url_request_.reset();
+}
+
+// Ensure "Range" header is built correctly for download resumption.
+// Notice download resumption requires strong validator(i.e. etag or
+// last-modified).
+TEST_F(DownloadRequestCoreTest, BuildRangeRequestWithoutLength) {
+  std::unique_ptr<DownloadUrlParameters> params =
+      BuildDownloadParameters("something.com");
+  params->set_etag("123");
+  params->set_offset(50);
+  CreateRequestOnIOThread(params.get());
+  CheckRequestHeaders(net::HttpRequestHeaders::kRange, "bytes=50-");
+  CheckRequestHeaders(net::HttpRequestHeaders::kIfRange, "123");
+  CheckRequestHeaders("If-Match", "");
+  CheckRequestHeaders("If-Unmodified-Since", "");
+  url_request_.reset();
+}

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

Add a test case with both an etag and a last-modified header?

Should only affect the length capped test cases.

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

Done.

+
+}  // namespace content