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

content/browser/background_fetch: 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=rayankans@chromium.org

Bug: 968047
Change-Id: Iac4a5f35210b2baf7280bc0d9be89b68183c2ba9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729257
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarRayan Kanso <rayankans@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684380}
parent 05a13f1c
...@@ -174,7 +174,7 @@ void BackgroundFetchContext::DidGetPermission( ...@@ -174,7 +174,7 @@ void BackgroundFetchContext::DidGetPermission(
BackgroundFetchPermission permission) { BackgroundFetchPermission permission) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce(&background_fetch::RecordBackgroundFetchUkmEvent, base::BindOnce(&background_fetch::RecordBackgroundFetchUkmEvent,
registration_id.origin(), requests.size(), options.Clone(), registration_id.origin(), requests.size(), options.Clone(),
...@@ -291,8 +291,7 @@ void BackgroundFetchContext::DidGetMatchingRequests( ...@@ -291,8 +291,7 @@ void BackgroundFetchContext::DidGetMatchingRequests(
void BackgroundFetchContext::Shutdown() { void BackgroundFetchContext::Shutdown() {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO},
base::BindOnce(&BackgroundFetchContext::ShutdownOnIO, this)); base::BindOnce(&BackgroundFetchContext::ShutdownOnIO, this));
} }
......
...@@ -45,7 +45,7 @@ class BackgroundFetchDelegateProxy::Core ...@@ -45,7 +45,7 @@ class BackgroundFetchDelegateProxy::Core
BackgroundFetchDelegate::GetPermissionForOriginCallback callback, BackgroundFetchDelegate::GetPermissionForOriginCallback callback,
BackgroundFetchPermission permission) { BackgroundFetchPermission permission) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO}, base::PostTask(FROM_HERE, {BrowserThread::IO},
base::BindOnce(std::move(callback), permission)); base::BindOnce(std::move(callback), permission));
} }
...@@ -69,7 +69,7 @@ class BackgroundFetchDelegateProxy::Core ...@@ -69,7 +69,7 @@ class BackgroundFetchDelegateProxy::Core
BackgroundFetchDelegate::GetIconDisplaySizeCallback callback, BackgroundFetchDelegate::GetIconDisplaySizeCallback callback,
const gfx::Size& display_size) { const gfx::Size& display_size) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO}, base::PostTask(FROM_HERE, {BrowserThread::IO},
base::BindOnce(std::move(callback), display_size)); base::BindOnce(std::move(callback), display_size));
} }
...@@ -82,8 +82,7 @@ class BackgroundFetchDelegateProxy::Core ...@@ -82,8 +82,7 @@ class BackgroundFetchDelegateProxy::Core
base::BindOnce(&Core::ForwardGetIconDisplaySizeCallbackToIO, base::BindOnce(&Core::ForwardGetIconDisplaySizeCallbackToIO,
weak_ptr_factory_.GetWeakPtr(), std::move(callback))); weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
} else { } else {
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO},
base::BindOnce(std::move(callback), gfx::Size(0, 0))); base::BindOnce(std::move(callback), gfx::Size(0, 0)));
} }
} }
...@@ -223,10 +222,9 @@ void BackgroundFetchDelegateProxy::Core::OnJobCancelled( ...@@ -223,10 +222,9 @@ void BackgroundFetchDelegateProxy::Core::OnJobCancelled(
const std::string& job_unique_id, const std::string& job_unique_id,
blink::mojom::BackgroundFetchFailureReason reason_to_abort) { blink::mojom::BackgroundFetchFailureReason reason_to_abort) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO}, base::BindOnce(&BackgroundFetchDelegateProxy::OnJobCancelled,
base::BindOnce(&BackgroundFetchDelegateProxy::OnJobCancelled, io_parent_, io_parent_, job_unique_id, reason_to_abort));
job_unique_id, reason_to_abort));
} }
void BackgroundFetchDelegateProxy::Core::OnDownloadUpdated( void BackgroundFetchDelegateProxy::Core::OnDownloadUpdated(
...@@ -235,7 +233,7 @@ void BackgroundFetchDelegateProxy::Core::OnDownloadUpdated( ...@@ -235,7 +233,7 @@ void BackgroundFetchDelegateProxy::Core::OnDownloadUpdated(
uint64_t bytes_uploaded, uint64_t bytes_uploaded,
uint64_t bytes_downloaded) { uint64_t bytes_downloaded) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce(&BackgroundFetchDelegateProxy::OnDownloadUpdated, base::BindOnce(&BackgroundFetchDelegateProxy::OnDownloadUpdated,
io_parent_, job_unique_id, guid, bytes_uploaded, io_parent_, job_unique_id, guid, bytes_uploaded,
...@@ -247,7 +245,7 @@ void BackgroundFetchDelegateProxy::Core::OnDownloadComplete( ...@@ -247,7 +245,7 @@ void BackgroundFetchDelegateProxy::Core::OnDownloadComplete(
const std::string& guid, const std::string& guid,
std::unique_ptr<BackgroundFetchResult> result) { std::unique_ptr<BackgroundFetchResult> result) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce(&BackgroundFetchDelegateProxy::OnDownloadComplete, base::BindOnce(&BackgroundFetchDelegateProxy::OnDownloadComplete,
io_parent_, job_unique_id, guid, std::move(result))); io_parent_, job_unique_id, guid, std::move(result)));
...@@ -259,7 +257,7 @@ void BackgroundFetchDelegateProxy::Core::OnDownloadStarted( ...@@ -259,7 +257,7 @@ void BackgroundFetchDelegateProxy::Core::OnDownloadStarted(
std::unique_ptr<content::BackgroundFetchResponse> response) { std::unique_ptr<content::BackgroundFetchResponse> response) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce(&BackgroundFetchDelegateProxy::DidStartRequest, io_parent_, base::BindOnce(&BackgroundFetchDelegateProxy::DidStartRequest, io_parent_,
job_unique_id, guid, std::move(response))); job_unique_id, guid, std::move(response)));
...@@ -269,20 +267,18 @@ void BackgroundFetchDelegateProxy::Core::OnUIActivated( ...@@ -269,20 +267,18 @@ void BackgroundFetchDelegateProxy::Core::OnUIActivated(
const std::string& job_unique_id) { const std::string& job_unique_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO}, base::BindOnce(&BackgroundFetchDelegateProxy::DidActivateUI,
base::BindOnce(&BackgroundFetchDelegateProxy::DidActivateUI, io_parent_, io_parent_, job_unique_id));
job_unique_id));
} }
void BackgroundFetchDelegateProxy::Core::OnUIUpdated( void BackgroundFetchDelegateProxy::Core::OnUIUpdated(
const std::string& job_unique_id) { const std::string& job_unique_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO}, base::BindOnce(&BackgroundFetchDelegateProxy::DidUpdateUI,
base::BindOnce(&BackgroundFetchDelegateProxy::DidUpdateUI, io_parent_, io_parent_, job_unique_id));
job_unique_id));
} }
void BackgroundFetchDelegateProxy::Core::GetUploadData( void BackgroundFetchDelegateProxy::Core::GetUploadData(
...@@ -297,16 +293,15 @@ void BackgroundFetchDelegateProxy::Core::GetUploadData( ...@@ -297,16 +293,15 @@ void BackgroundFetchDelegateProxy::Core::GetUploadData(
base::BindOnce( base::BindOnce(
[](BackgroundFetchDelegate::GetUploadDataCallback callback, [](BackgroundFetchDelegate::GetUploadDataCallback callback,
blink::mojom::SerializedBlobPtr blob) { blink::mojom::SerializedBlobPtr blob) {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce(std::move(callback), std::move(blob))); base::BindOnce(std::move(callback), std::move(blob)));
}, },
std::move(callback)); std::move(callback));
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO}, base::BindOnce(&BackgroundFetchDelegateProxy::GetUploadData,
base::BindOnce(&BackgroundFetchDelegateProxy::GetUploadData, io_parent_, io_parent_, job_unique_id, download_guid,
job_unique_id, download_guid,
std::move(wrapped_callback))); std::move(wrapped_callback)));
} }
...@@ -336,9 +331,9 @@ void BackgroundFetchDelegateProxy::SetClickEventDispatcher( ...@@ -336,9 +331,9 @@ void BackgroundFetchDelegateProxy::SetClickEventDispatcher(
void BackgroundFetchDelegateProxy::GetIconDisplaySize( void BackgroundFetchDelegateProxy::GetIconDisplaySize(
BackgroundFetchDelegate::GetIconDisplaySizeCallback callback) { BackgroundFetchDelegate::GetIconDisplaySizeCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Core::GetIconDisplaySize, base::BindOnce(&Core::GetIconDisplaySize, ui_core_ptr_,
ui_core_ptr_, std::move(callback))); std::move(callback)));
} }
void BackgroundFetchDelegateProxy::GetPermissionForOrigin( void BackgroundFetchDelegateProxy::GetPermissionForOrigin(
...@@ -346,10 +341,9 @@ void BackgroundFetchDelegateProxy::GetPermissionForOrigin( ...@@ -346,10 +341,9 @@ void BackgroundFetchDelegateProxy::GetPermissionForOrigin(
const WebContents::Getter& wc_getter, const WebContents::Getter& wc_getter,
BackgroundFetchDelegate::GetPermissionForOriginCallback callback) { BackgroundFetchDelegate::GetPermissionForOriginCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::UI},
FROM_HERE, {BrowserThread::UI}, base::BindOnce(&Core::GetPermissionForOrigin, ui_core_ptr_,
base::BindOnce(&Core::GetPermissionForOrigin, ui_core_ptr_, origin, origin, wc_getter, std::move(callback)));
wc_getter, std::move(callback)));
} }
void BackgroundFetchDelegateProxy::CreateDownloadJob( void BackgroundFetchDelegateProxy::CreateDownloadJob(
...@@ -360,8 +354,7 @@ void BackgroundFetchDelegateProxy::CreateDownloadJob( ...@@ -360,8 +354,7 @@ void BackgroundFetchDelegateProxy::CreateDownloadJob(
DCHECK(!controller_map_.count(fetch_description->job_unique_id)); DCHECK(!controller_map_.count(fetch_description->job_unique_id));
controller_map_[fetch_description->job_unique_id] = std::move(controller); controller_map_[fetch_description->job_unique_id] = std::move(controller);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::UI},
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Core::CreateDownloadJob, ui_core_ptr_, base::BindOnce(&Core::CreateDownloadJob, ui_core_ptr_,
std::move(fetch_description))); std::move(fetch_description)));
} }
...@@ -376,7 +369,7 @@ void BackgroundFetchDelegateProxy::StartRequest( ...@@ -376,7 +369,7 @@ void BackgroundFetchDelegateProxy::StartRequest(
DCHECK(request); DCHECK(request);
DCHECK(!request->download_guid().empty()); DCHECK(!request->download_guid().empty());
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Core::StartRequest, ui_core_ptr_, base::BindOnce(&Core::StartRequest, ui_core_ptr_,
job_unique_id, origin, request)); job_unique_id, origin, request));
} }
...@@ -392,22 +385,21 @@ void BackgroundFetchDelegateProxy::UpdateUI( ...@@ -392,22 +385,21 @@ void BackgroundFetchDelegateProxy::UpdateUI(
DCHECK(!update_ui_callback_map_.count(job_unique_id)); DCHECK(!update_ui_callback_map_.count(job_unique_id));
update_ui_callback_map_.emplace(job_unique_id, std::move(update_ui_callback)); update_ui_callback_map_.emplace(job_unique_id, std::move(update_ui_callback));
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Core::UpdateUI, ui_core_ptr_, base::BindOnce(&Core::UpdateUI, ui_core_ptr_, job_unique_id,
job_unique_id, title, icon)); title, icon));
} }
void BackgroundFetchDelegateProxy::Abort(const std::string& job_unique_id) { void BackgroundFetchDelegateProxy::Abort(const std::string& job_unique_id) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {BrowserThread::UI},
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Core::Abort, ui_core_ptr_, job_unique_id)); base::BindOnce(&Core::Abort, ui_core_ptr_, job_unique_id));
} }
void BackgroundFetchDelegateProxy::MarkJobComplete( void BackgroundFetchDelegateProxy::MarkJobComplete(
const std::string& job_unique_id) { const std::string& job_unique_id) {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce(&Core::MarkJobComplete, ui_core_ptr_, job_unique_id)); base::BindOnce(&Core::MarkJobComplete, ui_core_ptr_, job_unique_id));
controller_map_.erase(job_unique_id); controller_map_.erase(job_unique_id);
......
...@@ -70,7 +70,7 @@ class FakeBackgroundFetchDelegate : public BackgroundFetchDelegate { ...@@ -70,7 +70,7 @@ class FakeBackgroundFetchDelegate : public BackgroundFetchDelegate {
job_id_to_client_[job_unique_id]->OnDownloadStarted(job_unique_id, guid, job_id_to_client_[job_unique_id]->OnDownloadStarted(job_unique_id, guid,
std::move(response)); std::move(response));
if (complete_downloads_) { if (complete_downloads_) {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce(&FakeBackgroundFetchDelegate::CompleteDownload, base::BindOnce(&FakeBackgroundFetchDelegate::CompleteDownload,
base::Unretained(this), job_unique_id, guid)); base::Unretained(this), job_unique_id, guid));
......
...@@ -40,7 +40,7 @@ void BackgroundFetchServiceImpl::CreateForWorker( ...@@ -40,7 +40,7 @@ void BackgroundFetchServiceImpl::CreateForWorker(
const url::Origin& origin) { const url::Origin& origin) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce( base::BindOnce(
BackgroundFetchServiceImpl::CreateOnIoThread, BackgroundFetchServiceImpl::CreateOnIoThread,
...@@ -72,7 +72,7 @@ void BackgroundFetchServiceImpl::CreateForFrame( ...@@ -72,7 +72,7 @@ void BackgroundFetchServiceImpl::CreateForFrame(
render_frame_host->GetFrameTreeNodeId()); render_frame_host->GetFrameTreeNodeId());
} }
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce( base::BindOnce(
BackgroundFetchServiceImpl::CreateOnIoThread, BackgroundFetchServiceImpl::CreateOnIoThread,
......
...@@ -46,9 +46,10 @@ void SerializeIcon(const SkBitmap& icon, SerializeIconCallback callback) { ...@@ -46,9 +46,10 @@ void SerializeIcon(const SkBitmap& icon, SerializeIconCallback callback) {
// Do the serialization on a seperate thread to avoid blocking on // Do the serialization on a seperate thread to avoid blocking on
// expensive operations (image conversions), then post back to current // expensive operations (image conversions), then post back to current
// thread and continue normally. // thread and continue normally.
base::PostTaskWithTraitsAndReplyWithResult( base::PostTaskAndReplyWithResult(
FROM_HERE, FROM_HERE,
{base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN, {base::ThreadPool(), base::MayBlock(),
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN,
base::TaskPriority::BEST_EFFORT}, base::TaskPriority::BEST_EFFORT},
base::BindOnce(&ConvertAndSerializeIcon, icon), std::move(callback)); base::BindOnce(&ConvertAndSerializeIcon, icon), std::move(callback));
} }
...@@ -59,9 +60,10 @@ void DeserializeIcon(std::unique_ptr<std::string> serialized_icon, ...@@ -59,9 +60,10 @@ void DeserializeIcon(std::unique_ptr<std::string> serialized_icon,
// Do the deserialization on a seperate thread to avoid blocking on // Do the deserialization on a seperate thread to avoid blocking on
// expensive operations (image conversions), then post back to current // expensive operations (image conversions), then post back to current
// thread and continue normally. // thread and continue normally.
base::PostTaskWithTraitsAndReplyWithResult( base::PostTaskAndReplyWithResult(
FROM_HERE, FROM_HERE,
{base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN, {base::ThreadPool(), base::MayBlock(),
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN,
base::TaskPriority::BEST_EFFORT}, base::TaskPriority::BEST_EFFORT},
base::BindOnce(&DeserializeAndConvertIcon, std::move(serialized_icon)), base::BindOnce(&DeserializeAndConvertIcon, std::move(serialized_icon)),
base::BindOnce(std::move(callback))); base::BindOnce(std::move(callback)));
......
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