Test both download resumption code paths.

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 <vector>
 
 #include "base/callback_helpers.h"
@@ skipping 625 matching lines @@
 
     // Check the contents.
     EXPECT_EQ(value, file_contents);
     if (memcmp(file_contents.c_str(), value.c_str(), expected_size) != 0)
       return false;
 
     return true;
   }
 
   // Start a download and return the item.
-  DownloadItem* StartDownloadAndReturnItem(GURL url) {
+  DownloadItem* StartDownloadAndReturnItem(Shell* shell, GURL url) {
     scoped_ptr<DownloadCreateObserver> observer(
-        new DownloadCreateObserver(DownloadManagerForShell(shell())));
-    shell()->LoadURL(url);
+        new DownloadCreateObserver(DownloadManagerForShell(shell)));
+    shell->LoadURL(url);
     return observer->WaitForFinished();
   }
 
   static void ReadAndVerifyFileContents(int seed,
                                         int64_t expected_size,
                                         const base::FilePath& path) {
     base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
     ASSERT_TRUE(file.IsValid());
     int64_t file_length = file.GetLength();
     ASSERT_EQ(expected_size, file_length);
@@ skipping 23 matching lines @@
       *result = false;
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                             base::MessageLoop::QuitWhenIdleClosure());
   }
 
   // Location of the downloads directory for these tests
   base::ScopedTempDir downloads_directory_;
   scoped_ptr<TestShellDownloadManagerDelegate> test_delegate_;
 };
 
+// Parameters for DownloadResumptionContentTest.
+enum class DownloadResumptionTestType {
+  RESUME_WITH_RENDERER,  // Resume() is called while the originating WebContents
+                         // is still alive.
+  RESUME_WITHOUT_RENDERER  // Resume() is called after the originating
+                           // WebContents has been deleted.
+};
+
+// Parameterized test for download resumption. Tests using this fixure will be
+// run once with RESUME_WITH_RENDERER and once with RESUME_WITHOUT_RENDERER.
+// Use download_shell() to retrieve the Shell object that should be used as the
+// originator for the initial download. Prior to calling Resume(), call
+// PrepareToResume() which will cause the originating Shell to be deleted if the
+// test parameter is RESUME_WITHOUT_RENDERER.
+class DownloadResumptionContentTest
+    : public DownloadContentTest,
+      public ::testing::WithParamInterface<DownloadResumptionTestType> {
+ public:
+  void SetUpOnMainThread() override {
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kEnableDownloadResumption);
+    DownloadContentTest::SetUpOnMainThread();
+
+    if (GetParam() == DownloadResumptionTestType::RESUME_WITHOUT_RENDERER)
+      download_shell_ = CreateBrowser();
+    else
+      download_shell_ = shell();
+
+    ASSERT_EQ(DownloadManagerForShell(shell()),
+              DownloadManagerForShell(download_shell()));
+  }
+
+  // Shell to use for initiating a download. Only valid *before*
+  // PrepareToResume() is called.
+  Shell* download_shell() const {
+    DCHECK(download_shell_);
+    return download_shell_;
+  }
+
+  // Should be called once before calling DownloadItem::Resume() on an
+  // interrupted download. This may cause download_shell() to become
+  // invalidated.
+  void PrepareToResume() {
+    if (GetParam() == DownloadResumptionTestType::RESUME_WITH_RENDERER)
+      return;
+    DCHECK_NE(download_shell(), shell());
+    DCHECK(download_shell());
+    download_shell_->Close();
+    download_shell_ = nullptr;
+  }
+
+ private:
+  Shell* download_shell_ = nullptr;
+};
+
+INSTANTIATE_TEST_CASE_P(
+    _,
+    DownloadResumptionContentTest,
+    ::testing::Values(DownloadResumptionTestType::RESUME_WITH_RENDERER,

[svaldez, 2015/12/14 21:18:15]

Is the :: at the beginning necessary?

[asanka, 2015/12/14 21:31:02]

Not strictly necessary, but it's common for the testing namespace to use them
since it's a namespace defined by a third party library and shouldn't conflict
with any secondary namespaces that are also named 'testing'. See line 55 etc.

[svaldez, 2015/12/14 22:01:36]

Acknowledged.

+                      DownloadResumptionTestType::RESUME_WITHOUT_RENDERER));
+
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadCancelled) {
   SetupEnsureNoPendingDownloads();
 
   // Create a download, wait until it's started, and confirm
   // we're in the expected state.
   DownloadItem* download = StartDownloadAndReturnItem(
-      GURL(net::URLRequestSlowDownloadJob::kUnknownSizeUrl));
+      shell(), GURL(net::URLRequestSlowDownloadJob::kUnknownSizeUrl));
   ASSERT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
 
   // Cancel the download and wait for download system quiesce.
   download->Cancel(true);
   scoped_refptr<DownloadTestFlushObserver> flush_observer(
       new DownloadTestFlushObserver(DownloadManagerForShell(shell())));
   flush_observer->WaitForFlush();
 
   // Get the important info from other threads and check it.
   EXPECT_TRUE(EnsureNoPendingDownloads());
 }
 
 // Check that downloading multiple (in this case, 2) files does not result in
 // corrupted files.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, MultiDownload) {
   SetupEnsureNoPendingDownloads();
 
   // Create a download, wait until it's started, and confirm
   // we're in the expected state.
   DownloadItem* download1 = StartDownloadAndReturnItem(
-      GURL(net::URLRequestSlowDownloadJob::kUnknownSizeUrl));
+      shell(), GURL(net::URLRequestSlowDownloadJob::kUnknownSizeUrl));
   ASSERT_EQ(DownloadItem::IN_PROGRESS, download1->GetState());
 
   // Start the second download and wait until it's done.
   GURL url(net::URLRequestMockHTTPJob::GetMockUrl("download-test.lib"));
-  DownloadItem* download2 = StartDownloadAndReturnItem(url);
+  DownloadItem* download2 = StartDownloadAndReturnItem(shell(), url);
   WaitForCompletion(download2);
 
   ASSERT_EQ(DownloadItem::IN_PROGRESS, download1->GetState());
   ASSERT_EQ(DownloadItem::COMPLETE, download2->GetState());
 
   // Allow the first request to finish.
   scoped_ptr<DownloadTestObserver> observer2(CreateWaiter(shell(), 1));
   NavigateToURL(shell(),
                 GURL(net::URLRequestSlowDownloadJob::kFinishDownloadUrl));
   observer2->WaitForFinished();  // Wait for the third request.
@@ skipping 145 matching lines @@
 #define MAYBE_ShutdownInProgress DISABLED_ShutdownInProgress
 #else
 #define MAYBE_ShutdownInProgress ShutdownInProgress
 #endif
 
 // Try to shutdown with a download in progress to make sure shutdown path
 // works properly.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, MAYBE_ShutdownInProgress) {
   // Create a download that won't complete.
   DownloadItem* download = StartDownloadAndReturnItem(
-      GURL(net::URLRequestSlowDownloadJob::kUnknownSizeUrl));
+      shell(), GURL(net::URLRequestSlowDownloadJob::kUnknownSizeUrl));
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
 
   // Shutdown the download manager and make sure we get the right
   // notifications in the right order.
   StrictMock<MockDownloadItemObserver> item_observer;
   download->AddObserver(&item_observer);
   MockDownloadManagerObserver manager_observer(
       DownloadManagerForShell(shell()));
   // Don't care about ModelChanged() events.
