[Downloads] Always call DM::StartDownload() for explicit downloads.

content/browser/download/download_browsertest.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.
 
 // This file contains download browser tests that are known to be runnable
 // in a pure content context.  Over time tests should be migrated here.
 
 #include <stddef.h>
 #include <stdint.h>
 #include <utility>
@@ skipping 11 matching lines @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/browser/byte_stream.h"
 #include "content/browser/download/download_file_factory.h"
 #include "content/browser/download/download_file_impl.h"
 #include "content/browser/download/download_item_impl.h"
 #include "content/browser/download/download_manager_impl.h"
 #include "content/browser/download/download_resource_handler.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/power_save_blocker.h"
+#include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include "content/public/browser/resource_throttle.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/webplugininfo.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/download_test_observer.h"
 #include "content/public/test/test_download_request_handler.h"
 #include "content/public/test/test_file_error_injector.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
@@ skipping 67 matching lines @@
 static DownloadManagerImpl* DownloadManagerForShell(Shell* shell) {
   // We're in a content_browsertest; we know that the DownloadManager
   // is a DownloadManagerImpl.
   return static_cast<DownloadManagerImpl*>(
       BrowserContext::GetDownloadManager(
           shell->web_contents()->GetBrowserContext()));
 }
 
 class DownloadFileWithDelay : public DownloadFileImpl {
  public:
-  DownloadFileWithDelay(
-      scoped_ptr<DownloadSaveInfo> save_info,
-      const base::FilePath& default_download_directory,
-      const GURL& url,
-      const GURL& referrer_url,
-      bool calculate_hash,
-      scoped_ptr<ByteStreamReader> stream,
-      const net::BoundNetLog& bound_net_log,
-      scoped_ptr<PowerSaveBlocker> power_save_blocker,
-      base::WeakPtr<DownloadDestinationObserver> observer,
-      base::WeakPtr<DownloadFileWithDelayFactory> owner);
+  DownloadFileWithDelay(const DownloadSaveInfo& save_info,
+                        const base::FilePath& default_download_directory,
+                        const GURL& url,
+                        const GURL& referrer_url,
+                        bool calculate_hash,
+                        base::File file,
+                        scoped_ptr<ByteStreamReader> stream,
+                        const net::BoundNetLog& bound_net_log,
+                        scoped_ptr<PowerSaveBlocker> power_save_blocker,
+                        base::WeakPtr<DownloadDestinationObserver> observer,
+                        base::WeakPtr<DownloadFileWithDelayFactory> owner);
 
   ~DownloadFileWithDelay() override;
 
   // Wraps DownloadFileImpl::Rename* and intercepts the return callback,
   // storing it in the factory that produced this object for later
   // retrieval.
   void RenameAndUniquify(const base::FilePath& full_path,
                          const RenameCompletionCallback& callback) override;
   void RenameAndAnnotate(const base::FilePath& full_path,
                          const RenameCompletionCallback& callback) override;
@@ skipping 16 matching lines @@
 };
 
 // All routines on this class must be called on the UI thread.
 class DownloadFileWithDelayFactory : public DownloadFileFactory {
  public:
   DownloadFileWithDelayFactory();
   ~DownloadFileWithDelayFactory() override;
 
   // DownloadFileFactory interface.
   DownloadFile* CreateFile(
-      scoped_ptr<DownloadSaveInfo> save_info,
+      const DownloadSaveInfo& save_info,
       const base::FilePath& default_download_directory,
       const GURL& url,
       const GURL& referrer_url,
       bool calculate_hash,
+      base::File file,
       scoped_ptr<ByteStreamReader> stream,
       const net::BoundNetLog& bound_net_log,
       base::WeakPtr<DownloadDestinationObserver> observer) override;
 
   void AddRenameCallback(base::Closure callback);
   void GetAllRenameCallbacks(std::vector<base::Closure>* results);
 
   // Do not return until GetAllRenameCallbacks() will return a non-empty list.
   void WaitForSomeCallback();
 
  private:
   std::vector<base::Closure> rename_callbacks_;
   bool waiting_;
   base::WeakPtrFactory<DownloadFileWithDelayFactory> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadFileWithDelayFactory);
 };
 
 DownloadFileWithDelay::DownloadFileWithDelay(
-    scoped_ptr<DownloadSaveInfo> save_info,
+    const DownloadSaveInfo& save_info,
     const base::FilePath& default_download_directory,
     const GURL& url,
     const GURL& referrer_url,
     bool calculate_hash,
+    base::File file,
     scoped_ptr<ByteStreamReader> stream,
     const net::BoundNetLog& bound_net_log,
     scoped_ptr<PowerSaveBlocker> power_save_blocker,
     base::WeakPtr<DownloadDestinationObserver> observer,
     base::WeakPtr<DownloadFileWithDelayFactory> owner)
-    : DownloadFileImpl(std::move(save_info),
+    : DownloadFileImpl(save_info,
                        default_download_directory,
                        url,
                        referrer_url,
                        calculate_hash,
+                       std::move(file),
                        std::move(stream),
                        bound_net_log,
                        observer),
       owner_(owner) {}
 
 DownloadFileWithDelay::~DownloadFileWithDelay() {}
 
 void DownloadFileWithDelay::RenameAndUniquify(
     const base::FilePath& full_path,
     const RenameCompletionCallback& callback) {
@@ skipping 23 matching lines @@
   factory->AddRenameCallback(base::Bind(original_callback, reason, path));
 }
 
 DownloadFileWithDelayFactory::DownloadFileWithDelayFactory()
     : waiting_(false),
       weak_ptr_factory_(this) {}
 
 DownloadFileWithDelayFactory::~DownloadFileWithDelayFactory() {}
 
 DownloadFile* DownloadFileWithDelayFactory::CreateFile(
-    scoped_ptr<DownloadSaveInfo> save_info,
+    const DownloadSaveInfo& save_info,
     const base::FilePath& default_download_directory,
     const GURL& url,
     const GURL& referrer_url,
     bool calculate_hash,
+    base::File file,
     scoped_ptr<ByteStreamReader> stream,
     const net::BoundNetLog& bound_net_log,
     base::WeakPtr<DownloadDestinationObserver> observer) {
   scoped_ptr<PowerSaveBlocker> psb(PowerSaveBlocker::Create(
       PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension,
       PowerSaveBlocker::kReasonOther, "Download in progress"));
   return new DownloadFileWithDelay(
-      std::move(save_info), default_download_directory, url, referrer_url,
-      calculate_hash, std::move(stream), bound_net_log, std::move(psb),
+      save_info, default_download_directory, url, referrer_url, calculate_hash,
+      std::move(file), std::move(stream), bound_net_log, std::move(psb),
       observer, weak_ptr_factory_.GetWeakPtr());
 }
 
 void DownloadFileWithDelayFactory::AddRenameCallback(base::Closure callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   rename_callbacks_.push_back(callback);
   if (waiting_)
     base::MessageLoopForUI::current()->QuitWhenIdle();
 }
 
 void DownloadFileWithDelayFactory::GetAllRenameCallbacks(
     std::vector<base::Closure>* results) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   results->swap(rename_callbacks_);
 }
 
 void DownloadFileWithDelayFactory::WaitForSomeCallback() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (rename_callbacks_.empty()) {
     waiting_ = true;
     RunMessageLoop();
     waiting_ = false;
   }
 }
 
 class CountingDownloadFile : public DownloadFileImpl {
  public:
-  CountingDownloadFile(scoped_ptr<DownloadSaveInfo> save_info,
+  CountingDownloadFile(const DownloadSaveInfo& save_info,
                        const base::FilePath& default_downloads_directory,
                        const GURL& url,
                        const GURL& referrer_url,
                        bool calculate_hash,
+                       base::File file,
                        scoped_ptr<ByteStreamReader> stream,
                        const net::BoundNetLog& bound_net_log,
                        scoped_ptr<PowerSaveBlocker> power_save_blocker,
                        base::WeakPtr<DownloadDestinationObserver> observer)
-      : DownloadFileImpl(std::move(save_info),
+      : DownloadFileImpl(save_info,
                          default_downloads_directory,
                          url,
                          referrer_url,
                          calculate_hash,
+                         std::move(file),
                          std::move(stream),
                          bound_net_log,
                          observer) {}
 
   ~CountingDownloadFile() override {
     DCHECK_CURRENTLY_ON(BrowserThread::FILE);
     active_files_--;
   }
 
   void Initialize(const InitializeCallback& callback) override {
@@ skipping 26 matching lines @@
 
 int CountingDownloadFile::active_files_ = 0;
 
 class CountingDownloadFileFactory : public DownloadFileFactory {
  public:
   CountingDownloadFileFactory() {}
   ~CountingDownloadFileFactory() override {}
 
   // DownloadFileFactory interface.
   DownloadFile* CreateFile(
-      scoped_ptr<DownloadSaveInfo> save_info,
+      const DownloadSaveInfo& save_info,
       const base::FilePath& default_downloads_directory,
       const GURL& url,
       const GURL& referrer_url,
       bool calculate_hash,
+      base::File file,
       scoped_ptr<ByteStreamReader> stream,
       const net::BoundNetLog& bound_net_log,
       base::WeakPtr<DownloadDestinationObserver> observer) override {
     scoped_ptr<PowerSaveBlocker> psb(PowerSaveBlocker::Create(
         PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension,
         PowerSaveBlocker::kReasonOther, "Download in progress"));
-    return new CountingDownloadFile(
-        std::move(save_info), default_downloads_directory, url, referrer_url,
-        calculate_hash, std::move(stream), bound_net_log, std::move(psb),
-        observer);
+    return new CountingDownloadFile(save_info, default_downloads_directory, url,
+                                    referrer_url, calculate_hash,
+                                    std::move(file), std::move(stream),
+                                    bound_net_log, std::move(psb), observer);
   }
 };
 
 class TestShellDownloadManagerDelegate : public ShellDownloadManagerDelegate {
  public:
   TestShellDownloadManagerDelegate()
       : delay_download_open_(false) {}
   ~TestShellDownloadManagerDelegate() override {}
 
   bool ShouldOpenDownload(
@@ skipping 218 matching lines @@
         download, base::Bind(&IsDownloadInState, DownloadItem::IN_PROGRESS))
         .WaitForEvent();
   }
 
   void WaitForCompletion(DownloadItem* download) {
     DownloadUpdatedObserver(
         download, base::Bind(&IsDownloadInState, DownloadItem::COMPLETE))
         .WaitForEvent();
   }
 
+  void WaitForCancel(DownloadItem* download) {
+    DownloadUpdatedObserver(
+        download, base::Bind(&IsDownloadInState, DownloadItem::CANCELLED))
+        .WaitForEvent();
+  }
+
   // Note: Cannot be used with other alternative DownloadFileFactorys
   void SetupEnsureNoPendingDownloads() {
     DownloadManagerForShell(shell())->SetDownloadFileFactoryForTesting(
         scoped_ptr<DownloadFileFactory>(new CountingDownloadFileFactory()));
   }
 
   bool EnsureNoPendingDownloads() {
     bool result = true;
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
@@ skipping 501 matching lines @@
   ASSERT_TRUE(requests[1].request_headers.GetHeader(
       net::HttpRequestHeaders::kIfRange, &value));
   EXPECT_EQ(parameters.etag, value);
 
   ASSERT_TRUE(requests[1].request_headers.GetHeader(
       net::HttpRequestHeaders::kRange, &value));
   EXPECT_EQ(base::StringPrintf("bytes=%" PRId64 "-", interruption.offset),
             value);
 }
 
+// Resumption should only attempt to contact the final URL if the download has a
+// URL chain.
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, RedirectBeforeResume) {
+  TestDownloadRequestHandler request_handler_1(
+      GURL("http://example.com/first-url"));
+  request_handler_1.StartServingStaticResponse(
+      "HTTP/1.1 302 Redirect\r\n"
+      "Location: http://example.com/second-url\r\n"
+      "\r\n");
+
+  TestDownloadRequestHandler request_handler_2(
+      GURL("http://example.com/second-url"));
+  request_handler_2.StartServingStaticResponse(
+      "HTTP/1.1 302 Redirect\r\n"
+      "Location: http://example.com/third-url\r\n"
+      "\r\n");
+
+  TestDownloadRequestHandler request_handler_3(
+      GURL("http://example.com/third-url"));
+  request_handler_3.StartServingStaticResponse(
+      "HTTP/1.1 302 Redirect\r\n"
+      "Location: http://example.com/download\r\n"
+      "\r\n");
+
+  TestDownloadRequestHandler resumable_request_handler(
+      GURL("http://example.com/download"));
+  TestDownloadRequestHandler::Parameters parameters =
+      TestDownloadRequestHandler::Parameters::WithSingleInterruption();
+  resumable_request_handler.StartServing(parameters);
+
+  DownloadItem* download = StartDownloadAndReturnItem(
+      initiator_shell_for_resumption(), request_handler_1.url());
+  WaitForInterrupt(download);
+
+  EXPECT_EQ(4u, download->GetUrlChain().size());
+  EXPECT_EQ(request_handler_1.url(), download->GetOriginalUrl());
+  EXPECT_EQ(resumable_request_handler.url(), download->GetURL());
+
+  // Now that the download is interrupted, make all intermediate servers return
+  // a 404. The only way a resumption request would succeed if the resumption
+  // request is sent to the final server in the chain.
+  const char k404Response[] = "HTTP/1.1 404 Not found\r\n\r\n";
+  request_handler_1.StartServingStaticResponse(k404Response);
+  request_handler_2.StartServingStaticResponse(k404Response);
+  request_handler_3.StartServingStaticResponse(k404Response);
+
+  PrepareToResume();
+  download->Resume();
+  WaitForCompletion(download);
+
+  ASSERT_NO_FATAL_FAILURE(ReadAndVerifyFileContents(
+      parameters.pattern_generator_seed, parameters.size,
+      download->GetTargetFilePath()));
+}
+
+// If the server response for the resumption request specifies a bad range (i.e.
+// not the range that was requested or an invalid or missing Content-Range
+// header), then the download should be marked as interrupted again without
+// discarding the partial state.
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, BadRangeHeader) {
+  TestDownloadRequestHandler request_handler;
+  TestDownloadRequestHandler::Parameters parameters =
+      TestDownloadRequestHandler::Parameters::WithSingleInterruption();
+  request_handler.StartServing(parameters);
+
+  DownloadItem* download = StartDownloadAndReturnItem(
+      initiator_shell_for_resumption(), request_handler.url());
+  WaitForInterrupt(download);
+
+  // Upon resumption, the server starts responding with a bad range header.
+  request_handler.StartServingStaticResponse(
+      "HTTP/1.1 206 Partial Content\r\n"
+      "Content-Range: bytes 1000000-2000000/3000000\r\n"
+      "\r\n");
+  PrepareToResume();
+  download->Resume();
+  WaitForInterrupt(download);
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+            download->GetLastReason());
+
+  // Or this time, the server sends a response with an invalid Content-Range
+  // header.
+  request_handler.StartServingStaticResponse(
+      "HTTP/1.1 206 Partial Content\r\n"
+      "Content-Range: ooga-booga-booga-booga\r\n"
+      "\r\n");
+  download->Resume();
+  WaitForInterrupt(download);
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+            download->GetLastReason());
+
+  // Or no Content-Range header at all.
+  request_handler.StartServingStaticResponse(
+      "HTTP/1.1 206 Partial Content\r\n"
+      "Some-Headers: ooga-booga-booga-booga\r\n"
+      "\r\n");
+  download->Resume();
+  WaitForInterrupt(download);
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+            download->GetLastReason());
+
+  // Back to the original request handler. Resumption should now succeed, and
+  // use the partial data it had prior to the first interruption.
+  request_handler.StartServing(parameters);
+  download->Resume();
+  WaitForCompletion(download);
+
+  ASSERT_EQ(parameters.size, download->GetReceivedBytes());
+  ASSERT_EQ(parameters.size, download->GetTotalBytes());
+  ASSERT_NO_FATAL_FAILURE(ReadAndVerifyFileContents(
+      parameters.pattern_generator_seed, parameters.size,
+      download->GetTargetFilePath()));
+
+  // Characterization risk: The next portion of the test examines the requests
+  // that were sent out while downloading our resource. These requests
+  // correspond to the requests that were generated by the browser and the
+  // downloads system and may change as implementation details change.
+  TestDownloadRequestHandler::CompletedRequests requests;
+  request_handler.GetCompletedRequestInfo(&requests);
+
+  ASSERT_EQ(5u, requests.size());
+
+  // None of the request should have transferred the entire resource.
+  EXPECT_GT(parameters.size, requests[0].transferred_byte_count);
+  EXPECT_EQ(0, requests[1].transferred_byte_count);
+  EXPECT_EQ(0, requests[2].transferred_byte_count);
+  EXPECT_EQ(0, requests[3].transferred_byte_count);
+  EXPECT_GT(parameters.size, requests[4].transferred_byte_count);
+}
+
 // A partial resumption results in an HTTP 200 response. I.e. the server ignored
 // the range request and sent the entire resource instead. For If-Range requests
 // (as opposed to If-Match), the behavior for a precondition failure is also to
 // respond with a 200. So this test case covers both validation failure and
 // ignoring the range request.
 IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest,
                        RestartIfNotPartialResponse) {
   const int kOriginalPatternGeneratorSeed = 1;
   const int kNewPatternGeneratorSeed = 2;
 
@@ skipping 146 matching lines @@
 
   // Start and watch for interrupt.
   DownloadItem* download(StartDownloadAndReturnItem(
       initiator_shell_for_resumption(), request_handler.url()));
   WaitForInterrupt(download);
   ASSERT_EQ(DownloadItem::INTERRUPTED, download->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
             download->GetLastReason());
   EXPECT_EQ(0, download->GetReceivedBytes());
   EXPECT_TRUE(download->GetFullPath().empty());
-  EXPECT_TRUE(download->GetTargetFilePath().empty());
+  EXPECT_FALSE(download->GetTargetFilePath().empty());
 
   // We need to make sure that any cross-thread downloads communication has
   // quiesced before clearing and injecting the new errors, as the
   // InjectErrors() routine alters the currently in use download file
   // factory, which is a file thread object.
   RunAllPendingInMessageLoop(BrowserThread::FILE);
   RunAllPendingInMessageLoop();
 
   // Clear the old errors list.
   injector->ClearErrors();
@@ skipping 524 matching lines @@
   std::vector<DownloadItem*> downloads;
   DownloadManagerForShell(shell())->GetAllDownloads(&downloads);
   ASSERT_EQ(1u, downloads.size());
 
   EXPECT_EQ(FILE_PATH_LITERAL("suggested-filename"),
             downloads[0]->GetTargetFilePath().BaseName().value());
   ASSERT_TRUE(origin_one.ShutdownAndWaitUntilComplete());
   ASSERT_TRUE(origin_two.ShutdownAndWaitUntilComplete());
 }
 
