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

storage: 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=pwnall@chromium.org

Bug: 968047
Change-Id: Iff33707395af8d9edff34551ba90787c2bf8cdfe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729079
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarVictor Costan <pwnall@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682879}
parent ccd9f529
...@@ -154,7 +154,7 @@ class BlobBuilderFromStream::WritePipeToFileHelper ...@@ -154,7 +154,7 @@ class BlobBuilderFromStream::WritePipeToFileHelper
base::FilePath file_path, base::FilePath file_path,
uint64_t max_file_size, uint64_t max_file_size,
DoneCallback callback) { DoneCallback callback) {
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}) base::CreateSequencedTaskRunner({base::ThreadPool(), base::MayBlock()})
->PostTask( ->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce( base::BindOnce(
...@@ -170,7 +170,7 @@ class BlobBuilderFromStream::WritePipeToFileHelper ...@@ -170,7 +170,7 @@ class BlobBuilderFromStream::WritePipeToFileHelper
base::File file, base::File file,
uint64_t max_file_size, uint64_t max_file_size,
DoneCallback callback) { DoneCallback callback) {
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}) base::CreateSequencedTaskRunner({base::ThreadPool(), base::MayBlock()})
->PostTask( ->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&WritePipeToFileHelper::CreateAndStartOnFileSequence, base::BindOnce(&WritePipeToFileHelper::CreateAndStartOnFileSequence,
......
...@@ -52,7 +52,7 @@ class BlobBuilderFromStreamTestWithDelayedLimits ...@@ -52,7 +52,7 @@ class BlobBuilderFromStreamTestWithDelayedLimits
ASSERT_TRUE(data_dir_.CreateUniqueTempDir()); ASSERT_TRUE(data_dir_.CreateUniqueTempDir());
context_ = std::make_unique<BlobStorageContext>( context_ = std::make_unique<BlobStorageContext>(
data_dir_.GetPath(), data_dir_.GetPath(),
base::CreateTaskRunnerWithTraits({base::MayBlock()})); base::CreateTaskRunner({base::ThreadPool(), base::MayBlock()}));
limits_.max_ipc_memory_size = kTestBlobStorageMaxBytesDataItemSize; limits_.max_ipc_memory_size = kTestBlobStorageMaxBytesDataItemSize;
limits_.max_shared_memory_size = kTestBlobStorageMaxBytesDataItemSize; limits_.max_shared_memory_size = kTestBlobStorageMaxBytesDataItemSize;
......
...@@ -71,8 +71,9 @@ int ConvertBlobErrorToNetError(BlobStatus reason) { ...@@ -71,8 +71,9 @@ int ConvertBlobErrorToNetError(BlobStatus reason) {
BlobReader::FileStreamReaderProvider::~FileStreamReaderProvider() = default; BlobReader::FileStreamReaderProvider::~FileStreamReaderProvider() = default;
BlobReader::BlobReader(const BlobDataHandle* blob_handle) BlobReader::BlobReader(const BlobDataHandle* blob_handle)
: file_task_runner_(base::CreateTaskRunnerWithTraits( : file_task_runner_(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE})), base::CreateTaskRunner({base::ThreadPool(), base::MayBlock(),
base::TaskPriority::USER_VISIBLE})),
net_error_(net::OK), net_error_(net::OK),
weak_factory_(this) { weak_factory_(this) {
if (blob_handle) { if (blob_handle) {
......
...@@ -58,7 +58,7 @@ class BlobRegistryImplTest : public testing::Test { ...@@ -58,7 +58,7 @@ class BlobRegistryImplTest : public testing::Test {
ASSERT_TRUE(data_dir_.CreateUniqueTempDir()); ASSERT_TRUE(data_dir_.CreateUniqueTempDir());
context_ = std::make_unique<BlobStorageContext>( context_ = std::make_unique<BlobStorageContext>(
data_dir_.GetPath(), data_dir_.GetPath(),
base::CreateTaskRunnerWithTraits({base::MayBlock()})); base::CreateTaskRunner({base::ThreadPool(), base::MayBlock()}));
auto storage_policy = auto storage_policy =
base::MakeRefCounted<content::MockSpecialStoragePolicy>(); base::MakeRefCounted<content::MockSpecialStoragePolicy>();
file_system_context_ = base::MakeRefCounted<storage::FileSystemContext>( file_system_context_ = base::MakeRefCounted<storage::FileSystemContext>(
...@@ -140,8 +140,8 @@ class BlobRegistryImplTest : public testing::Test { ...@@ -140,8 +140,8 @@ class BlobRegistryImplTest : public testing::Test {
blink::mojom::BytesProviderPtrInfo CreateBytesProvider( blink::mojom::BytesProviderPtrInfo CreateBytesProvider(
const std::string& bytes) { const std::string& bytes) {
if (!bytes_provider_runner_) { if (!bytes_provider_runner_) {
bytes_provider_runner_ = bytes_provider_runner_ = base::CreateSequencedTaskRunner(
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}); {base::ThreadPool(), base::MayBlock()});
} }
blink::mojom::BytesProviderPtrInfo result; blink::mojom::BytesProviderPtrInfo result;
auto provider = std::make_unique<MockBytesProvider>( auto provider = std::make_unique<MockBytesProvider>(
...@@ -156,8 +156,8 @@ class BlobRegistryImplTest : public testing::Test { ...@@ -156,8 +156,8 @@ class BlobRegistryImplTest : public testing::Test {
void CreateBytesProvider(const std::string& bytes, void CreateBytesProvider(const std::string& bytes,
blink::mojom::BytesProviderRequest request) { blink::mojom::BytesProviderRequest request) {
if (!bytes_provider_runner_) { if (!bytes_provider_runner_) {
bytes_provider_runner_ = bytes_provider_runner_ = base::CreateSequencedTaskRunner(
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}); {base::ThreadPool(), base::MayBlock()});
} }
auto provider = std::make_unique<MockBytesProvider>( auto provider = std::make_unique<MockBytesProvider>(
std::vector<uint8_t>(bytes.begin(), bytes.end()), &reply_request_count_, std::vector<uint8_t>(bytes.begin(), bytes.end()), &reply_request_count_,
......
...@@ -51,7 +51,7 @@ class BlobTransportStrategyTest : public testing::Test { ...@@ -51,7 +51,7 @@ class BlobTransportStrategyTest : public testing::Test {
ASSERT_TRUE(data_dir_.CreateUniqueTempDir()); ASSERT_TRUE(data_dir_.CreateUniqueTempDir());
bytes_provider_runner_ = bytes_provider_runner_ =
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()}); base::CreateSequencedTaskRunner({base::ThreadPool(), base::MayBlock()});
mock_time_ = base::Time::Now(); mock_time_ = base::Time::Now();
limits_.max_ipc_memory_size = kTestBlobStorageIPCThresholdBytes; limits_.max_ipc_memory_size = kTestBlobStorageIPCThresholdBytes;
......
...@@ -96,8 +96,9 @@ DatabaseTracker::DatabaseTracker( ...@@ -96,8 +96,9 @@ DatabaseTracker::DatabaseTracker(
db_(new sql::Database()), db_(new sql::Database()),
special_storage_policy_(special_storage_policy), special_storage_policy_(special_storage_policy),
quota_manager_proxy_(quota_manager_proxy), quota_manager_proxy_(quota_manager_proxy),
task_runner_(base::CreateSequencedTaskRunnerWithTraits( task_runner_(base::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE, {base::ThreadPool(), base::MayBlock(),
base::TaskPriority::USER_VISIBLE,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})) { base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})) {
if (quota_manager_proxy) { if (quota_manager_proxy) {
quota_manager_proxy->RegisterClient(new DatabaseQuotaClient(this)); quota_manager_proxy->RegisterClient(new DatabaseQuotaClient(this));
......
...@@ -887,8 +887,9 @@ QuotaManager::QuotaManager( ...@@ -887,8 +887,9 @@ QuotaManager::QuotaManager(
db_disabled_(false), db_disabled_(false),
eviction_disabled_(false), eviction_disabled_(false),
io_thread_(io_thread), io_thread_(io_thread),
db_runner_(base::CreateSequencedTaskRunnerWithTraits( db_runner_(base::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE, {base::ThreadPool(), base::MayBlock(),
base::TaskPriority::USER_VISIBLE,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN})), base::TaskShutdownBehavior::BLOCK_SHUTDOWN})),
get_settings_function_(get_settings_function), get_settings_function_(get_settings_function),
is_getting_eviction_origin_(false), is_getting_eviction_origin_(false),
......
...@@ -138,9 +138,9 @@ void GetNominalDynamicSettings(const base::FilePath& partition_path, ...@@ -138,9 +138,9 @@ void GetNominalDynamicSettings(const base::FilePath& partition_path,
bool is_incognito, bool is_incognito,
QuotaDiskInfoHelper* disk_info_helper, QuotaDiskInfoHelper* disk_info_helper,
OptionalQuotaSettingsCallback callback) { OptionalQuotaSettingsCallback callback) {
base::PostTaskWithTraitsAndReplyWithResult( base::PostTaskAndReplyWithResult(
FROM_HERE, FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT, {base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}, base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
base::BindOnce(&CalculateNominalDynamicSettings, partition_path, base::BindOnce(&CalculateNominalDynamicSettings, partition_path,
is_incognito, base::Unretained(disk_info_helper)), is_incognito, base::Unretained(disk_info_helper)),
......
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