@@ skipping 79 matching lines @@
   MockDownloadItemObserver observer;
   items[0]->AddObserver(&observer);
   EXPECT_CALL(observer, OnDownloadDestroyed(items[0]));
 
   // Shutdown the download manager.  Mostly this is confirming a lack of
   // crashes.
   DownloadManagerForShell(shell())->Shutdown();
 }
 
 // Test resumption with a response that contains strong validators.
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, Resume_WithStrongValidators) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
-
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, StrongValidators) {
   TestDownloadRequestHandler request_handler;
   TestDownloadRequestHandler::Parameters parameters =
       TestDownloadRequestHandler::Parameters::WithSingleInterruption();
   const TestDownloadRequestHandler::InjectedError interruption =
       parameters.injected_errors.front();
   request_handler.StartServing(parameters);
 
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   ASSERT_EQ(interruption.offset, download->GetReceivedBytes());
   ASSERT_EQ(parameters.size, download->GetTotalBytes());
 
+  PrepareToResume();
   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
@@ skipping 22 matching lines @@
       net::HttpRequestHeaders::kRange, &value));
   EXPECT_EQ(base::StringPrintf("bytes=%" PRId64 "-", interruption.offset),
             value);
 }
 
 // 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_F(DownloadContentTest,
-                       Resume_RestartIfNotPartialResponse) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest,
+                       RestartIfNotPartialResponse) {
   const int kOriginalPatternGeneratorSeed = 1;
   const int kNewPatternGeneratorSeed = 2;
 
   TestDownloadRequestHandler::Parameters parameters =
       TestDownloadRequestHandler::Parameters::WithSingleInterruption();
   parameters.pattern_generator_seed = kOriginalPatternGeneratorSeed;
   const TestDownloadRequestHandler::InjectedError interruption =
       parameters.injected_errors.front();
 
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(parameters);
 
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   ASSERT_EQ(interruption.offset, download->GetReceivedBytes());
   ASSERT_EQ(parameters.size, download->GetTotalBytes());
 
   parameters = TestDownloadRequestHandler::Parameters();
   parameters.support_byte_ranges = false;
   parameters.pattern_generator_seed = kNewPatternGeneratorSeed;
   request_handler.StartServing(parameters);
 
+  PrepareToResume();
   download->Resume();
   WaitForCompletion(download);
 
   ASSERT_EQ(parameters.size, download->GetReceivedBytes());
   ASSERT_EQ(parameters.size, download->GetTotalBytes());
   ASSERT_NO_FATAL_FAILURE(
       ReadAndVerifyFileContents(kNewPatternGeneratorSeed, parameters.size,
                                 download->GetTargetFilePath()));
 
   // When the downloads system sees the full response, it should accept the
@@ skipping 17 matching lines @@
       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);
 }
 
 // Confirm we restart if we don't have a verifier.
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, Resume_RestartIfNoETag) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, RestartIfNoETag) {
   const int kOriginalPatternGeneratorSeed = 1;
   const int kNewPatternGeneratorSeed = 2;
 
   TestDownloadRequestHandler::Parameters parameters =
       TestDownloadRequestHandler::Parameters::WithSingleInterruption();
   ASSERT_EQ(1u, parameters.injected_errors.size());
   parameters.etag.clear();
   parameters.pattern_generator_seed = kOriginalPatternGeneratorSeed;
 
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(parameters);
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   parameters.pattern_generator_seed = kNewPatternGeneratorSeed;
   parameters.ClearInjectedErrors();
   request_handler.StartServing(parameters);
 
+  PrepareToResume();
   download->Resume();
   WaitForCompletion(download);
 
   ASSERT_EQ(parameters.size, download->GetReceivedBytes());
   ASSERT_EQ(parameters.size, download->GetTotalBytes());
   ASSERT_NO_FATAL_FAILURE(
       ReadAndVerifyFileContents(kNewPatternGeneratorSeed, parameters.size,
                                 download->GetTargetFilePath()));
 
   TestDownloadRequestHandler::CompletedRequests requests;
   request_handler.GetCompletedRequestInfo(&requests);
 
   // Neither If-Range nor Range headers should be present in the second request.
   ASSERT_EQ(2u, requests.size());
   std::string value;
   EXPECT_FALSE(requests[1].request_headers.GetHeader(
       net::HttpRequestHeaders::kIfRange, &value));
   EXPECT_FALSE(requests[1].request_headers.GetHeader(
       net::HttpRequestHeaders::kRange, &value));
 }
 
 // Partial file goes missing before the download is resumed. The download should
 // restart.
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, Resume_RestartIfNoPartialFile) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, RestartIfNoPartialFile) {
   TestDownloadRequestHandler::Parameters parameters =
       TestDownloadRequestHandler::Parameters::WithSingleInterruption();
 
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(parameters);
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   // Delete the intermediate file.
   ASSERT_TRUE(base::PathExists(download->GetFullPath()));
   ASSERT_TRUE(base::DeleteFile(download->GetFullPath(), false));
 
   parameters.ClearInjectedErrors();
   request_handler.StartServing(parameters);
 
+  PrepareToResume();
   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()));
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, Resume_RecoverFromInitFileError) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest,
+                       RecoverFromInitFileError) {
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(TestDownloadRequestHandler::Parameters());
 
   // Setup the error injector.
   scoped_refptr<TestFileErrorInjector> injector(
-      TestFileErrorInjector::Create(DownloadManagerForShell(shell())));
+      TestFileErrorInjector::Create(DownloadManagerForShell(download_shell())));
 
   const TestFileErrorInjector::FileErrorInfo err = {
       request_handler.url().spec(),
       TestFileErrorInjector::FILE_OPERATION_INITIALIZE, 0,
       DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE};
   injector->AddError(err);
   injector->InjectErrors();
 
   // Start and watch for interrupt.
-  DownloadItem* download(StartDownloadAndReturnItem(request_handler.url()));
+  DownloadItem* download(
+      StartDownloadAndReturnItem(download_shell(), 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());
 
   // 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();
   injector->InjectErrors();
 
   // Resume and watch completion.
+  PrepareToResume();
   download->Resume();
   WaitForCompletion(download);
   EXPECT_EQ(download->GetState(), DownloadItem::COMPLETE);
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadContentTest,
-                       Resume_RecoverFromIntermediateFileRenameError) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest,
+                       RecoverFromIntermediateFileRenameError) {
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(TestDownloadRequestHandler::Parameters());
 
   // Setup the error injector.
   scoped_refptr<TestFileErrorInjector> injector(
-      TestFileErrorInjector::Create(DownloadManagerForShell(shell())));
+      TestFileErrorInjector::Create(DownloadManagerForShell(download_shell())));
 
   const TestFileErrorInjector::FileErrorInfo err = {
       request_handler.url().spec(),
       TestFileErrorInjector::FILE_OPERATION_RENAME_UNIQUIFY, 0,
       DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE};
   injector->AddError(err);
   injector->InjectErrors();
 
   // Start and watch for interrupt.
-  DownloadItem* download(StartDownloadAndReturnItem(request_handler.url()));
+  DownloadItem* download(
+      StartDownloadAndReturnItem(download_shell(), request_handler.url()));
   WaitForInterrupt(download);
   ASSERT_EQ(DownloadItem::INTERRUPTED, download->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
             download->GetLastReason());
   EXPECT_TRUE(download->GetFullPath().empty());
   // Target path will have been set after file name determination. GetFullPath()
   // being empty is sufficient to signal that filename determination needs to be
   // redone.
   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();
   injector->InjectErrors();
 
+  PrepareToResume();
   download->Resume();
   WaitForCompletion(download);
   EXPECT_EQ(download->GetState(), DownloadItem::COMPLETE);
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadContentTest,
-                       Resume_RecoverFromFinalRenameError) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest,
+                       RecoverFromFinalRenameError) {
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(TestDownloadRequestHandler::Parameters());
 
   // Setup the error injector.
   scoped_refptr<TestFileErrorInjector> injector(
-      TestFileErrorInjector::Create(DownloadManagerForShell(shell())));
+      TestFileErrorInjector::Create(DownloadManagerForShell(download_shell())));
 
-  DownloadManagerForShell(shell())->RemoveAllDownloads();
+  DownloadManagerForShell(download_shell())->RemoveAllDownloads();
   TestFileErrorInjector::FileErrorInfo err = {
       request_handler.url().spec(),
       TestFileErrorInjector::FILE_OPERATION_RENAME_ANNOTATE, 0,
       DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE};
   injector->AddError(err);
   injector->InjectErrors();
 
   // Start and watch for interrupt.
-  DownloadItem* download(StartDownloadAndReturnItem(request_handler.url()));
+  DownloadItem* download(
+      StartDownloadAndReturnItem(download_shell(), request_handler.url()));
   WaitForInterrupt(download);
   ASSERT_EQ(DownloadItem::INTERRUPTED, download->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE,
             download->GetLastReason());
   EXPECT_TRUE(download->GetFullPath().empty());
   // Target path should still be intact.
   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();
   injector->InjectErrors();
 
+  PrepareToResume();
   download->Resume();
   WaitForCompletion(download);
   EXPECT_EQ(download->GetState(), DownloadItem::COMPLETE);
 }
 
 // An interrupted download should remove the intermediate file when it is
 // cancelled.
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, CancelInterruptedDownload) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest,
+                       CancelInterruptedDownload) {
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(
       TestDownloadRequestHandler::Parameters::WithSingleInterruption());
 
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   base::FilePath intermediate_path = download->GetFullPath();
   ASSERT_FALSE(intermediate_path.empty());
   ASSERT_TRUE(base::PathExists(intermediate_path));
 
   download->Cancel(true /* user_cancel */);
   RunAllPendingInMessageLoop(BrowserThread::FILE);
   RunAllPendingInMessageLoop();
 
   // The intermediate file should now be gone.
   EXPECT_FALSE(base::PathExists(intermediate_path));
   EXPECT_TRUE(download->GetFullPath().empty());
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, RemoveInterruptedDownload) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest,
+                       RemoveInterruptedDownload) {
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(
       TestDownloadRequestHandler::Parameters::WithSingleInterruption());
 
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   base::FilePath intermediate_path = download->GetFullPath();
   ASSERT_FALSE(intermediate_path.empty());
   ASSERT_TRUE(base::PathExists(intermediate_path));
 
   download->Remove();
   RunAllPendingInMessageLoop(BrowserThread::FILE);
   RunAllPendingInMessageLoop();
 
   // The intermediate file should now be gone.
   EXPECT_FALSE(base::PathExists(intermediate_path));
 }
 
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, RemoveCompletedDownload) {
   // A completed download shouldn't delete the downloaded file when it is
   // removed.
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(TestDownloadRequestHandler::Parameters());
   scoped_ptr<DownloadTestObserver> completion_observer(
       CreateWaiter(shell(), 1));

[svaldez, 2015/12/14 21:18:15]

download_shell()?

[asanka, 2015/12/14 21:31:02]

This isn't a resumption test, and hence doesn't need to distinguish between an
initiator shell and the shell provided by the outer test fixture.

It is unfortunately sandwiched between two resumption tests, but I didn't move
it in this CL. I can move this out if you think it's hurting readability.

[svaldez, 2015/12/14 22:01:36]

Ah, okay. Might be better to move it out from between the resumption tests, but
I don't think it hurts readability too much if we just leave it here.

-  DownloadItem* download(StartDownloadAndReturnItem(request_handler.url()));
+  DownloadItem* download(
+      StartDownloadAndReturnItem(shell(), request_handler.url()));

[svaldez, 2015/12/14 21:18:15]

download_shell()?

[asanka, 2015/12/14 21:31:02]

Ditto.

[svaldez, 2015/12/14 22:01:36]

Acknowledged.

   completion_observer->WaitForFinished();
 
   // The target path should exist.
   base::FilePath target_path(download->GetTargetFilePath());
   EXPECT_TRUE(base::PathExists(target_path));
   download->Remove();
   RunAllPendingInMessageLoop(BrowserThread::FILE);
   RunAllPendingInMessageLoop();
 
   // The file should still exist.
   EXPECT_TRUE(base::PathExists(target_path));
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, RemoveResumingDownload) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, RemoveResumingDownload) {
   TestDownloadRequestHandler::Parameters parameters =
       TestDownloadRequestHandler::Parameters::WithSingleInterruption();
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(parameters);
 
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   base::FilePath intermediate_path(download->GetFullPath());
   ASSERT_FALSE(intermediate_path.empty());
   EXPECT_TRUE(base::PathExists(intermediate_path));
 
   // Resume and remove download. We expect only a single OnDownloadCreated()
   // call, and that's for the second download created below.
-  MockDownloadManagerObserver dm_observer(DownloadManagerForShell(shell()));
+  MockDownloadManagerObserver dm_observer(
+      DownloadManagerForShell(download_shell()));
   EXPECT_CALL(dm_observer, OnDownloadCreated(_,_)).Times(1);
 
   TestRequestStartHandler request_start_handler;
   parameters.on_start_handler = request_start_handler.GetOnStartHandler();
   request_handler.StartServing(parameters);
 
+  PrepareToResume();
   download->Resume();
   request_start_handler.WaitForCallback();
 
   // At this point, the download resumption request has been sent out, but the
   // reponse hasn't been received yet.
   download->Remove();
 
   request_start_handler.RespondWith(std::string(), net::OK);
 
   // The intermediate file should now be gone.
   RunAllPendingInMessageLoop(BrowserThread::FILE);
   RunAllPendingInMessageLoop();
   EXPECT_FALSE(base::PathExists(intermediate_path));
 
   parameters.ClearInjectedErrors();
   parameters.on_start_handler.Reset();
   request_handler.StartServing(parameters);
 
   // Start the second download and wait until it's done. This exercises the
   // entire downloads stack and effectively flushes all of our worker threads.
   // We are testing whether the URL request created in the previous
   // DownloadItem::Resume() call reulted in a new download or not.
   NavigateToURLAndWaitForDownload(shell(), request_handler.url(),

[svaldez, 2015/12/14 21:18:15]

download_shell()?

[asanka, 2015/12/14 21:31:02]

This call is intentionally using shell() instead of download_shell() since its
usage doesn't need a transient initiator shell.

Also, once PrepareToResume() is called download_shell() may no longer exist.

[svaldez, 2015/12/14 22:01:36]

Acknowledged.

                                   DownloadItem::COMPLETE);
   EXPECT_TRUE(EnsureNoPendingDownloads());
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadContentTest, CancelResumingDownload) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kEnableDownloadResumption);
+IN_PROC_BROWSER_TEST_P(DownloadResumptionContentTest, CancelResumingDownload) {
   TestDownloadRequestHandler::Parameters parameters =
       TestDownloadRequestHandler::Parameters::WithSingleInterruption();
   TestDownloadRequestHandler request_handler;
   request_handler.StartServing(parameters);
 
-  DownloadItem* download = StartDownloadAndReturnItem(request_handler.url());
+  DownloadItem* download =
+      StartDownloadAndReturnItem(download_shell(), request_handler.url());
   WaitForInterrupt(download);
 
   base::FilePath intermediate_path(download->GetFullPath());
   ASSERT_FALSE(intermediate_path.empty());
   EXPECT_TRUE(base::PathExists(intermediate_path));
 
   // Resume and remove download. We expect only a single OnDownloadCreated()
   // call, and that's for the second download created below.
-  MockDownloadManagerObserver dm_observer(DownloadManagerForShell(shell()));
+  MockDownloadManagerObserver dm_observer(
+      DownloadManagerForShell(download_shell()));
   EXPECT_CALL(dm_observer, OnDownloadCreated(_,_)).Times(1);
 
   TestRequestStartHandler request_start_handler;
   parameters.on_start_handler = request_start_handler.GetOnStartHandler();
   request_handler.StartServing(parameters);
 
+  PrepareToResume();
   download->Resume();
   request_start_handler.WaitForCallback();
 
   // At this point, the download item has initiated a network request for the
   // resumption attempt, but hasn't received a response yet.
   download->Cancel(true /* user_cancel */);
 
   request_start_handler.RespondWith(std::string(), net::OK);
 
   // The intermediate file should now be gone.
@@ skipping 171 matching lines @@
 
   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