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

chrome/browser/service_process: 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=rbpotter@chromium.org

Bug: 968047
Change-Id: I5ec1af17a9d1408c6dc656abc9c44a5fcd47bb74
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1739866
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684437}
parent b011a4f1
...@@ -60,8 +60,10 @@ void ConnectAsyncWithBackoff( ...@@ -60,8 +60,10 @@ void ConnectAsyncWithBackoff(
response_task_runner->PostTask( response_task_runner->PostTask(
FROM_HERE, base::BindOnce(std::move(response_callback), nullptr)); FROM_HERE, base::BindOnce(std::move(response_callback), nullptr));
} else { } else {
base::PostDelayedTaskWithTraits( base::PostDelayedTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, FROM_HERE,
{base::ThreadPool(), base::MayBlock(),
base::TaskPriority::BEST_EFFORT},
base::BindOnce( base::BindOnce(
&ConnectAsyncWithBackoff, std::move(interface_provider_request), &ConnectAsyncWithBackoff, std::move(interface_provider_request),
server_name, num_retries_left - 1, retry_delay * 2, server_name, num_retries_left - 1, retry_delay * 2,
...@@ -104,8 +106,9 @@ void ServiceProcessControl::ConnectInternal() { ...@@ -104,8 +106,9 @@ void ServiceProcessControl::ConnectInternal() {
service_manager::mojom::InterfaceProviderPtr remote_interfaces; service_manager::mojom::InterfaceProviderPtr remote_interfaces;
auto interface_provider_request = mojo::MakeRequest(&remote_interfaces); auto interface_provider_request = mojo::MakeRequest(&remote_interfaces);
SetMojoHandle(std::move(remote_interfaces)); SetMojoHandle(std::move(remote_interfaces));
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT}, FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce( base::BindOnce(
&ConnectAsyncWithBackoff, std::move(interface_provider_request), &ConnectAsyncWithBackoff, std::move(interface_provider_request),
GetServiceProcessServerName(), kMaxConnectionAttempts, GetServiceProcessServerName(), kMaxConnectionAttempts,
...@@ -299,9 +302,8 @@ bool ServiceProcessControl::GetHistograms( ...@@ -299,9 +302,8 @@ bool ServiceProcessControl::GetHistograms(
// Run timeout task to make sure |histograms_callback| is called. // Run timeout task to make sure |histograms_callback| is called.
histograms_timeout_callback_.Reset(base::Bind( histograms_timeout_callback_.Reset(base::Bind(
&ServiceProcessControl::RunHistogramsCallback, base::Unretained(this))); &ServiceProcessControl::RunHistogramsCallback, base::Unretained(this)));
base::PostDelayedTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostDelayedTask(FROM_HERE, {BrowserThread::UI},
histograms_timeout_callback_.callback(), histograms_timeout_callback_.callback(), timeout);
timeout);
histograms_callback_ = histograms_callback; histograms_callback_ = histograms_callback;
return true; return true;
...@@ -356,7 +358,7 @@ void ServiceProcessControl::Launcher::DoDetectLaunched() { ...@@ -356,7 +358,7 @@ void ServiceProcessControl::Launcher::DoDetectLaunched() {
if (launched_ || (retry_count_ >= kMaxLaunchDetectRetries) || if (launched_ || (retry_count_ >= kMaxLaunchDetectRetries) ||
process_.WaitForExitWithTimeout(base::TimeDelta(), &exit_code)) { process_.WaitForExitWithTimeout(base::TimeDelta(), &exit_code)) {
process_.Close(); process_.Close();
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Launcher::Notify, this)); base::BindOnce(&Launcher::Notify, this));
return; return;
} }
...@@ -379,10 +381,10 @@ void ServiceProcessControl::Launcher::DoRun() { ...@@ -379,10 +381,10 @@ void ServiceProcessControl::Launcher::DoRun() {
process_ = base::LaunchProcess(*cmd_line_, options); process_ = base::LaunchProcess(*cmd_line_, options);
if (process_.IsValid()) { if (process_.IsValid()) {
saved_pid_ = process_.Pid(); saved_pid_ = process_.Pid();
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO}, base::PostTask(FROM_HERE, {BrowserThread::IO},
base::BindOnce(&Launcher::DoDetectLaunched, this)); base::BindOnce(&Launcher::DoDetectLaunched, this));
} else { } else {
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Launcher::Notify, this)); base::BindOnce(&Launcher::Notify, this));
} }
} }
......
...@@ -18,6 +18,6 @@ using content::BrowserThread; ...@@ -18,6 +18,6 @@ using content::BrowserThread;
void ServiceProcessControl::Launcher::DoRun() { void ServiceProcessControl::Launcher::DoRun() {
launched_ = mac::services::SubmitJob( launched_ = mac::services::SubmitJob(
GetServiceProcessJobOptions(cmd_line_.get(), false)); GetServiceProcessJobOptions(cmd_line_.get(), false));
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Launcher::Notify, this)); base::BindOnce(&Launcher::Notify, this));
} }
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