+// A request for a non-existent resource should still result in a DownloadItem
+// that's created in an interrupted state.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeServerError) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL download_url =
+      embedded_test_server()->GetURL("/download/does-not-exist");
+  GURL document_url = embedded_test_server()->GetURL(
+      std::string("/download/download-attribute.html?target=") +
+      download_url.spec());
+
+  DownloadItem* download = StartDownloadAndReturnItem(shell(), document_url);
+  WaitForInterrupt(download);
+
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT,
+            download->GetLastReason());
+}
+
+namespace {
+
+void ErrorReturningRequestHandler(
+    const net::HttpRequestHeaders& headers,
+    const TestDownloadRequestHandler::OnStartResponseCallback& callback) {
+  callback.Run(std::string(), net::ERR_INTERNET_DISCONNECTED);
+}
+
+}  // namespace
+
+// A request that fails before it gets a response from the server should also
+// result in a DownloadItem that's created in an interrupted state.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeNetworkError) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  TestDownloadRequestHandler request_handler;
+  TestDownloadRequestHandler::Parameters parameters;
+
+  parameters.on_start_handler = base::Bind(&ErrorReturningRequestHandler);
+  request_handler.StartServing(parameters);
+
+  GURL document_url = embedded_test_server()->GetURL(
+      std::string("/download/download-attribute.html?target=") +
+      request_handler.url().spec());
+  DownloadItem* download = StartDownloadAndReturnItem(shell(), document_url);
+  WaitForInterrupt(download);
+
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED,
+            download->GetLastReason());
+}
+
+// A request that fails due to it being rejected by policy should result in a
+// DownloadItem that's marked as interrupted.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadAttributeInvalidURL) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL document_url = embedded_test_server()->GetURL(
+      "/download/download-attribute.html?target=about:version");
+  DownloadItem* download = StartDownloadAndReturnItem(shell(), document_url);
+  WaitForInterrupt(download);
+
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NETWORK_INVALID_REQUEST,
+            download->GetLastReason());
+  EXPECT_FALSE(download->CanResume());
+}
+
 // The file empty.bin is served with a MIME type of application/octet-stream.
 // The content body is empty. Make sure this case is handled properly and we
 // don't regress on http://crbug.com/320394.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadGZipWithNoContent) {
   GURL url = net::URLRequestMockHTTPJob::GetMockUrl("empty.bin");
   NavigateToURLAndWaitForDownload(shell(), url, DownloadItem::COMPLETE);
   // That's it. This should work without crashing.
 }
 
 // Make sure that sniffed MIME types are correctly passed through to the
 // download item.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, SniffedMimeType) {
   GURL url = net::URLRequestMockHTTPJob::GetMockUrl("gzip-content.gz");
   DownloadItem* item = StartDownloadAndReturnItem(shell(), url);
   WaitForCompletion(item);
 
   EXPECT_STREQ("application/x-gzip", item->GetMimeType().c_str());
   EXPECT_TRUE(item->GetOriginalMimeType().empty());
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, Spam) {
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, DuplicateContentDisposition) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  // double-content-disposition.txt is served with two Content-Disposition
+  // headers, both of which are identical.
   NavigateToURLAndWaitForDownload(
       shell(),
       embedded_test_server()->GetURL(
           "/download/double-content-disposition.txt"),
       DownloadItem::COMPLETE);
 
   std::vector<DownloadItem*> downloads;
   DownloadManagerForShell(shell())->GetAllDownloads(&downloads);
   ASSERT_EQ(1u, downloads.size());
 
   EXPECT_EQ(FILE_PATH_LITERAL("Jumboshrimp.txt"),
             downloads[0]->GetTargetFilePath().BaseName().value());
 }
 
 }  // namespace content