Commit 713cbf92 authored by Sami Kyostila's avatar Sami Kyostila Committed by Commit Bot

mojo: 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=jam@chromium.org

Bug: 968047
Change-Id: I61bc20b765ad68c07e33d14e7ef3e28ee2282c2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729153
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@google.com>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#682852}
parent df677410
...@@ -76,8 +76,9 @@ bool MessageDumper::Accept(mojo::Message* message) { ...@@ -76,8 +76,9 @@ bool MessageDumper::Accept(mojo::Message* message) {
message->interface_name(), message->method_name()); message->interface_name(), message->method_name());
static base::NoDestructor<scoped_refptr<base::TaskRunner>> task_runner( static base::NoDestructor<scoped_refptr<base::TaskRunner>> task_runner(
base::CreateSequencedTaskRunnerWithTraits( base::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_BLOCKING, {base::ThreadPool(), base::MayBlock(),
base::TaskPriority::USER_BLOCKING,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})); base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}));
(*task_runner) (*task_runner)
......
...@@ -39,7 +39,7 @@ class RandomMojoDelays { ...@@ -39,7 +39,7 @@ class RandomMojoDelays {
public: public:
RandomMojoDelays() RandomMojoDelays()
: runner_for_pauses_( : runner_for_pauses_(
base::CreateSequencedTaskRunnerWithTraits(base::TaskTraits())) { base::CreateSequencedTaskRunner({base::ThreadPool()})) {
DETACH_FROM_SEQUENCE(runner_for_pauses_sequence_checker); DETACH_FROM_SEQUENCE(runner_for_pauses_sequence_checker);
} }
...@@ -152,7 +152,7 @@ class RandomMojoDelays { ...@@ -152,7 +152,7 @@ class RandomMojoDelays {
} }
// Set the bindings to resume soon. // Set the bindings to resume soon.
// TODO(mpdenton) may cause deadlock on shutdown if this doesn't run. But // TODO(mpdenton) may cause deadlock on shutdown if this doesn't run. But
// there is no PostDelayedTaskWithTraits for a SequencedTaskRunner. // there is no PostDelayedTask for a SequencedTaskRunner.
if (paused_binding_state_bases.size() > 0) { if (paused_binding_state_bases.size() > 0) {
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, FROM_HERE,
......
...@@ -210,7 +210,7 @@ TEST_F(AssociatedInterfaceTest, InterfacesAtBothEnds) { ...@@ -210,7 +210,7 @@ TEST_F(AssociatedInterfaceTest, InterfacesAtBothEnds) {
class TestSender { class TestSender {
public: public:
TestSender() TestSender()
: task_runner_(base::CreateSequencedTaskRunnerWithTraits({})), : task_runner_(base::CreateSequencedTaskRunner({base::ThreadPool()})),
next_sender_(nullptr), next_sender_(nullptr),
max_value_to_send_(-1) {} max_value_to_send_(-1) {}
...@@ -257,7 +257,7 @@ class TestSender { ...@@ -257,7 +257,7 @@ class TestSender {
class TestReceiver { class TestReceiver {
public: public:
TestReceiver() TestReceiver()
: task_runner_(base::CreateSequencedTaskRunnerWithTraits({})), : task_runner_(base::CreateSequencedTaskRunner({base::ThreadPool()})),
expected_calls_(0) {} expected_calls_(0) {}
void SetUp(PendingAssociatedReceiver<IntegerSender> receiver0, void SetUp(PendingAssociatedReceiver<IntegerSender> receiver0,
...@@ -981,7 +981,8 @@ TEST_F(AssociatedInterfaceTest, SharedAssociatedRemote) { ...@@ -981,7 +981,8 @@ TEST_F(AssociatedInterfaceTest, SharedAssociatedRemote) {
// Test the thread safe pointer can be used from another thread. // Test the thread safe pointer can be used from another thread.
base::RunLoop run_loop; base::RunLoop run_loop;
auto sender_task_runner = base::CreateSequencedTaskRunnerWithTraits({}); auto sender_task_runner =
base::CreateSequencedTaskRunner({base::ThreadPool()});
auto quit_closure = run_loop.QuitClosure(); auto quit_closure = run_loop.QuitClosure();
sender_task_runner->PostTask( sender_task_runner->PostTask(
FROM_HERE, base::BindLambdaForTesting([&] { FROM_HERE, base::BindLambdaForTesting([&] {
...@@ -1005,7 +1006,7 @@ struct ForwarderTestContext { ...@@ -1005,7 +1006,7 @@ struct ForwarderTestContext {
TEST_F(AssociatedInterfaceTest, SharedAssociatedRemoteWithTaskRunner) { TEST_F(AssociatedInterfaceTest, SharedAssociatedRemoteWithTaskRunner) {
const scoped_refptr<base::SequencedTaskRunner> other_thread_task_runner = const scoped_refptr<base::SequencedTaskRunner> other_thread_task_runner =
base::CreateSequencedTaskRunnerWithTraits({}); base::CreateSequencedTaskRunner({base::ThreadPool()});
ForwarderTestContext* context = new ForwarderTestContext(); ForwarderTestContext* context = new ForwarderTestContext();
PendingAssociatedRemote<IntegerSender> pending_remote; PendingAssociatedRemote<IntegerSender> pending_remote;
......
...@@ -258,7 +258,7 @@ TEST_P(EndToEndRemoteTest, EndToEnd) { ...@@ -258,7 +258,7 @@ TEST_P(EndToEndRemoteTest, EndToEnd) {
} }
TEST_P(EndToEndRemoteTest, EndToEndOnSequence) { TEST_P(EndToEndRemoteTest, EndToEndOnSequence) {
RunTest(base::CreateSequencedTaskRunnerWithTraits({})); RunTest(base::CreateSequencedTaskRunner({base::ThreadPool()}));
} }
TEST_P(RemoteTest, Movable) { TEST_P(RemoteTest, Movable) {
...@@ -877,7 +877,8 @@ TEST_P(RemoteTest, SharedRemote) { ...@@ -877,7 +877,8 @@ TEST_P(RemoteTest, SharedRemote) {
// Send a message on |thread_safe_remote| from a different sequence. // Send a message on |thread_safe_remote| from a different sequence.
auto main_task_runner = base::SequencedTaskRunnerHandle::Get(); auto main_task_runner = base::SequencedTaskRunnerHandle::Get();
auto sender_task_runner = base::CreateSequencedTaskRunnerWithTraits({}); auto sender_task_runner =
base::CreateSequencedTaskRunner({base::ThreadPool()});
sender_task_runner->PostTask( sender_task_runner->PostTask(
FROM_HERE, base::BindLambdaForTesting([&] { FROM_HERE, base::BindLambdaForTesting([&] {
shared_remote->Add( shared_remote->Add(
...@@ -894,7 +895,7 @@ TEST_P(RemoteTest, SharedRemote) { ...@@ -894,7 +895,7 @@ TEST_P(RemoteTest, SharedRemote) {
TEST_P(RemoteTest, SharedRemoteWithTaskRunner) { TEST_P(RemoteTest, SharedRemoteWithTaskRunner) {
const scoped_refptr<base::SequencedTaskRunner> other_thread_task_runner = const scoped_refptr<base::SequencedTaskRunner> other_thread_task_runner =
base::CreateSequencedTaskRunnerWithTraits({}); base::CreateSequencedTaskRunner({base::ThreadPool()});
PendingRemote<math::Calculator> remote; PendingRemote<math::Calculator> remote;
auto receiver = remote.InitWithNewPipeAndPassReceiver(); auto receiver = remote.InitWithNewPipeAndPassReceiver();
......
...@@ -252,7 +252,7 @@ template <typename Interface> ...@@ -252,7 +252,7 @@ template <typename Interface>
class TestSyncServiceSequence { class TestSyncServiceSequence {
public: public:
TestSyncServiceSequence() TestSyncServiceSequence()
: task_runner_(base::CreateSequencedTaskRunnerWithTraits({})), : task_runner_(base::CreateSequencedTaskRunner({base::ThreadPool()})),
ping_called_(false) {} ping_called_(false) {}
void SetUp(InterfaceRequest<Interface> request) { void SetUp(InterfaceRequest<Interface> request) {
...@@ -432,7 +432,8 @@ void RunTestOnSequencedTaskRunner( ...@@ -432,7 +432,8 @@ void RunTestOnSequencedTaskRunner(
std::unique_ptr<SequencedTaskRunnerTestBase> test) { std::unique_ptr<SequencedTaskRunnerTestBase> test) {
base::RunLoop run_loop; base::RunLoop run_loop;
test->Init(run_loop.QuitClosure()); test->Init(run_loop.QuitClosure());
base::CreateSequencedTaskRunnerWithTraits({base::WithBaseSyncPrimitives()}) base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::WithBaseSyncPrimitives()})
->PostTask(FROM_HERE, ->PostTask(FROM_HERE,
base::BindOnce(&SequencedTaskRunnerTestBase::RunTest, base::BindOnce(&SequencedTaskRunnerTestBase::RunTest,
base::Unretained(test.release()))); base::Unretained(test.release())));
......
...@@ -184,8 +184,8 @@ void DataPipeProducer::InitializeNewRequest(CompletionCallback callback) { ...@@ -184,8 +184,8 @@ void DataPipeProducer::InitializeNewRequest(CompletionCallback callback) {
// TODO(crbug.com/924416): Re-evaluate how TaskPriority is set here and in // TODO(crbug.com/924416): Re-evaluate how TaskPriority is set here and in
// other file URL-loading-related code. Some callers require USER_VISIBLE // other file URL-loading-related code. Some callers require USER_VISIBLE
// (i.e., BEST_EFFORT is not enough). // (i.e., BEST_EFFORT is not enough).
auto file_task_runner = base::CreateSequencedTaskRunnerWithTraits( auto file_task_runner = base::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE}); {base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE});
sequence_state_ = new SequenceState( sequence_state_ = new SequenceState(
std::move(producer_), file_task_runner, std::move(producer_), file_task_runner,
base::BindOnce(&DataPipeProducer::OnWriteComplete, base::BindOnce(&DataPipeProducer::OnWriteComplete,
......
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