Commit 032c3bea authored by Sami Kyostila's avatar Sami Kyostila Committed by Commit Bot

chrome/browser/apps: 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=benwells@chromium.org, pwnall@chromium.org, rsesek@chromium.org

Bug: 968047
Change-Id: I103a15fe7e1ac0e6dfbbe02875bca4f6a31c0456
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1738595
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarVictor Costan <pwnall@chromium.org>
Reviewed-by: default avatarBen Wells <benwells@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685520}
parent f54a2913
...@@ -323,8 +323,10 @@ void LoadIconFromFileWithFallback( ...@@ -323,8 +323,10 @@ void LoadIconFromFileWithFallback(
case apps::mojom::IconCompression::kUncompressed: case apps::mojom::IconCompression::kUncompressed:
case apps::mojom::IconCompression::kCompressed: { case apps::mojom::IconCompression::kCompressed: {
base::PostTaskWithTraitsAndReplyWithResult( base::PostTaskAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE}, FROM_HERE,
{base::ThreadPool(), base::MayBlock(),
base::TaskPriority::USER_VISIBLE},
base::BindOnce(&ReadFileAsCompressedData, path), base::BindOnce(&ReadFileAsCompressedData, path),
base::BindOnce(&RunCallbackWithFallback, size_hint_in_dip, base::BindOnce(&RunCallbackWithFallback, size_hint_in_dip,
is_placeholder_icon, icon_effects, icon_compression, is_placeholder_icon, icon_effects, icon_compression,
......
...@@ -34,13 +34,14 @@ void AppShimHostManager::Init() { ...@@ -34,13 +34,14 @@ void AppShimHostManager::Init() {
// If running the shim triggers Chrome startup, the user must wait for the // If running the shim triggers Chrome startup, the user must wait for the
// socket to be set up before the shim will be usable. This also requires // socket to be set up before the shim will be usable. This also requires
// IO, so use MayBlock() with USER_VISIBLE. // IO, so use MayBlock() with USER_VISIBLE.
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE}, FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE},
base::BindOnce(&AppShimHostManager::InitOnBackgroundThread, this)); base::BindOnce(&AppShimHostManager::InitOnBackgroundThread, this));
} }
AppShimHostManager::~AppShimHostManager() { AppShimHostManager::~AppShimHostManager() {
base::CreateSingleThreadTaskRunnerWithTraits({content::BrowserThread::IO}) base::CreateSingleThreadTaskRunner({content::BrowserThread::IO})
->DeleteSoon(FROM_HERE, std::move(mach_acceptor_)); ->DeleteSoon(FROM_HERE, std::move(mach_acceptor_));
// The AppShimHostManager is only initialized if the Chrome process // The AppShimHostManager is only initialized if the Chrome process
...@@ -56,9 +57,9 @@ AppShimHostManager::~AppShimHostManager() { ...@@ -56,9 +57,9 @@ AppShimHostManager::~AppShimHostManager() {
base::FilePath version_path = base::FilePath version_path =
user_data_dir.Append(app_mode::kRunningChromeVersionSymlinkName); user_data_dir.Append(app_mode::kRunningChromeVersionSymlinkName);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT, {base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN}, base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), version_path, base::BindOnce(base::IgnoreResult(&base::DeleteFile), version_path,
false)); false));
...@@ -92,7 +93,7 @@ void AppShimHostManager::InitOnBackgroundThread() { ...@@ -92,7 +93,7 @@ void AppShimHostManager::InitOnBackgroundThread() {
void AppShimHostManager::OnClientConnected( void AppShimHostManager::OnClientConnected(
mojo::PlatformChannelEndpoint endpoint, mojo::PlatformChannelEndpoint endpoint,
base::ProcessId peer_pid) { base::ProcessId peer_pid) {
base::CreateSingleThreadTaskRunnerWithTraits({content::BrowserThread::UI}) base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})
->PostTask( ->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&AppShimHostBootstrap::CreateForChannelAndPeerID, base::BindOnce(&AppShimHostBootstrap::CreateForChannelAndPeerID,
......
...@@ -447,8 +447,8 @@ IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, MAYBE_Launch) { ...@@ -447,8 +447,8 @@ IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, MAYBE_Launch) {
// final request to close the window is posted to this thread's queue. // final request to close the window is posted to this thread's queue.
GetFirstAppWindow()->GetBaseWindow()->Close(); GetFirstAppWindow()->GetBaseWindow()->Close();
base::RunLoop run_loop; base::RunLoop run_loop;
base::PostTaskWithTraitsAndReply( base::PostTaskAndReply(
FROM_HERE, {base::MayBlock()}, FROM_HERE, {base::ThreadPool(), base::MayBlock()},
base::BindOnce( base::BindOnce(
[](base::Process* shim_process) { [](base::Process* shim_process) {
base::ScopedAllowBaseSyncPrimitivesForTesting base::ScopedAllowBaseSyncPrimitivesForTesting
......
...@@ -102,7 +102,7 @@ bool SyncFileSystemDeleteFileSystemFunction::RunAsync() { ...@@ -102,7 +102,7 @@ bool SyncFileSystemDeleteFileSystemFunction::RunAsync() {
storage::FileSystemURL file_system_url( storage::FileSystemURL file_system_url(
file_system_context->CrackURL(GURL(url))); file_system_context->CrackURL(GURL(url)));
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
BindOnce( BindOnce(
&storage::FileSystemContext::DeleteFileSystem, file_system_context, &storage::FileSystemContext::DeleteFileSystem, file_system_context,
...@@ -117,7 +117,7 @@ void SyncFileSystemDeleteFileSystemFunction::DidDeleteFileSystem( ...@@ -117,7 +117,7 @@ void SyncFileSystemDeleteFileSystemFunction::DidDeleteFileSystem(
// Repost to switch from IO thread to UI thread for SendResponse(). // Repost to switch from IO thread to UI thread for SendResponse().
if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
BindOnce(&SyncFileSystemDeleteFileSystemFunction::DidDeleteFileSystem, BindOnce(&SyncFileSystemDeleteFileSystemFunction::DidDeleteFileSystem,
this, error)); this, error));
...@@ -146,13 +146,12 @@ bool SyncFileSystemRequestFileSystemFunction::RunAsync() { ...@@ -146,13 +146,12 @@ bool SyncFileSystemRequestFileSystemFunction::RunAsync() {
// Initializes sync context for this extension and continue to open // Initializes sync context for this extension and continue to open
// a new file system. // a new file system.
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO}, BindOnce(&storage::FileSystemContext::OpenFileSystem,
BindOnce(&storage::FileSystemContext::OpenFileSystem, GetFileSystemContext(), source_url().GetOrigin(),
GetFileSystemContext(), source_url().GetOrigin(), storage::kFileSystemTypeSyncable,
storage::kFileSystemTypeSyncable, storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT, base::Bind(&self::DidOpenFileSystem, this)));
base::Bind(&self::DidOpenFileSystem, this)));
return true; return true;
} }
...@@ -171,7 +170,7 @@ void SyncFileSystemRequestFileSystemFunction::DidOpenFileSystem( ...@@ -171,7 +170,7 @@ void SyncFileSystemRequestFileSystemFunction::DidOpenFileSystem(
// Repost to switch from IO thread to UI thread for SendResponse(). // Repost to switch from IO thread to UI thread for SendResponse().
if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
BindOnce(&SyncFileSystemRequestFileSystemFunction::DidOpenFileSystem, BindOnce(&SyncFileSystemRequestFileSystemFunction::DidOpenFileSystem,
this, root_url, file_system_name, error)); this, root_url, file_system_name, error));
...@@ -334,7 +333,7 @@ bool SyncFileSystemGetUsageAndQuotaFunction::RunAsync() { ...@@ -334,7 +333,7 @@ bool SyncFileSystemGetUsageAndQuotaFunction::RunAsync() {
GetProfile(), render_frame_host()->GetSiteInstance()) GetProfile(), render_frame_host()->GetSiteInstance())
->GetQuotaManager(); ->GetQuotaManager();
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
BindOnce( BindOnce(
&storage::QuotaManager::GetUsageAndQuotaForWebApps, quota_manager, &storage::QuotaManager::GetUsageAndQuotaForWebApps, quota_manager,
...@@ -353,7 +352,7 @@ void SyncFileSystemGetUsageAndQuotaFunction::DidGetUsageAndQuota( ...@@ -353,7 +352,7 @@ void SyncFileSystemGetUsageAndQuotaFunction::DidGetUsageAndQuota(
// Repost to switch from IO thread to UI thread for SendResponse(). // Repost to switch from IO thread to UI thread for SendResponse().
if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
BindOnce(&SyncFileSystemGetUsageAndQuotaFunction::DidGetUsageAndQuota, BindOnce(&SyncFileSystemGetUsageAndQuotaFunction::DidGetUsageAndQuota,
this, status, usage, quota)); this, status, usage, quota));
......
...@@ -63,8 +63,9 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest, ...@@ -63,8 +63,9 @@ class SyncFileSystemTest : public extensions::PlatformAppBrowserTest,
SyncFileSystemTest() : remote_service_(nullptr) {} SyncFileSystemTest() : remote_service_(nullptr) {}
scoped_refptr<base::SequencedTaskRunner> MakeSequencedTaskRunner() { scoped_refptr<base::SequencedTaskRunner> MakeSequencedTaskRunner() {
return base::CreateSequencedTaskRunnerWithTraits( return base::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}); {base::ThreadPool(), base::MayBlock(),
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
} }
void SetUpOnMainThread() override { void SetUpOnMainThread() override {
......
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