Downloads: replace BrowserThread::FILE with task scheduler.

content/browser/download/download_item_impl_unittest.cc

Index: content/browser/download/download_item_impl_unittest.cc
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index 4431bdd4c00e0156058c3ef310ad24456a4398d3..e72f97e7f0e8173e7e838a254bb3aa3bf602fbd8 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -21,6 +21,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"

[gab, 2017/07/20 16:54:31]

Maybe can get rid of run_loop.h, message_loop.h and browser_thread.h?

[Sigurður Ásgeirsson, 2017/07/20 17:23:21]

BrowserThread::UI still makes an appearance.

+#include "base/test/scoped_task_environment.h"
 #include "base/threading/thread.h"
 #include "content/browser/byte_stream.h"
 #include "content/browser/download/download_create_info.h"
@@ -36,6 +37,7 @@
 #include "content/public/test/mock_download_item.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
 #include "content/public/test/web_contents_tester.h"
 #include "crypto/secure_hash.h"
 #include "net/http/http_response_headers.h"
@@ -258,7 +260,12 @@ const uint8_t kHashOfTestData1[] = {
 class DownloadItemTest : public testing::Test {
  public:
   DownloadItemTest()
-      : delegate_(), next_download_id_(DownloadItem::kInvalidId + 1) {
+      : task_environment_(
+            base::test::ScopedTaskEnvironment::MainThreadType::UI,
+            base::test::ScopedTaskEnvironment::ExecutionMode::QUEUED),
+        mock_delegate_(new StrictMock<MockDelegate>),
+        next_download_id_(DownloadItem::kInvalidId + 1),
+        browser_context_(new TestBrowserContext) {
     create_info_.reset(new DownloadCreateInfo());
     create_info_->save_info =
         std::unique_ptr<DownloadSaveInfo>(new DownloadSaveInfo());
@@ -267,15 +274,11 @@ class DownloadItemTest : public testing::Test {
     create_info_->etag = "SomethingToSatisfyResumption";
   }
 
-  ~DownloadItemTest() {
-    RunAllPendingInMessageLoops();
-  }
-
   DownloadItemImpl* CreateDownloadItemWithCreateInfo(
       std::unique_ptr<DownloadCreateInfo> info) {
     DownloadItemImpl* download =
-        new DownloadItemImpl(&delegate_, next_download_id_++, *(info.get()),
-                             net::NetLogWithSource());
+        new DownloadItemImpl(mock_delegate(), next_download_id_++,
+                             *(info.get()), net::NetLogWithSource());
     allocated_downloads_[download] = base::WrapUnique(download);
     return download;
   }
@@ -293,7 +296,7 @@ class DownloadItemTest : public testing::Test {
   DownloadItemImpl* CreateDownloadItem() {
     create_info_->download_id = ++next_download_id_;
     DownloadItemImpl* download =
-        new DownloadItemImpl(&delegate_, create_info_->download_id,
+        new DownloadItemImpl(mock_delegate(), create_info_->download_id,
                              *create_info_, net::NetLogWithSource());
     allocated_downloads_[download] = base::WrapUnique(download);
     return download;
@@ -321,7 +324,7 @@ class DownloadItemTest : public testing::Test {
         base::MakeUnique<NiceMock<MockRequestHandle>>();
     item->Start(std::move(download_file), std::move(request_handle),
                 *create_info_);
-    RunAllPendingInMessageLoops();
+    task_environment_.RunUntilIdle();
 
     // So that we don't have a function writing to a stack variable
     // lying around if the above failed.
@@ -353,7 +356,7 @@ class DownloadItemTest : public testing::Test {
     callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                  danger_type, intermediate_path,
                  DOWNLOAD_INTERRUPT_REASON_NONE);
-    RunAllPendingInMessageLoops();
+    task_environment_.RunUntilIdle();
     return download_file;
   }
 
@@ -371,7 +374,7 @@ class DownloadItemTest : public testing::Test {
 
     item->DestinationObserverAsWeakPtr()->DestinationCompleted(
         0, std::unique_ptr<crypto::SecureHash>());
-    RunAllPendingInMessageLoops();
+    task_environment_.RunUntilIdle();
   }
 
   // Cleanup a download item (specifically get rid of the DownloadFile on it).
@@ -385,7 +388,7 @@ class DownloadItemTest : public testing::Test {
       if (download_file)
         EXPECT_CALL(*download_file, Cancel());
       item->Cancel(true);
-      RunAllPendingInMessageLoops();
+      task_environment_.RunUntilIdle();
     }
   }
 
@@ -394,11 +397,7 @@ class DownloadItemTest : public testing::Test {
     allocated_downloads_.erase(item);
   }
 
-  void RunAllPendingInMessageLoops() { base::RunLoop().RunUntilIdle(); }
-
-  MockDelegate* mock_delegate() {
-    return &delegate_;
-  }
+  MockDelegate* mock_delegate() { return mock_delegate_.get(); }
 
   void OnDownloadFileAcquired(base::FilePath* return_path,
                               const base::FilePath& path) {
@@ -407,15 +406,17 @@ class DownloadItemTest : public testing::Test {
 
   DownloadCreateInfo* create_info() { return create_info_.get(); }
 
-  BrowserContext* browser_context() { return &browser_context_; }
+  BrowserContext* browser_context() { return browser_context_.get(); }
+
+  base::test::ScopedTaskEnvironment task_environment_;
 
  private:
   TestBrowserThreadBundle thread_bundle_;
-  StrictMock<MockDelegate> delegate_;
+  std::unique_ptr<StrictMock<MockDelegate>> mock_delegate_;
   std::map<DownloadItem*, std::unique_ptr<DownloadItem>> allocated_downloads_;
   std::unique_ptr<DownloadCreateInfo> create_info_;
   uint32_t next_download_id_ = DownloadItem::kInvalidId + 1;
-  TestBrowserContext browser_context_;
+  std::unique_ptr<TestBrowserContext> browser_context_;

[gab, 2017/07/20 16:54:31]

Don't think you need the unique_ptr and explicit construction change anymore now
that we got rid of need for explicit destruction order in a precursor CL.

[Sigurður Ásgeirsson, 2017/07/20 17:23:21]

Done.

 };
 
 // Tests to ensure calls that change a DownloadItem generate an update to
@@ -591,7 +592,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnDownloadTargetDetermined) {
                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
                DOWNLOAD_INTERRUPT_REASON_NONE);
   EXPECT_FALSE(observer.CheckAndResetDownloadUpdated());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
   EXPECT_EQ(new_intermediate_path, item->GetFullPath());
 
@@ -619,7 +620,7 @@ TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
   item->Resume();
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
 
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS);
 }
@@ -662,7 +663,7 @@ TEST_F(DownloadItemTest, AutomaticResumption_Continue) {
   // the mock doesn't follow through with the resumption.
   // ResumeInterruptedDownload() being called is sufficient for verifying that
   // the automatic resumption was triggered.
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   // The download item is currently in RESUMING_INTERNAL state, which maps to
   // IN_PROGRESS.
@@ -696,7 +697,7 @@ TEST_F(DownloadItemTest, AutomaticResumption_Restart) {
   // Since the download is resumed automatically, the interrupt count doesn't
   // increase.
   ASSERT_EQ(0, observer.interrupt_count());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
 }
@@ -718,7 +719,7 @@ TEST_F(DownloadItemTest, AutomaticResumption_NeedsUserAction) {
   // Should not try to auto-resume.
   ASSERT_EQ(1, observer.interrupt_count());
   ASSERT_EQ(0, observer.resume_count());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   CleanupItem(item, nullptr, DownloadItem::INTERRUPTED);
 }
@@ -757,7 +758,7 @@ TEST_F(DownloadItemTest, AutomaticResumption_ContentLengthMismatch) {
   ASSERT_EQ(0, observer.interrupt_count());
   ASSERT_EQ(0, observer.resume_count());
 
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
 }
 
@@ -780,7 +781,7 @@ TEST_F(DownloadItemTest, UnresumableInterrupt) {
   // Complete download to trigger final rename.
   item->DestinationObserverAsWeakPtr()->DestinationCompleted(
       0, std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   // Should not try to auto-resume.
@@ -826,7 +827,7 @@ TEST_F(DownloadItemTest, AutomaticResumption_AttemptLimit) {
     // to allow for holding onto the request handle.
     item->Start(std::move(mock_download_file), std::move(mock_request_handle),
                 *create_info());
-    RunAllPendingInMessageLoops();
+    task_environment_.RunUntilIdle();
 
     base::FilePath target_path(kDummyTargetPath);
     base::FilePath intermediate_path(kDummyIntermediatePath);
@@ -846,7 +847,7 @@ TEST_F(DownloadItemTest, AutomaticResumption_AttemptLimit) {
         target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
         DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
         DOWNLOAD_INTERRUPT_REASON_NONE);
-    RunAllPendingInMessageLoops();
+    task_environment_.RunUntilIdle();
 
     // Use a continuable interrupt.
     EXPECT_CALL(*mock_download_file_ref, Cancel()).Times(0);
@@ -854,7 +855,7 @@ TEST_F(DownloadItemTest, AutomaticResumption_AttemptLimit) {
         DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 1,
         std::unique_ptr<crypto::SecureHash>());
 
-    RunAllPendingInMessageLoops();
+    task_environment_.RunUntilIdle();
     ::testing::Mock::VerifyAndClearExpectations(mock_download_file_ref);
   }
 
@@ -902,7 +903,7 @@ TEST_F(DownloadItemTest, FailedResumptionDoesntUpdateOriginState) {
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 1,
       std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
 
   // Now change the create info. The changes should not cause the DownloadItem
@@ -934,7 +935,7 @@ TEST_F(DownloadItemTest, FailedResumptionDoesntUpdateOriginState) {
                       DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
                       base::FilePath(kDummyIntermediatePath),
                       DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   ASSERT_TRUE(item->GetResponseHeaders());
   EXPECT_EQ(kFirstResponseCode, item->GetResponseHeaders()->response_code());
@@ -973,7 +974,7 @@ TEST_F(DownloadItemTest, SucceededResumptionUpdatesOriginState) {
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 0,
       std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
 
   // Now change the create info. The changes should not cause the DownloadItem
@@ -1032,7 +1033,7 @@ TEST_F(DownloadItemTest, ClearReceivedSliceIfEtagChanged) {
       std::unique_ptr<crypto::SecureHash>());
   EXPECT_EQ(kReceivedSlice, item->GetReceivedSlices());
 
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   // Change the strong validator and resume the download, the received slices
   // should be cleared.
@@ -1076,7 +1077,7 @@ TEST_F(DownloadItemTest, ResumeUsesFinalURL) {
   // the mock doesn't follow through with the resumption.
   // ResumeInterruptedDownload() being called is sufficient for verifying that
   // the resumption was triggered.
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   // The download is currently in RESUMING_INTERNAL, which maps to IN_PROGRESS.
   CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
@@ -1097,7 +1098,7 @@ TEST_F(DownloadItemTest, DisplayName) {
   callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(FILE_PATH_LITERAL("foo.bar"),
             item->GetFileNameToReportUser().value());
   item->SetDisplayName(base::FilePath(FILE_PATH_LITERAL("new.name")));
@@ -1117,7 +1118,7 @@ TEST_F(DownloadItemTest, Start) {
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _));
   item->Start(std::move(download_file), std::move(request_handle),
               *create_info());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS);
 }
@@ -1136,21 +1137,19 @@ TEST_F(DownloadItemTest, InitDownloadFileFails) {
       .WillOnce(ScheduleCallbackWithParam(
           DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED));
 
-  base::RunLoop start_download_loop;
   DownloadTargetCallback download_target_callback;
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
-      .WillOnce(DoAll(SaveArg<1>(&download_target_callback),
-                      ScheduleClosure(start_download_loop.QuitClosure())));
+      .WillOnce(SaveArg<1>(&download_target_callback));
 
   item->Start(std::move(file), std::move(request_handle), *create_info());
-  start_download_loop.Run();
+  task_environment_.RunUntilIdle();
 
   download_target_callback.Run(base::FilePath(kDummyTargetPath),
                                DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
                                base::FilePath(kDummyIntermediatePath),
                                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
@@ -1175,7 +1174,7 @@ TEST_F(DownloadItemTest, StartFailedDownload) {
   item->Start(std::move(null_download_file), std::move(null_request_handle),
               *create_info());
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   // The DownloadItemImpl should attempt to determine a target path even if the
   // download was interrupted.
@@ -1186,7 +1185,7 @@ TEST_F(DownloadItemTest, StartFailedDownload) {
                                DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, target_path,
                                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   EXPECT_EQ(target_path, item->GetTargetFilePath());
   CleanupItem(item, NULL, DownloadItem::INTERRUPTED);
@@ -1209,7 +1208,7 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
   mock_delegate()->VerifyAndClearExpectations();
@@ -1224,7 +1223,7 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
   EXPECT_CALL(*download_file, Detach());
   item->DestinationObserverAsWeakPtr()->DestinationCompleted(
       0, std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   ::testing::Mock::VerifyAndClearExpectations(download_file);
   mock_delegate()->VerifyAndClearExpectations();
 }
@@ -1249,7 +1248,7 @@ TEST_F(DownloadItemTest, CallbackAfterInterruptedRename) {
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
   mock_delegate()->VerifyAndClearExpectations();
@@ -1267,7 +1266,7 @@ TEST_F(DownloadItemTest, Interrupted) {
   EXPECT_CALL(*download_file, Cancel());
   item->DestinationObserverAsWeakPtr()->DestinationError(
       reason, 0, std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(reason, item->GetLastReason());
 
@@ -1302,7 +1301,7 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Restart) {
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
   mock_delegate()->VerifyAndClearExpectations();
@@ -1341,7 +1340,7 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) {
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
   mock_delegate()->VerifyAndClearExpectations();
@@ -1375,7 +1374,7 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Failed) {
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
                DOWNLOAD_INTERRUPT_REASON_NONE);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
   mock_delegate()->VerifyAndClearExpectations();
@@ -1598,7 +1597,7 @@ TEST_F(DownloadItemTest, EnabledActionsForNormalDownload) {
   EXPECT_CALL(*download_file, Detach());
   item->DestinationObserverAsWeakPtr()->DestinationCompleted(
       0, std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   ASSERT_EQ(DownloadItem::COMPLETE, item->GetState());
   EXPECT_TRUE(item->CanShowInFolder());
@@ -1631,7 +1630,7 @@ TEST_F(DownloadItemTest, EnabledActionsForTemporaryDownload) {
   EXPECT_CALL(*download_file, Detach());
   item->DestinationObserverAsWeakPtr()->DestinationCompleted(
       0, std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   ASSERT_EQ(DownloadItem::COMPLETE, item->GetState());
   EXPECT_FALSE(item->CanShowInFolder());
@@ -1647,7 +1646,7 @@ TEST_F(DownloadItemTest, EnabledActionsForInterruptedDownload) {
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 0,
       std::unique_ptr<crypto::SecureHash>());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   ASSERT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   ASSERT_FALSE(item->GetTargetFilePath().empty());
@@ -1662,7 +1661,7 @@ TEST_F(DownloadItemTest, EnabledActionsForCancelledDownload) {
 
   EXPECT_CALL(*download_file, Cancel());
   item->Cancel(true);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   ASSERT_EQ(DownloadItem::CANCELLED, item->GetState());
   EXPECT_FALSE(item->CanShowInFolder());
@@ -1697,7 +1696,7 @@ TEST_F(DownloadItemTest, CompleteDelegate_ReturnTrue) {
   EXPECT_CALL(*download_file, FullPath())
       .WillOnce(ReturnRefOfCopy(base::FilePath()));
   EXPECT_CALL(*download_file, Detach());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
 }
 
@@ -1705,11 +1704,11 @@ TEST_F(DownloadItemTest, CompleteDelegate_ReturnTrue) {
 TEST_F(DownloadItemTest, CompleteDelegate_BlockOnce) {
   // Test to confirm that if we have a callback that returns true,
   // we complete immediately.
+
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
 
-  // Drive the delegate interaction.
   base::Closure delegate_callback;
   base::Closure copy_delegate_callback;
   EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
@@ -1737,7 +1736,7 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockOnce) {
   EXPECT_CALL(*download_file, FullPath())
       .WillOnce(ReturnRefOfCopy(base::FilePath()));
   EXPECT_CALL(*download_file, Detach());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
 }
 
@@ -1780,13 +1779,13 @@ TEST_F(DownloadItemTest, CompleteDelegate_SetDanger) {
   EXPECT_CALL(*download_file, FullPath())
       .WillOnce(ReturnRefOfCopy(base::FilePath()));
   EXPECT_CALL(*download_file, Detach());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
   EXPECT_TRUE(item->IsDangerous());
 
   item->ValidateDangerousDownload();
   EXPECT_EQ(DOWNLOAD_DANGER_TYPE_USER_VALIDATED, item->GetDangerType());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
 }
 
@@ -1833,7 +1832,7 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockTwice) {
   EXPECT_CALL(*download_file, FullPath())
       .WillOnce(ReturnRefOfCopy(base::FilePath()));
   EXPECT_CALL(*download_file, Detach());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
 }
 
@@ -1855,7 +1854,7 @@ TEST_F(DownloadItemTest, StealDangerousDownloadAndDiscard) {
       base::Bind(&DownloadItemTest::OnDownloadFileAcquired,
                  weak_ptr_factory.GetWeakPtr(),
                  base::Unretained(&returned_path)));
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(full_path, returned_path);
 }
 
@@ -1874,7 +1873,7 @@ TEST_F(DownloadItemTest, StealDangerousDownloadAndKeep) {
       base::Bind(&DownloadItemTest::OnDownloadFileAcquired,
                  weak_ptr_factory.GetWeakPtr(),
                  base::Unretained(&returned_path)));
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_NE(full_path, returned_path);
   CleanupItem(item, download_file, DownloadItem::IN_PROGRESS);
 }
@@ -1900,7 +1899,7 @@ TEST_F(DownloadItemTest, StealInterruptedContinuableDangerousDownload) {
       true, base::Bind(&DownloadItemTest::OnDownloadFileAcquired,
                        weak_ptr_factory.GetWeakPtr(),
                        base::Unretained(&returned_path)));
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_EQ(full_path, returned_path);
 }
 
@@ -1922,7 +1921,7 @@ TEST_F(DownloadItemTest, StealInterruptedNonContinuableDangerousDownload) {
       true, base::Bind(&DownloadItemTest::OnDownloadFileAcquired,
                        weak_ptr_factory.GetWeakPtr(),
                        base::Unretained(&returned_path)));
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   EXPECT_TRUE(returned_path.empty());
 }
 
@@ -2167,31 +2166,25 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) {
   EXPECT_CALL(*file_, Cancel());
   EXPECT_CALL(*request_handle_, CancelRequest(_));
 
-  base::RunLoop download_start_loop;
   DownloadFile::InitializeCallback initialize_callback;
   EXPECT_CALL(*file_, Initialize(_, _, _, _))
-      .WillOnce(DoAll(SaveArg<0>(&initialize_callback),
-                      ScheduleClosure(download_start_loop.QuitClosure())));
+      .WillOnce(SaveArg<0>(&initialize_callback));
   item_->Start(std::move(file_), std::move(request_handle_), *create_info());
-  download_start_loop.Run();
+  task_environment_.RunUntilIdle();
 
   base::WeakPtr<DownloadDestinationObserver> destination_observer =
       item_->DestinationObserverAsWeakPtr();
 
   ScheduleObservations(PreInitializeFileObservations(), destination_observer);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
-  base::RunLoop initialize_completion_loop;
   DownloadTargetCallback target_callback;
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _))
-      .WillOnce(
-          DoAll(SaveArg<1>(&target_callback),
-                ScheduleClosure(initialize_completion_loop.QuitClosure())));
+      .WillOnce(SaveArg<1>(&target_callback));
   ScheduleObservations(PostInitializeFileObservations(), destination_observer);
   initialize_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE);
-  initialize_completion_loop.Run();
 
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   ASSERT_FALSE(target_callback.is_null());
   ScheduleObservations(PostTargetDeterminationObservations(),
@@ -2201,7 +2194,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) {
                       DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, base::FilePath(),
                       DOWNLOAD_INTERRUPT_REASON_NONE);
   EXPECT_EQ(DownloadItem::CANCELLED, item_->GetState());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 }
 
 // Run through the DII workflow, but the intermediate rename fails.
@@ -2214,37 +2207,30 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) {
   // Intermediate rename loop is not used immediately, but let's set up the
   // DownloadFile expectations since we are about to transfer its ownership to
   // the DownloadItem.
-  base::RunLoop intermediate_rename_loop;
   DownloadFile::RenameCompletionCallback intermediate_rename_callback;
   EXPECT_CALL(*file_, RenameAndUniquify(_, _))
-      .WillOnce(DoAll(SaveArg<1>(&intermediate_rename_callback),
-                      ScheduleClosure(intermediate_rename_loop.QuitClosure())));
+      .WillOnce(SaveArg<1>(&intermediate_rename_callback));
 
-  base::RunLoop download_start_loop;
   DownloadFile::InitializeCallback initialize_callback;
   EXPECT_CALL(*file_, Initialize(_, _, _, _))
-      .WillOnce(DoAll(SaveArg<0>(&initialize_callback),
-                      ScheduleClosure(download_start_loop.QuitClosure())));
+      .WillOnce(SaveArg<0>(&initialize_callback));
 
   item_->Start(std::move(file_), std::move(request_handle_), *create_info());
-  download_start_loop.Run();
+  task_environment_.RunUntilIdle();
+
   base::WeakPtr<DownloadDestinationObserver> destination_observer =
       item_->DestinationObserverAsWeakPtr();
 
   ScheduleObservations(PreInitializeFileObservations(), destination_observer);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
-  base::RunLoop initialize_completion_loop;
   DownloadTargetCallback target_callback;
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _))
-      .WillOnce(
-          DoAll(SaveArg<1>(&target_callback),
-                ScheduleClosure(initialize_completion_loop.QuitClosure())));
+      .WillOnce(SaveArg<1>(&target_callback));
   ScheduleObservations(PostInitializeFileObservations(), destination_observer);
   initialize_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE);
-  initialize_completion_loop.Run();
 
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   ASSERT_FALSE(target_callback.is_null());
 
   ScheduleObservations(PostTargetDeterminationObservations(),
@@ -2255,14 +2241,14 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) {
                       base::FilePath(kDummyIntermediatePath),
                       DOWNLOAD_INTERRUPT_REASON_NONE);
 
-  intermediate_rename_loop.Run();
+  task_environment_.RunUntilIdle();
   ASSERT_FALSE(intermediate_rename_callback.is_null());
 
   ScheduleObservations(PostIntermediateRenameObservations(),
                        destination_observer);
   intermediate_rename_callback.Run(DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
                                    base::FilePath());
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   EXPECT_EQ(DownloadItem::INTERRUPTED, item_->GetState());
 }
@@ -2284,37 +2270,30 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) {
   // Intermediate rename loop is not used immediately, but let's set up the
   // DownloadFile expectations since we are about to transfer its ownership to
   // the DownloadItem.
-  base::RunLoop intermediate_rename_loop;
   DownloadFile::RenameCompletionCallback intermediate_rename_callback;
   EXPECT_CALL(*file_, RenameAndUniquify(_, _))
-      .WillOnce(DoAll(SaveArg<1>(&intermediate_rename_callback),
-                      ScheduleClosure(intermediate_rename_loop.QuitClosure())));
+      .WillOnce(SaveArg<1>(&intermediate_rename_callback));
 
-  base::RunLoop download_start_loop;
   DownloadFile::InitializeCallback initialize_callback;
   EXPECT_CALL(*file_, Initialize(_, _, _, _))
-      .WillOnce(DoAll(SaveArg<0>(&initialize_callback),
-                      ScheduleClosure(download_start_loop.QuitClosure())));
+      .WillOnce(SaveArg<0>(&initialize_callback));
 
   item_->Start(std::move(file_), std::move(request_handle_), *create_info());
-  download_start_loop.Run();
+  task_environment_.RunUntilIdle();
+
   base::WeakPtr<DownloadDestinationObserver> destination_observer =
       item_->DestinationObserverAsWeakPtr();
 
   ScheduleObservations(PreInitializeFileObservations(), destination_observer);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
-  base::RunLoop initialize_completion_loop;
   DownloadTargetCallback target_callback;
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _))
-      .WillOnce(
-          DoAll(SaveArg<1>(&target_callback),
-                ScheduleClosure(initialize_completion_loop.QuitClosure())));
+      .WillOnce(SaveArg<1>(&target_callback));
   ScheduleObservations(PostInitializeFileObservations(), destination_observer);
   initialize_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE);
-  initialize_completion_loop.Run();
 
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
   ASSERT_FALSE(target_callback.is_null());
 
   ScheduleObservations(PostTargetDeterminationObservations(),
@@ -2325,7 +2304,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) {
                       base::FilePath(kDummyIntermediatePath),
                       DOWNLOAD_INTERRUPT_REASON_NONE);
 
-  intermediate_rename_loop.Run();
+  task_environment_.RunUntilIdle();
   ASSERT_FALSE(intermediate_rename_callback.is_null());
 
   // This may or may not be called, depending on whether there are any errors in
@@ -2337,7 +2316,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) {
                        destination_observer);
   intermediate_rename_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE,
                                    base::FilePath(kDummyIntermediatePath));
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 
   // The state of the download depends on the observer events that were played
   // back to the DownloadItemImpl. Hence we can't establish a single expectation
@@ -2350,7 +2329,7 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) {
     EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, item_->GetLastReason());
 
   item_->Cancel(true);
-  RunAllPendingInMessageLoops();
+  task_environment_.RunUntilIdle();
 }
 
 TEST(MockDownloadItem, Compiles) {