Commit a46c813c authored by Sami Kyostila's avatar Sami Kyostila Committed by Commit Bot

chrome/browser/net: 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://crrev.com/c/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=nharper@chromium.org

Bug: 968047
Change-Id: Ie94b75f8055d40ca1225b1da7154e8f86a4b1350
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1738596
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarNick Harper <nharper@chromium.org>
Commit-Queue: Nick Harper <nharper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684461}
parent fc457ca8
...@@ -157,9 +157,8 @@ IN_PROC_BROWSER_TEST_F(ChromeMojoProxyResolverFactoryBrowserTest, ...@@ -157,9 +157,8 @@ IN_PROC_BROWSER_TEST_F(ChromeMojoProxyResolverFactoryBrowserTest,
// Wait a little bit and check it's still running. // Wait a little bit and check it's still running.
{ {
base::RunLoop run_loop; base::RunLoop run_loop;
base::PostDelayedTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, base::PostDelayedTask(FROM_HERE, {content::BrowserThread::UI},
run_loop.QuitClosure(), run_loop.QuitClosure(), kServiceShutdownTimeout);
kServiceShutdownTimeout);
run_loop.Run(); run_loop.Run();
} }
...@@ -202,9 +201,8 @@ IN_PROC_BROWSER_TEST_F(ChromeMojoProxyResolverFactoryBrowserTest, ...@@ -202,9 +201,8 @@ IN_PROC_BROWSER_TEST_F(ChromeMojoProxyResolverFactoryBrowserTest,
// Wait a little bit and check it's still running. // Wait a little bit and check it's still running.
{ {
base::RunLoop run_loop; base::RunLoop run_loop;
base::PostDelayedTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, base::PostDelayedTask(FROM_HERE, {content::BrowserThread::UI},
run_loop.QuitClosure(), run_loop.QuitClosure(), kServiceShutdownTimeout);
kServiceShutdownTimeout);
run_loop.Run(); run_loop.Run();
} }
......
...@@ -58,10 +58,10 @@ namespace chrome_browser_net { ...@@ -58,10 +58,10 @@ namespace chrome_browser_net {
namespace { namespace {
// Postable function to run a Closure on the UI thread. Since // Postable function to run a Closure on the UI thread. Since
// base::PostTaskWithTraits returns a bool, it can't directly be posted to // base::PostTask returns a bool, it can't directly be posted to
// another thread. // another thread.
void RunClosureOnUIThread(const base::Closure& closure) { void RunClosureOnUIThread(const base::Closure& closure) {
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, closure); base::PostTask(FROM_HERE, {BrowserThread::UI}, closure);
} }
// Wraps DnsProbeService and delays probes until someone calls // Wraps DnsProbeService and delays probes until someone calls
...@@ -419,8 +419,7 @@ void DnsProbeBrowserTest::SetUpOnMainThread() { ...@@ -419,8 +419,7 @@ void DnsProbeBrowserTest::SetUpOnMainThread() {
browser()->profile()->GetPrefs()->SetBoolean( browser()->profile()->GetPrefs()->SetBoolean(
prefs::kAlternateErrorPagesEnabled, true); prefs::kAlternateErrorPagesEnabled, true);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO},
BindOnce(&DnsProbeBrowserTestIOThreadHelper::SetUpOnIOThread, BindOnce(&DnsProbeBrowserTestIOThreadHelper::SetUpOnIOThread,
Unretained(helper_))); Unretained(helper_)));
...@@ -434,7 +433,7 @@ void DnsProbeBrowserTest::SetUpOnMainThread() { ...@@ -434,7 +433,7 @@ void DnsProbeBrowserTest::SetUpOnMainThread() {
} }
void DnsProbeBrowserTest::TearDownOnMainThread() { void DnsProbeBrowserTest::TearDownOnMainThread() {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
BindOnce( BindOnce(
&DnsProbeBrowserTestIOThreadHelper::CleanUpOnIOThreadAndDeleteHelper, &DnsProbeBrowserTestIOThreadHelper::CleanUpOnIOThreadAndDeleteHelper,
...@@ -500,7 +499,7 @@ void DnsProbeBrowserTest::SetFakeHostResolverResults( ...@@ -500,7 +499,7 @@ void DnsProbeBrowserTest::SetFakeHostResolverResults(
void DnsProbeBrowserTest::SetCorrectionServiceBroken(bool broken) { void DnsProbeBrowserTest::SetCorrectionServiceBroken(bool broken) {
int net_error = broken ? net::ERR_NAME_NOT_RESOLVED : net::OK; int net_error = broken ? net::ERR_NAME_NOT_RESOLVED : net::OK;
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
BindOnce(&DnsProbeBrowserTestIOThreadHelper::SetCorrectionServiceNetError, BindOnce(&DnsProbeBrowserTestIOThreadHelper::SetCorrectionServiceNetError,
Unretained(helper_), net_error)); Unretained(helper_), net_error));
...@@ -508,7 +507,7 @@ void DnsProbeBrowserTest::SetCorrectionServiceBroken(bool broken) { ...@@ -508,7 +507,7 @@ void DnsProbeBrowserTest::SetCorrectionServiceBroken(bool broken) {
void DnsProbeBrowserTest::SetCorrectionServiceDelayRequests( void DnsProbeBrowserTest::SetCorrectionServiceDelayRequests(
bool delay_requests) { bool delay_requests) {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
BindOnce( BindOnce(
&DnsProbeBrowserTestIOThreadHelper::SetCorrectionServiceDelayRequests, &DnsProbeBrowserTestIOThreadHelper::SetCorrectionServiceDelayRequests,
...@@ -517,7 +516,7 @@ void DnsProbeBrowserTest::SetCorrectionServiceDelayRequests( ...@@ -517,7 +516,7 @@ void DnsProbeBrowserTest::SetCorrectionServiceDelayRequests(
void DnsProbeBrowserTest::WaitForDelayedRequestDestruction() { void DnsProbeBrowserTest::WaitForDelayedRequestDestruction() {
base::RunLoop run_loop; base::RunLoop run_loop;
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
BindOnce( BindOnce(
&DnsProbeBrowserTestIOThreadHelper::SetRequestDestructionCallback, &DnsProbeBrowserTestIOThreadHelper::SetRequestDestructionCallback,
......
...@@ -331,8 +331,7 @@ class DNSErrorPageTest : public ErrorPageTest { ...@@ -331,8 +331,7 @@ class DNSErrorPageTest : public ErrorPageTest {
EXPECT_EQ(origin, "null"); EXPECT_EQ(origin, "null");
// Send RequestCreated so that anyone blocking on // Send RequestCreated so that anyone blocking on
// WaitForRequests can continue. // WaitForRequests can continue.
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::UI},
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&DNSErrorPageTest::RequestCreated, base::BindOnce(&DNSErrorPageTest::RequestCreated,
base::Unretained(owner))); base::Unretained(owner)));
content::URLLoaderInterceptor::WriteResponse( content::URLLoaderInterceptor::WriteResponse(
......
...@@ -48,8 +48,9 @@ FileDownloader::FileDownloader( ...@@ -48,8 +48,9 @@ FileDownloader::FileDownloader(
base::Unretained(this))); base::Unretained(this)));
} else { } else {
base::PostTaskAndReplyWithResult( base::PostTaskAndReplyWithResult(
base::CreateTaskRunnerWithTraits( base::CreateTaskRunner(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT, {base::ThreadPool(), base::MayBlock(),
base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}) base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})
.get(), .get(),
FROM_HERE, base::Bind(&base::PathExists, local_path_), FROM_HERE, base::Bind(&base::PathExists, local_path_),
...@@ -75,8 +76,8 @@ void FileDownloader::OnSimpleDownloadComplete(base::FilePath response_path) { ...@@ -75,8 +76,8 @@ void FileDownloader::OnSimpleDownloadComplete(base::FilePath response_path) {
} }
base::PostTaskAndReplyWithResult( base::PostTaskAndReplyWithResult(
base::CreateTaskRunnerWithTraits( base::CreateTaskRunner({base::ThreadPool(), base::MayBlock(),
{base::MayBlock(), base::TaskPriority::BEST_EFFORT, base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}) base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})
.get(), .get(),
FROM_HERE, base::Bind(&base::Move, response_path, local_path_), FROM_HERE, base::Bind(&base::Move, response_path, local_path_),
......
...@@ -124,8 +124,7 @@ class NetworkQualityTrackerBrowserTest : public InProcessBrowserTest { ...@@ -124,8 +124,7 @@ class NetworkQualityTrackerBrowserTest : public InProcessBrowserTest {
void SimulateNetworkQualityChange(net::EffectiveConnectionType type) { void SimulateNetworkQualityChange(net::EffectiveConnectionType type) {
if (!content::IsOutOfProcessNetworkService()) { if (!content::IsOutOfProcessNetworkService()) {
scoped_refptr<base::SequencedTaskRunner> task_runner = scoped_refptr<base::SequencedTaskRunner> task_runner =
base::CreateSequencedTaskRunnerWithTraits( base::CreateSequencedTaskRunner({content::BrowserThread::IO});
{content::BrowserThread::IO});
if (content::IsInProcessNetworkService()) if (content::IsInProcessNetworkService())
task_runner = content::GetNetworkTaskRunner(); task_runner = content::GetNetworkTaskRunner();
task_runner->PostTask( task_runner->PostTask(
......
...@@ -52,7 +52,7 @@ void GetNSSCertDatabaseForProfile( ...@@ -52,7 +52,7 @@ void GetNSSCertDatabaseForProfile(
const base::Callback<void(net::NSSCertDatabase*)>& callback) { const base::Callback<void(net::NSSCertDatabase*)>& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce(&GetCertDBOnIOThread, profile->GetResourceContext(), base::BindOnce(&GetCertDBOnIOThread, profile->GetResourceContext(),
base::ThreadTaskRunnerHandle::Get(), callback)); base::ThreadTaskRunnerHandle::Get(), callback));
......
...@@ -42,7 +42,7 @@ class DBTester { ...@@ -42,7 +42,7 @@ class DBTester {
// Returns true if the database was retrieved successfully. // Returns true if the database was retrieved successfully.
bool DoGetDBTests() { bool DoGetDBTests() {
base::RunLoop run_loop; base::RunLoop run_loop;
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {content::BrowserThread::IO}, FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&DBTester::GetDBAndDoTestsOnIOThread, base::BindOnce(&DBTester::GetDBAndDoTestsOnIOThread,
base::Unretained(this), profile_->GetResourceContext(), base::Unretained(this), profile_->GetResourceContext(),
...@@ -54,7 +54,7 @@ class DBTester { ...@@ -54,7 +54,7 @@ class DBTester {
// Test retrieving the database again, should be called after DoGetDBTests. // Test retrieving the database again, should be called after DoGetDBTests.
void DoGetDBAgainTests() { void DoGetDBAgainTests() {
base::RunLoop run_loop; base::RunLoop run_loop;
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {content::BrowserThread::IO}, FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&DBTester::DoGetDBAgainTestsOnIOThread, base::BindOnce(&DBTester::DoGetDBAgainTestsOnIOThread,
base::Unretained(this), profile_->GetResourceContext(), base::Unretained(this), profile_->GetResourceContext(),
...@@ -95,8 +95,7 @@ class DBTester { ...@@ -95,8 +95,7 @@ class DBTester {
EXPECT_EQ(db->GetPublicSlot().get(), db->GetPrivateSlot().get()); EXPECT_EQ(db->GetPublicSlot().get(), db->GetPrivateSlot().get());
} }
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, base::PostTask(FROM_HERE, {content::BrowserThread::UI}, done_callback);
done_callback);
} }
void DoGetDBAgainTestsOnIOThread(content::ResourceContext* context, void DoGetDBAgainTestsOnIOThread(content::ResourceContext* context,
...@@ -108,8 +107,7 @@ class DBTester { ...@@ -108,8 +107,7 @@ class DBTester {
// Should return the same db as before. // Should return the same db as before.
EXPECT_EQ(db_, db); EXPECT_EQ(db_, db);
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, base::PostTask(FROM_HERE, {content::BrowserThread::UI}, done_callback);
done_callback);
} }
Profile* profile_; Profile* profile_;
......
...@@ -226,9 +226,9 @@ ProfileNetworkContextService::CreateNetworkContext( ...@@ -226,9 +226,9 @@ ProfileNetworkContextService::CreateNetworkContext(
// stable users, which would be after M83 branches. // stable users, which would be after M83 branches.
base::FilePath media_cache_path = GetPartitionPath(relative_partition_path) base::FilePath media_cache_path = GetPartitionPath(relative_partition_path)
.Append(chrome::kMediaCacheDirname); .Append(chrome::kMediaCacheDirname);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, FROM_HERE,
{base::TaskPriority::BEST_EFFORT, base::MayBlock(), {base::ThreadPool(), base::TaskPriority::BEST_EFFORT, base::MayBlock(),
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}, base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), media_cache_path, base::BindOnce(base::IgnoreResult(&base::DeleteFile), media_cache_path,
true /* recursive */)); true /* recursive */));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment