Commit 201cafdd authored by Sami Kyostila's avatar Sami Kyostila Committed by Commit Bot

chrome/browser/media/webrtc: 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=eladalon@chromium.org

Bug: 968047
Change-Id: I32f6d432ffa2550f58d433775e0adef216cf9ad5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1738593
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarElad Alon <eladalon@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684406}
parent 5d33ab62
...@@ -46,7 +46,7 @@ class PeerConnectionTrackerProxyImpl ...@@ -46,7 +46,7 @@ class PeerConnectionTrackerProxyImpl
void EnableWebRtcEventLogging(const WebRtcEventLogPeerConnectionKey& key, void EnableWebRtcEventLogging(const WebRtcEventLogPeerConnectionKey& key,
int output_period_ms) override { int output_period_ms) override {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce( base::BindOnce(
&PeerConnectionTrackerProxyImpl::EnableWebRtcEventLoggingInternal, &PeerConnectionTrackerProxyImpl::EnableWebRtcEventLoggingInternal,
...@@ -55,7 +55,7 @@ class PeerConnectionTrackerProxyImpl ...@@ -55,7 +55,7 @@ class PeerConnectionTrackerProxyImpl
void DisableWebRtcEventLogging( void DisableWebRtcEventLogging(
const WebRtcEventLogPeerConnectionKey& key) override { const WebRtcEventLogPeerConnectionKey& key) override {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce( base::BindOnce(
&PeerConnectionTrackerProxyImpl::DisableWebRtcEventLoggingInternal, &PeerConnectionTrackerProxyImpl::DisableWebRtcEventLoggingInternal,
...@@ -148,8 +148,8 @@ inline void MaybeReply(const base::Location& location, ...@@ -148,8 +148,8 @@ inline void MaybeReply(const base::Location& location,
base::OnceCallback<void(Args...)> reply, base::OnceCallback<void(Args...)> reply,
Args... args) { Args... args) {
if (reply) { if (reply) {
base::PostTaskWithTraits(location, {BrowserThread::UI}, base::PostTask(location, {BrowserThread::UI},
base::BindOnce(std::move(reply), args...)); base::BindOnce(std::move(reply), args...));
} }
} }
...@@ -181,8 +181,9 @@ base::FilePath WebRtcEventLogManager::GetRemoteBoundWebRtcEventLogsDir( ...@@ -181,8 +181,9 @@ base::FilePath WebRtcEventLogManager::GetRemoteBoundWebRtcEventLogsDir(
} }
WebRtcEventLogManager::WebRtcEventLogManager() WebRtcEventLogManager::WebRtcEventLogManager()
: task_runner_(base::CreateSequencedTaskRunnerWithTraits( : task_runner_(base::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT, {base::ThreadPool(), base::MayBlock(),
base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})),
remote_logging_feature_enabled_(IsRemoteLoggingFeatureEnabled()), remote_logging_feature_enabled_(IsRemoteLoggingFeatureEnabled()),
local_logs_observer_(nullptr), local_logs_observer_(nullptr),
...@@ -450,9 +451,9 @@ void WebRtcEventLogManager::StartRemoteLogging( ...@@ -450,9 +451,9 @@ void WebRtcEventLogManager::StartRemoteLogging(
} }
if (error) { if (error) {
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(std::move(reply), false, base::BindOnce(std::move(reply), false, std::string(),
std::string(), std::string(error))); std::string(error)));
return; return;
} }
...@@ -925,7 +926,7 @@ void WebRtcEventLogManager::StartRemoteLoggingInternal( ...@@ -925,7 +926,7 @@ void WebRtcEventLogManager::StartRemoteLoggingInternal(
DCHECK_EQ(result, !log_id.empty()); DCHECK_EQ(result, !log_id.empty());
DCHECK_EQ(!result, !error_message.empty()); DCHECK_EQ(!result, !error_message.empty());
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce(std::move(reply), result, log_id, error_message)); base::BindOnce(std::move(reply), result, log_id, error_message));
} }
...@@ -962,7 +963,7 @@ void WebRtcEventLogManager::SetLocalLogsObserverInternal( ...@@ -962,7 +963,7 @@ void WebRtcEventLogManager::SetLocalLogsObserverInternal(
local_logs_observer_ = observer; local_logs_observer_ = observer;
if (reply) { if (reply) {
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, std::move(reply)); base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply));
} }
} }
...@@ -974,7 +975,7 @@ void WebRtcEventLogManager::SetRemoteLogsObserverInternal( ...@@ -974,7 +975,7 @@ void WebRtcEventLogManager::SetRemoteLogsObserverInternal(
remote_logs_observer_ = observer; remote_logs_observer_ = observer;
if (reply) { if (reply) {
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, std::move(reply)); base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply));
} }
} }
...@@ -987,7 +988,7 @@ void WebRtcEventLogManager::SetClockForTesting(base::Clock* clock, ...@@ -987,7 +988,7 @@ void WebRtcEventLogManager::SetClockForTesting(base::Clock* clock,
base::OnceClosure reply) { base::OnceClosure reply) {
manager->local_logs_manager_.SetClockForTesting(clock); manager->local_logs_manager_.SetClockForTesting(clock);
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, std::move(reply)); base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply));
}; };
// |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this) // |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this)
...@@ -1007,7 +1008,7 @@ void WebRtcEventLogManager::SetPeerConnectionTrackerProxyForTesting( ...@@ -1007,7 +1008,7 @@ void WebRtcEventLogManager::SetPeerConnectionTrackerProxyForTesting(
base::OnceClosure reply) { base::OnceClosure reply) {
manager->pc_tracker_proxy_ = std::move(pc_tracker_proxy); manager->pc_tracker_proxy_ = std::move(pc_tracker_proxy);
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, std::move(reply)); base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply));
}; };
// |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this) // |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this)
...@@ -1031,8 +1032,7 @@ void WebRtcEventLogManager::SetWebRtcEventLogUploaderFactoryForTesting( ...@@ -1031,8 +1032,7 @@ void WebRtcEventLogManager::SetWebRtcEventLogUploaderFactoryForTesting(
remote_logs_manager.SetWebRtcEventLogUploaderFactoryForTesting( remote_logs_manager.SetWebRtcEventLogUploaderFactoryForTesting(
std::move(uploader_factory)); std::move(uploader_factory));
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, base::PostTask(FROM_HERE, {BrowserThread::UI}, std::move(reply));
std::move(reply));
}; };
// |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this) // |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this)
......
...@@ -542,8 +542,8 @@ void WebRtcRemoteEventLogManager::GetHistory( ...@@ -542,8 +542,8 @@ void WebRtcRemoteEventLogManager::GetHistory(
if (!BrowserContextEnabled(browser_context_id)) { if (!BrowserContextEnabled(browser_context_id)) {
LOG(ERROR) << "Unknown |browser_context_id|."; LOG(ERROR) << "Unknown |browser_context_id|.";
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(std::move(reply), history)); base::BindOnce(std::move(reply), history));
return; return;
} }
...@@ -591,8 +591,8 @@ void WebRtcRemoteEventLogManager::GetHistory( ...@@ -591,8 +591,8 @@ void WebRtcRemoteEventLogManager::GetHistory(
}; };
std::sort(history.begin(), history.end(), cmp); std::sort(history.begin(), history.end(), cmp);
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(std::move(reply), history)); base::BindOnce(std::move(reply), history));
} }
void WebRtcRemoteEventLogManager::RemovePendingLogsForNotEnabledBrowserContext( void WebRtcRemoteEventLogManager::RemovePendingLogsForNotEnabledBrowserContext(
...@@ -664,17 +664,16 @@ void WebRtcRemoteEventLogManager::SetWebRtcEventLogUploaderFactoryForTesting( ...@@ -664,17 +664,16 @@ void WebRtcRemoteEventLogManager::SetWebRtcEventLogUploaderFactoryForTesting(
void WebRtcRemoteEventLogManager::UploadConditionsHoldForTesting( void WebRtcRemoteEventLogManager::UploadConditionsHoldForTesting(
base::OnceCallback<void(bool)> callback) { base::OnceCallback<void(bool)> callback) {
DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(task_runner_->RunsTasksInCurrentSequence());
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {content::BrowserThread::UI},
FROM_HERE, {content::BrowserThread::UI}, base::BindOnce(std::move(callback), UploadConditionsHold()));
base::BindOnce(std::move(callback), UploadConditionsHold()));
} }
void WebRtcRemoteEventLogManager::ShutDownForTesting(base::OnceClosure reply) { void WebRtcRemoteEventLogManager::ShutDownForTesting(base::OnceClosure reply) {
DCHECK(task_runner_->RunsTasksInCurrentSequence()); DCHECK(task_runner_->RunsTasksInCurrentSequence());
weak_ptr_factory_->InvalidateWeakPtrs(); weak_ptr_factory_->InvalidateWeakPtrs();
weak_ptr_factory_.reset(); weak_ptr_factory_.reset();
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI}, base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(std::move(reply))); base::BindOnce(std::move(reply)));
} }
bool WebRtcRemoteEventLogManager::AreLogParametersValid( bool WebRtcRemoteEventLogManager::AreLogParametersValid(
......
...@@ -279,10 +279,9 @@ void WebRtcEventLogUploaderImpl::StartUpload(const std::string& upload_data) { ...@@ -279,10 +279,9 @@ void WebRtcEventLogUploaderImpl::StartUpload(const std::string& upload_data) {
// immediately, even though it needs to finish initialization on the UI // immediately, even though it needs to finish initialization on the UI
// thread. // thread.
network::mojom::URLLoaderFactoryPtr url_loader_factory_ptr; network::mojom::URLLoaderFactoryPtr url_loader_factory_ptr;
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {content::BrowserThread::UI},
FROM_HERE, {content::BrowserThread::UI}, base::BindOnce(BindURLLoaderFactoryRequest,
base::BindOnce(BindURLLoaderFactoryRequest, mojo::MakeRequest(&url_loader_factory_ptr)));
mojo::MakeRequest(&url_loader_factory_ptr)));
url_loader_ = network::SimpleURLLoader::Create( url_loader_ = network::SimpleURLLoader::Create(
std::move(resource_request), kWebrtcEventLogUploaderTrafficAnnotation); std::move(resource_request), kWebrtcEventLogUploaderTrafficAnnotation);
......
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