chrome/browser/safe_browsing/download_protection: Always specify thread affinity when posting tasks

*** Note: There is no behavior change from this patch. ***

The PostTask APIs will shortly be changed to require all tasks to explicitly
specify their thread affinity, i.e., whether the task should run on the thread
pool or a specific named thread such as a BrowserThread. This patch updates all
call sites with thread affinity annotation. We also remove the "WithTraits"
suffix to make the call sites more readable.

Before:

    // Thread pool task.
    base::PostTaskWithTraits(FROM_HERE, {...}, ...);

    // UI thread task.
    base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI, ...}, ...);

After:

    // Thread pool task.
    base::PostTask(FROM_HERE, {base::ThreadPool(), ...}, ...);

    // UI thread task.
    base::PostTask(FROM_HERE, {BrowserThread::UI, ...}, ...);

This patch was semi-automatically prepared with these steps:

    1. Patch in https://chromium-review.googlesource.com/c/chromium/src/+/1635827
       to make thread affinity a build-time requirement.
    2. Run an initial pass with a clang rewriter:
       https://chromium-review.googlesource.com/c/chromium/src/+/1635623
    3. ninja -C out/Debug | grep 'requested here' | cut -d: -f1-3 | sort | \
           uniq > errors.txt
    4. while read line; do
         f=$(echo $line | cut -d: -f 1)
         r=$(echo $line | cut -d: -f 2)
         c=$(echo $line | cut -d: -f 3)
         sed -i "${r}s/./&base::ThreadPool(),/$c" $f
       done < errors.txt
    5. GOTO 3 until build succeeds.
    6. Remove the "WithTraits" suffix from task API call sites:

       $ tools/git/mffr.py -i <(cat <<EOF
       [
         ["PostTaskWithTraits",                            "PostTask"],
         ["PostDelayedTaskWithTraits",                     "PostDelayedTask"],
         ["PostTaskWithTraitsAndReply",                    "PostTaskAndReply"],
         ["CreateTaskRunnerWithTraits",                    "CreateTaskRunner"],
         ["CreateSequencedTaskRunnerWithTraits",           "CreateSequencedTaskRunner"],
         ["CreateUpdateableSequencedTaskRunnerWithTraits", "CreateUpdateableSequencedTaskRunner"],
         ["CreateSingleThreadTaskRunnerWithTraits",        "CreateSingleThreadTaskRunner"],
         ["CreateCOMSTATaskRunnerWithTraits",              "CreateCOMSTATaskRunner"]
       ]
       EOF
       )

This CL was uploaded by git cl split.

R=nparker@chromium.org

Bug: 968047
Change-Id: I11dd0befca765ffa4b25c8df66bd7557621b2b08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729223
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682860}

chrome/browser/safe_browsing/download_protection/check_client_download_request.cc

