Commit 2917593c authored by Sami Kyostila's avatar Sami Kyostila Committed by Commit Bot

third_party: 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=grunell@chromium.org, jbroman@chromium.org

Bug: 968047
Change-Id: Id8a11433f2c3e2d37c5452bab643ed098441b435
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728886
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Reviewed-by: default avatarHenrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683513}
parent 48e765ca
...@@ -467,10 +467,10 @@ bool ScriptStreamer::TryStartStreaming( ...@@ -467,10 +467,10 @@ bool ScriptStreamer::TryStartStreaming(
// Script streaming tasks are high priority, as they can block the parser, // Script streaming tasks are high priority, as they can block the parser,
// and they can (and probably will) block during their own execution as // and they can (and probably will) block during their own execution as
// they wait for more input. // they wait for more input.
//
// TODO(leszeks): Decrease the priority of these tasks where possible. // TODO(leszeks): Decrease the priority of these tasks where possible.
worker_pool::PostTaskWithTraits( worker_pool::PostTask(
FROM_HERE, {base::TaskPriority::USER_BLOCKING, base::MayBlock()}, FROM_HERE,
{base::ThreadPool(), base::TaskPriority::USER_BLOCKING, base::MayBlock()},
CrossThreadBindOnce(RunScriptStreamingTask, CrossThreadBindOnce(RunScriptStreamingTask,
WTF::Passed(std::move(script_streaming_task)), WTF::Passed(std::move(script_streaming_task)),
WrapCrossThreadPersistent(this), WrapCrossThreadPersistent(this),
......
...@@ -57,7 +57,7 @@ TEST(StorageControllerTest, CacheLimit) { ...@@ -57,7 +57,7 @@ TEST(StorageControllerTest, CacheLimit) {
mojom::blink::StoragePartitionServicePtr storage_partition_service_ptr; mojom::blink::StoragePartitionServicePtr storage_partition_service_ptr;
PostCrossThreadTask( PostCrossThreadTask(
*base::CreateSequencedTaskRunnerWithTraits({}), FROM_HERE, *base::CreateSequencedTaskRunner({base::ThreadPool()}), FROM_HERE,
CrossThreadBindOnce( CrossThreadBindOnce(
[](mojom::blink::StoragePartitionServiceRequest request) { [](mojom::blink::StoragePartitionServiceRequest request) {
mojo::MakeStrongBinding( mojo::MakeStrongBinding(
...@@ -108,7 +108,7 @@ TEST(StorageControllerTest, CacheLimitSessionStorage) { ...@@ -108,7 +108,7 @@ TEST(StorageControllerTest, CacheLimitSessionStorage) {
Persistent<FakeAreaSource> source_area = Persistent<FakeAreaSource> source_area =
MakeGarbageCollected<FakeAreaSource>(kPageUrl); MakeGarbageCollected<FakeAreaSource>(kPageUrl);
auto task_runner = base::CreateSequencedTaskRunnerWithTraits({}); auto task_runner = base::CreateSequencedTaskRunner({base::ThreadPool()});
auto mock_storage_partition_service = auto mock_storage_partition_service =
std::make_unique<MockStoragePartitionService>(); std::make_unique<MockStoragePartitionService>();
......
...@@ -53,7 +53,7 @@ TEST_F(StorageNamespaceTest, BasicStorageAreas) { ...@@ -53,7 +53,7 @@ TEST_F(StorageNamespaceTest, BasicStorageAreas) {
mojom::blink::StoragePartitionServicePtr storage_partition_service_ptr; mojom::blink::StoragePartitionServicePtr storage_partition_service_ptr;
PostCrossThreadTask( PostCrossThreadTask(
*base::CreateSequencedTaskRunnerWithTraits({}), FROM_HERE, *base::CreateSequencedTaskRunner({base::ThreadPool()}), FROM_HERE,
CrossThreadBindOnce( CrossThreadBindOnce(
[](mojom::blink::StoragePartitionServiceRequest request) { [](mojom::blink::StoragePartitionServiceRequest request) {
mojo::MakeStrongBinding( mojo::MakeStrongBinding(
......
...@@ -38,8 +38,9 @@ mojom::blink::WebDatabaseHost& WebDatabaseHost::GetWebDatabaseHost() { ...@@ -38,8 +38,9 @@ mojom::blink::WebDatabaseHost& WebDatabaseHost::GetWebDatabaseHost() {
if (!shared_remote_) { if (!shared_remote_) {
DCHECK(pending_remote_); DCHECK(pending_remote_);
shared_remote_ = mojo::SharedRemote<mojom::blink::WebDatabaseHost>( shared_remote_ = mojo::SharedRemote<mojom::blink::WebDatabaseHost>(
std::move(pending_remote_), base::CreateSequencedTaskRunnerWithTraits( std::move(pending_remote_),
{base::WithBaseSyncPrimitives()})); base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::WithBaseSyncPrimitives()}));
} }
return *shared_remote_; return *shared_remote_;
......
...@@ -117,8 +117,8 @@ constexpr size_t BlobBytesProvider::kMaxConsolidatedItemSizeInBytes; ...@@ -117,8 +117,8 @@ constexpr size_t BlobBytesProvider::kMaxConsolidatedItemSizeInBytes;
// static // static
BlobBytesProvider* BlobBytesProvider::CreateAndBind( BlobBytesProvider* BlobBytesProvider::CreateAndBind(
mojom::blink::BytesProviderRequest request) { mojom::blink::BytesProviderRequest request) {
auto task_runner = base::CreateSequencedTaskRunnerWithTraits( auto task_runner = base::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE}); {base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE});
auto provider = base::WrapUnique(new BlobBytesProvider(task_runner)); auto provider = base::WrapUnique(new BlobBytesProvider(task_runner));
auto* result = provider.get(); auto* result = provider.get();
// TODO(mek): Consider binding BytesProvider on the IPC thread instead, only // TODO(mek): Consider binding BytesProvider on the IPC thread instead, only
......
...@@ -385,8 +385,9 @@ void MediaStreamAudioProcessor::OnStartDump(base::File dump_file) { ...@@ -385,8 +385,9 @@ void MediaStreamAudioProcessor::OnStartDump(base::File dump_file) {
std::move(dump_file), worker_queue_.get()); std::move(dump_file), worker_queue_.get());
} else { } else {
// Post the file close to avoid blocking the main thread. // Post the file close to avoid blocking the main thread.
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {base::TaskPriority::LOWEST, base::MayBlock()}, FROM_HERE,
{base::ThreadPool(), base::TaskPriority::LOWEST, base::MayBlock()},
base::BindOnce([](base::File) {}, std::move(dump_file))); base::BindOnce([](base::File) {}, std::move(dump_file)));
} }
} }
......
...@@ -59,8 +59,9 @@ void AudioServiceAudioProcessorProxy::OnStartDump(base::File dump_file) { ...@@ -59,8 +59,9 @@ void AudioServiceAudioProcessorProxy::OnStartDump(base::File dump_file) {
processor_controls_->StartEchoCancellationDump(std::move(dump_file)); processor_controls_->StartEchoCancellationDump(std::move(dump_file));
} else { } else {
// Post the file close to avoid blocking the main thread. // Post the file close to avoid blocking the main thread.
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {base::TaskPriority::LOWEST, base::MayBlock()}, FROM_HERE,
{base::ThreadPool(), base::TaskPriority::LOWEST, base::MayBlock()},
base::BindOnce([](base::File) {}, std::move(dump_file))); base::BindOnce([](base::File) {}, std::move(dump_file)));
} }
} }
......
...@@ -155,7 +155,7 @@ worker_pool::PostTask, which uses a thread pool ...@@ -155,7 +155,7 @@ worker_pool::PostTask, which uses a thread pool
behind the scenes. behind the scenes.
Do not create your own dedicated thread if you need ordering for your tasks, Do not create your own dedicated thread if you need ordering for your tasks,
use worker_pool::CreateTaskRunnerWithTraits instead — use worker_pool::CreateTaskRunner instead —
this creates a sequence (virtual thread which can run tasks in order on this creates a sequence (virtual thread which can run tasks in order on
any of the threads in the thread pool). any of the threads in the thread pool).
(Note: this doesn't exist yet because we haven't encountered a use case in Blink (Note: this doesn't exist yet because we haven't encountered a use case in Blink
......
...@@ -12,16 +12,17 @@ namespace blink { ...@@ -12,16 +12,17 @@ namespace blink {
namespace worker_pool { namespace worker_pool {
void PostTask(const base::Location& location, CrossThreadOnceClosure closure) { void PostTask(const base::Location& location, CrossThreadOnceClosure closure) {
PostTaskWithTraits(location, PostTask(
{base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}, location,
std::move(closure)); {base::ThreadPool(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
std::move(closure));
} }
void PostTaskWithTraits(const base::Location& location, void PostTask(const base::Location& location,
const base::TaskTraits& traits, const base::TaskTraits& traits,
CrossThreadOnceClosure closure) { CrossThreadOnceClosure closure) {
base::PostTaskWithTraits(location, traits, base::PostTask(location, traits,
ConvertToBaseOnceCallback(std::move(closure))); ConvertToBaseOnceCallback(std::move(closure)));
} }
} // namespace worker_pool } // namespace worker_pool
......
...@@ -27,11 +27,11 @@ namespace worker_pool { ...@@ -27,11 +27,11 @@ namespace worker_pool {
// (e.g. FrameScheduler for main thread tasks). // (e.g. FrameScheduler for main thread tasks).
PLATFORM_EXPORT void PostTask(const base::Location&, CrossThreadOnceClosure); PLATFORM_EXPORT void PostTask(const base::Location&, CrossThreadOnceClosure);
PLATFORM_EXPORT void PostTaskWithTraits(const base::Location&, PLATFORM_EXPORT void PostTask(const base::Location&,
const base::TaskTraits&, const base::TaskTraits&,
CrossThreadOnceClosure); CrossThreadOnceClosure);
// TODO(altimin): Expose CreateSequencedTaskRunnerWithTraits when the // TODO(altimin): Expose CreateSequencedTaskRunner when the
// need arises. // need arises.
} // namespace worker_pool } // namespace worker_pool
......
...@@ -33,7 +33,7 @@ _CONFIG = [ ...@@ -33,7 +33,7 @@ _CONFIG = [
'base::AdoptRef', 'base::AdoptRef',
'base::AutoReset', 'base::AutoReset',
'base::Contains', 'base::Contains',
'base::CreateSequencedTaskRunnerWithTraits', 'base::CreateSequencedTaskRunner',
'base::DefaultTickClock', 'base::DefaultTickClock',
'base::ElapsedTimer', 'base::ElapsedTimer',
'base::File', 'base::File',
...@@ -182,6 +182,7 @@ _CONFIG = [ ...@@ -182,6 +182,7 @@ _CONFIG = [
'base::TaskPriority', 'base::TaskPriority',
'base::TaskShutdownBehavior', 'base::TaskShutdownBehavior',
'base::WithBaseSyncPrimitives', 'base::WithBaseSyncPrimitives',
'base::ThreadPool',
# Byte order # Byte order
'base::ByteSwap', 'base::ByteSwap',
......
...@@ -19,7 +19,7 @@ namespace { ...@@ -19,7 +19,7 @@ namespace {
class WebrtcTaskQueue final : public webrtc::TaskQueueBase { class WebrtcTaskQueue final : public webrtc::TaskQueueBase {
public: public:
explicit WebrtcTaskQueue(const base::TaskTraits& traits) explicit WebrtcTaskQueue(const base::TaskTraits& traits)
: task_runner_(base::CreateSequencedTaskRunnerWithTraits(traits)), : task_runner_(base::CreateSequencedTaskRunner(traits)),
is_active_(new base::RefCountedData<bool>(true)) { is_active_(new base::RefCountedData<bool>(true)) {
DCHECK(task_runner_); DCHECK(task_runner_);
} }
...@@ -102,19 +102,21 @@ base::TaskTraits TaskQueuePriority2Traits( ...@@ -102,19 +102,21 @@ base::TaskTraits TaskQueuePriority2Traits(
switch (priority) { switch (priority) {
case webrtc::TaskQueueFactory::Priority::HIGH: case webrtc::TaskQueueFactory::Priority::HIGH:
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
return {base::WithBaseSyncPrimitives(), base::TaskPriority::HIGHEST}; return {base::ThreadPool(), base::WithBaseSyncPrimitives(),
base::TaskPriority::HIGHEST};
#else #else
return {base::TaskPriority::HIGHEST}; return {base::ThreadPool(), base::TaskPriority::HIGHEST};
#endif #endif
break; break;
case webrtc::TaskQueueFactory::Priority::LOW: case webrtc::TaskQueueFactory::Priority::LOW:
return {base::MayBlock(), base::TaskPriority::BEST_EFFORT}; return {base::ThreadPool(), base::MayBlock(),
base::TaskPriority::BEST_EFFORT};
case webrtc::TaskQueueFactory::Priority::NORMAL: case webrtc::TaskQueueFactory::Priority::NORMAL:
default: default:
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
return {base::WithBaseSyncPrimitives()}; return {base::ThreadPool(), base::WithBaseSyncPrimitives()};
#else #else
return {}; return {base::ThreadPool()};
#endif #endif
} }
} }
......
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