@@ -200,7 +200,7 @@ void CheckClientDownloadRequest::Start() {
   // If whitelist check passes, FinishRequest() will be called to avoid
   // analyzing file. Otherwise, AnalyzeFile() will be called to continue with
   // analysis.
-  base::PostTaskWithTraitsAndReplyWithResult(
+  base::PostTaskAndReplyWithResult(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(&CheckUrlAgainstWhitelist, url_chain_.back(),
                      database_manager_),
@@ -213,7 +213,7 @@ void CheckClientDownloadRequest::Start() {
 void CheckClientDownloadRequest::StartTimeout() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   timeout_start_time_ = base::TimeTicks::Now();
-  base::PostDelayedTaskWithTraits(
+  base::PostDelayedTask(
       FROM_HERE, {BrowserThread::UI},
       base::BindOnce(&CheckClientDownloadRequest::FinishRequest,
                      weakptr_factory_.GetWeakPtr(),
@@ -295,7 +295,7 @@ void CheckClientDownloadRequest::OnURLLoaderComplete(
       }
     }
 
-    base::PostTaskWithTraits(
+    base::PostTask(
         FROM_HERE, {content::BrowserThread::UI},
         base::BindOnce(
             &WebUIInfoSingleton::AddToClientDownloadResponsesReceived,
@@ -458,7 +458,7 @@ void CheckClientDownloadRequest::OnFileFeatureExtractionDone(
   detached_code_signatures_.CopyFrom(results.detached_code_signatures);
 #endif
 
-  base::PostTaskWithTraitsAndReplyWithResult(
+  base::PostTaskAndReplyWithResult(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(&CheckCertificateChainAgainstWhitelist, signature_info_,
                      database_manager_),
@@ -469,10 +469,9 @@ void CheckClientDownloadRequest::OnFileFeatureExtractionDone(
   // We wait until after the file checks finish to start the timeout, as
   // windows can cause permissions errors if the timeout fired while we were
   // checking the file signature and we tried to complete the download.
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&CheckClientDownloadRequest::StartTimeout,
-                     weakptr_factory_.GetWeakPtr()));
+  base::PostTask(FROM_HERE, {BrowserThread::UI},
+                 base::BindOnce(&CheckClientDownloadRequest::StartTimeout,
+                                weakptr_factory_.GetWeakPtr()));
 }
 
 bool CheckClientDownloadRequest::ShouldSampleWhitelistedDownload() {
@@ -789,7 +788,7 @@ void CheckClientDownloadRequest::SendRequest() {
   // The following is to log this ClientDownloadRequest on any open
   // chrome://safe-browsing pages. If no such page is open, the request is
   // dropped and the |request| object deleted.
-  base::PostTaskWithTraits(
+  base::PostTask(
       FROM_HERE, {content::BrowserThread::UI},
       base::BindOnce(&WebUIInfoSingleton::AddToClientDownloadRequestsSent,
                      base::Unretained(WebUIInfoSingleton::GetInstance()),

chrome/browser/safe_browsing/download_protection/download_feedback_service_unittest.cc

@@ -122,8 +122,9 @@ bool WillStorePings(DownloadCheckResult result,
 class DownloadFeedbackServiceTest : public testing::Test {
  public:
   DownloadFeedbackServiceTest()
-      : file_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
-            {base::MayBlock(), base::TaskPriority::BEST_EFFORT})) {}
+      : file_task_runner_(base::CreateSequencedTaskRunner(
+            {base::ThreadPool(), base::MayBlock(),
+             base::TaskPriority::BEST_EFFORT})) {}
 
   void SetUp() override {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());

chrome/browser/safe_browsing/download_protection/download_feedback_unittest.cc

@@ -97,10 +97,11 @@ std::unique_ptr<TwoPhaseUploader> FakeUploaderFactory::CreateTwoPhaseUploader(
 class DownloadFeedbackTest : public testing::Test {
  public:
   DownloadFeedbackTest()
-      : file_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
-            {base::MayBlock(), base::TaskPriority::BEST_EFFORT})),
-        io_task_runner_(base::CreateSingleThreadTaskRunnerWithTraits(
-            {content::BrowserThread::IO})),
+      : file_task_runner_(base::CreateSequencedTaskRunner(
+            {base::ThreadPool(), base::MayBlock(),
+             base::TaskPriority::BEST_EFFORT})),
+        io_task_runner_(
+            base::CreateSingleThreadTaskRunner({content::BrowserThread::IO})),
         feedback_finish_called_(false) {
     EXPECT_NE(io_task_runner_, file_task_runner_);
     shared_url_loader_factory_ =

chrome/browser/safe_browsing/download_protection/download_protection_service.cc

@@ -110,8 +110,8 @@ DownloadProtectionService::DownloadProtectionService(
       download_request_timeout_ms_(kDownloadRequestTimeoutMs),
       feedback_service_(new DownloadFeedbackService(
           url_loader_factory_,
-          base::CreateSequencedTaskRunnerWithTraits(
-              {base::MayBlock(), base::TaskPriority::BEST_EFFORT})
+          base::CreateSequencedTaskRunner({base::ThreadPool(), base::MayBlock(),
+                                           base::TaskPriority::BEST_EFFORT})
               .get())),
       whitelist_sample_rate_(kWhitelistDownloadSampleRate) {
   if (sb_service) {
@@ -197,9 +197,8 @@ void DownloadProtectionService::CheckDownloadUrl(
   scoped_refptr<DownloadUrlSBClient> client(new DownloadUrlSBClient(
       item, this, callback, ui_manager_, database_manager_));
   // The client will release itself once it is done.
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&DownloadUrlSBClient::StartCheck, client));
+  base::PostTask(FROM_HERE, {BrowserThread::IO},
+                 base::BindOnce(&DownloadUrlSBClient::StartCheck, client));
 }
 
 bool DownloadProtectionService::IsSupportedDownload(

chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc

@@ -201,7 +201,7 @@ ACTION_P(TrustSignature, contents) {
 }
 
 ACTION_P(CheckDownloadUrlDone, threat_type) {
-  base::PostTaskWithTraits(
+  base::PostTask(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(
           &SafeBrowsingDatabaseManager::Client::OnCheckDownloadUrlResult,
@@ -456,12 +456,12 @@ class DownloadProtectionServiceTest : public ChromeRenderViewHostTestHarness {
   // Helper functions for FlushThreadMessageLoops.
   void RunAllPendingAndQuitUI(const base::Closure& quit_closure) {
     RunLoop().RunUntilIdle();
-    base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, quit_closure);
+    base::PostTask(FROM_HERE, {BrowserThread::UI}, quit_closure);
   }
 
   void PostRunMessageLoopTask(BrowserThread::ID thread,
                               const base::Closure& quit_closure) {
-    base::PostTaskWithTraits(
+    base::PostTask(
         FROM_HERE, {thread},
         base::BindOnce(&DownloadProtectionServiceTest::RunAllPendingAndQuitUI,
                        base::Unretained(this), quit_closure));
@@ -469,7 +469,7 @@ class DownloadProtectionServiceTest : public ChromeRenderViewHostTestHarness {
 
   void FlushMessageLoop(BrowserThread::ID thread) {
     RunLoop run_loop;
-    base::PostTaskWithTraits(
+    base::PostTask(
         FROM_HERE, {BrowserThread::UI},
         base::BindOnce(&DownloadProtectionServiceTest::PostRunMessageLoopTask,
                        base::Unretained(this), thread, run_loop.QuitClosure()));

chrome/browser/safe_browsing/download_protection/download_url_sb_client.cc

@@ -91,18 +91,18 @@ void DownloadUrlSBClient::CheckDone(SBThreatType threat_type) {
   UpdateDownloadCheckStats(total_type_);
   if (threat_type != SB_THREAT_TYPE_SAFE) {
     UpdateDownloadCheckStats(dangerous_type_);
-    base::PostTaskWithTraits(
+    base::PostTask(
         FROM_HERE, {BrowserThread::UI},
         base::BindOnce(&DownloadUrlSBClient::ReportMalware, this, threat_type));
   } else {
     // Identify download referrer chain, which will be used in
     // ClientDownloadRequest.
-    base::PostTaskWithTraits(
+    base::PostTask(
         FROM_HERE, {BrowserThread::UI},
         base::BindOnce(&DownloadUrlSBClient::IdentifyReferrerChain, this));
   }
-  base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI},
-                           base::BindOnce(callback_, result));
+  base::PostTask(FROM_HERE, {BrowserThread::UI},
+                 base::BindOnce(callback_, result));
 }
 
 void DownloadUrlSBClient::ReportMalware(SBThreatType threat_type) {

chrome/browser/safe_browsing/download_protection/file_analyzer.cc

@@ -107,8 +107,10 @@ void FileAnalyzer::Start(const base::FilePath& target_path,
     // Checks for existence of "koly" signature even if file doesn't have
     // archive-type extension, then calls ExtractFileOrDmgFeatures() with
     // result.
-    base::PostTaskWithTraitsAndReplyWithResult(
-        FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
+    base::PostTaskAndReplyWithResult(
+        FROM_HERE,
+        {base::ThreadPool(), base::MayBlock(),
+         base::TaskPriority::USER_VISIBLE},
         base::BindOnce(DiskImageTypeSnifferMac::IsAppleDiskImage, tmp_path_),
         base::BindOnce(&FileAnalyzer::ExtractFileOrDmgFeatures,
                        weakptr_factory_.GetWeakPtr()));
@@ -121,9 +123,9 @@ void FileAnalyzer::Start(const base::FilePath& target_path,
 void FileAnalyzer::StartExtractFileFeatures() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::PostTaskWithTraitsAndReplyWithResult(
+  base::PostTaskAndReplyWithResult(
       FROM_HERE,
-      {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
+      {base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE,
        base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
       base::BindOnce(&ExtractFileFeatures, binary_feature_extractor_,
                      tmp_path_),

chrome/browser/safe_browsing/download_protection/multipart_uploader.cc

@@ -113,11 +113,10 @@ void MultipartUploadRequest::RetryOrFinish(
     std::move(callback_).Run(net::OK, net::HTTP_OK, *response_body.get());
   } else {
     if (retry_count_ < kMaxRetryAttempts) {
-      base::PostDelayedTaskWithTraits(
-          FROM_HERE, {content::BrowserThread::UI},
-          base::BindOnce(&MultipartUploadRequest::SendRequest,
-                         weak_factory_.GetWeakPtr()),
-          current_backoff_);
+      base::PostDelayedTask(FROM_HERE, {content::BrowserThread::UI},
+                            base::BindOnce(&MultipartUploadRequest::SendRequest,
+                                           weak_factory_.GetWeakPtr()),
+                            current_backoff_);
       current_backoff_ *= kBackoffFactor;
       retry_count_++;
     } else {

chrome/browser/safe_browsing/download_protection/ppapi_download_request.cc

@@ -107,14 +107,13 @@ void PPAPIDownloadRequest::Start() {
   // verdict. The weak pointer used for the timeout will be invalidated (and
   // hence would prevent the timeout) if the check completes on time and
   // execution reaches Finish().
-  base::PostDelayedTaskWithTraits(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&PPAPIDownloadRequest::OnRequestTimedOut,
-                     weakptr_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(
-          service_->download_request_timeout_ms()));
-
-  base::PostTaskWithTraits(
+  base::PostDelayedTask(FROM_HERE, {BrowserThread::UI},
+                        base::BindOnce(&PPAPIDownloadRequest::OnRequestTimedOut,
+                                       weakptr_factory_.GetWeakPtr()),
+                        base::TimeDelta::FromMilliseconds(
+                            service_->download_request_timeout_ms()));
+
+  base::PostTask(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(&PPAPIDownloadRequest::CheckWhitelistsOnIOThread,
                      requestor_url_, database_manager_,
@@ -142,10 +141,9 @@ void PPAPIDownloadRequest::CheckWhitelistsOnIOThread(
   bool url_was_whitelisted =
       requestor_url.is_valid() && database_manager &&
       database_manager->MatchDownloadWhitelistUrl(requestor_url);
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&PPAPIDownloadRequest::WhitelistCheckComplete,
-                     download_request, url_was_whitelisted));
+  base::PostTask(FROM_HERE, {BrowserThread::UI},
+                 base::BindOnce(&PPAPIDownloadRequest::WhitelistCheckComplete,
+                                download_request, url_was_whitelisted));
 }
 
 void PPAPIDownloadRequest::WhitelistCheckComplete(bool was_on_whitelist) {

chrome/browser/safe_browsing/download_protection/two_phase_uploader_unittest.cc

@@ -91,8 +91,8 @@ class TwoPhaseUploaderTest : public testing::Test {
  protected:
   content::TestBrowserThreadBundle thread_bundle_;
   const scoped_refptr<base::SequencedTaskRunner> task_runner_ =
-      base::CreateSequencedTaskRunnerWithTraits(
-          {base::MayBlock(), base::TaskPriority::BEST_EFFORT});
+      base::CreateSequencedTaskRunner({base::ThreadPool(), base::MayBlock(),
+                                       base::TaskPriority::BEST_EFFORT});
   std::unique_ptr<network::TestNetworkServiceClient> network_service_client_;
   scoped_refptr<network::TestSharedURLLoaderFactory> shared_url_loader_factory_;
 };