Commit 85432f15 authored by Sami Kyostila's avatar Sami Kyostila Committed by Commit Bot

chrome/browser/media: 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://crrev.com/c/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=takumif@chromium.org
TBR=miu@chromium.org

Bug: 968047
Change-Id: Iec76e7a1dcd7bc894b4fcec92c84d4294108e63a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1739727
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarTakumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686057}
parent a223d5b1
...@@ -53,8 +53,8 @@ void CreateVideoCaptureHostOnIO(const std::string& device_id, ...@@ -53,8 +53,8 @@ void CreateVideoCaptureHostOnIO(const std::string& device_id,
media::mojom::VideoCaptureHostRequest request) { media::mojom::VideoCaptureHostRequest request) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
scoped_refptr<base::SingleThreadTaskRunner> device_task_runner = scoped_refptr<base::SingleThreadTaskRunner> device_task_runner =
base::CreateSingleThreadTaskRunnerWithTraits( base::CreateSingleThreadTaskRunner(
{base::TaskPriority::USER_BLOCKING, {base::ThreadPool(), base::TaskPriority::USER_BLOCKING,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}, base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
base::SingleThreadTaskRunnerThreadMode::DEDICATED); base::SingleThreadTaskRunnerThreadMode::DEDICATED);
mojo::MakeStrongBinding( mojo::MakeStrongBinding(
...@@ -240,7 +240,7 @@ gfx::Size CastMirroringServiceHost::GetClampedResolution( ...@@ -240,7 +240,7 @@ gfx::Size CastMirroringServiceHost::GetClampedResolution(
void CastMirroringServiceHost::GetVideoCaptureHost( void CastMirroringServiceHost::GetVideoCaptureHost(
media::mojom::VideoCaptureHostRequest request) { media::mojom::VideoCaptureHostRequest request) {
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {BrowserThread::IO}, FROM_HERE, {BrowserThread::IO},
base::BindOnce(&CreateVideoCaptureHostOnIO, source_media_id_.ToString(), base::BindOnce(&CreateVideoCaptureHostOnIO, source_media_id_.ToString(),
ConvertVideoStreamType(source_media_id_.type), ConvertVideoStreamType(source_media_id_.type),
......
...@@ -403,7 +403,7 @@ device::mojom::WakeLock* CastTransportHostFilter::GetWakeLock() { ...@@ -403,7 +403,7 @@ device::mojom::WakeLock* CastTransportHostFilter::GetWakeLock() {
service_manager::mojom::ConnectorRequest connector_request; service_manager::mojom::ConnectorRequest connector_request;
auto connector = service_manager::Connector::Create(&connector_request); auto connector = service_manager::Connector::Create(&connector_request);
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {content::BrowserThread::UI}, FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&CastBindConnectorRequest, std::move(connector_request))); base::BindOnce(&CastBindConnectorRequest, std::move(connector_request)));
......
...@@ -50,8 +50,7 @@ DialMediaSinkService::CreateImpl( ...@@ -50,8 +50,7 @@ DialMediaSinkService::CreateImpl(
// Note: The SequencedTaskRunner needs to be IO thread because DialRegistry // Note: The SequencedTaskRunner needs to be IO thread because DialRegistry
// runs on IO thread. // runs on IO thread.
scoped_refptr<base::SequencedTaskRunner> task_runner = scoped_refptr<base::SequencedTaskRunner> task_runner =
base::CreateSingleThreadTaskRunnerWithTraits( base::CreateSingleThreadTaskRunner({content::BrowserThread::IO});
{content::BrowserThread::IO});
return std::unique_ptr<DialMediaSinkServiceImpl, base::OnTaskRunnerDeleter>( return std::unique_ptr<DialMediaSinkServiceImpl, base::OnTaskRunnerDeleter>(
new DialMediaSinkServiceImpl(connector, sink_discovery_cb, task_runner), new DialMediaSinkServiceImpl(connector, sink_discovery_cb, task_runner),
base::OnTaskRunnerDeleter(task_runner)); base::OnTaskRunnerDeleter(task_runner));
......
...@@ -50,7 +50,7 @@ DialRegistry::DialRegistry() ...@@ -50,7 +50,7 @@ DialRegistry::DialRegistry()
clock_(base::DefaultClock::GetInstance()) { clock_(base::DefaultClock::GetInstance()) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK_GT(max_devices_, 0U); DCHECK_GT(max_devices_, 0U);
base::PostTaskWithTraitsAndReplyWithResult( base::PostTaskAndReplyWithResult(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce(&content::GetNetworkConnectionTracker), base::BindOnce(&content::GetNetworkConnectionTracker),
base::BindOnce(&DialRegistry::SetNetworkConnectionTracker, base::BindOnce(&DialRegistry::SetNetworkConnectionTracker,
......
...@@ -62,9 +62,9 @@ void PostSendNetworkList( ...@@ -62,9 +62,9 @@ void PostSendNetworkList(
base::WeakPtr<DialServiceImpl> impl, base::WeakPtr<DialServiceImpl> impl,
const base::Optional<net::NetworkInterfaceList>& networks) { const base::Optional<net::NetworkInterfaceList>& networks) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO}, base::PostTask(FROM_HERE, {BrowserThread::IO},
base::BindOnce(&DialServiceImpl::SendNetworkList, base::BindOnce(&DialServiceImpl::SendNetworkList,
std::move(impl), networks)); std::move(impl), networks));
} }
#endif // !defined(OS_CHROMEOS) #endif // !defined(OS_CHROMEOS)
...@@ -461,8 +461,7 @@ void DialServiceImpl::StartDiscovery() { ...@@ -461,8 +461,7 @@ void DialServiceImpl::StartDiscovery() {
return; return;
} }
auto task_runner = auto task_runner = base::CreateSingleThreadTaskRunner({BrowserThread::UI});
base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI});
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
task_tracker_.PostTaskAndReplyWithResult( task_tracker_.PostTaskAndReplyWithResult(
......
...@@ -168,10 +168,9 @@ void DialURLFetcher::StartDownload() { ...@@ -168,10 +168,9 @@ void DialURLFetcher::StartDownload() {
// this conditional. // this conditional.
auto mojo_request = mojo::MakeRequest(&loader_factory); auto mojo_request = mojo::MakeRequest(&loader_factory);
if (content::BrowserThread::IsThreadInitialized(content::BrowserThread::UI)) { if (content::BrowserThread::IsThreadInitialized(content::BrowserThread::UI)) {
base::PostTaskWithTraits( base::PostTask(FROM_HERE, {content::BrowserThread::UI},
FROM_HERE, {content::BrowserThread::UI}, base::BindOnce(&BindURLLoaderFactoryRequestOnUIThread,
base::BindOnce(&BindURLLoaderFactoryRequestOnUIThread, std::move(mojo_request)));
std::move(mojo_request)));
} }
loader_->DownloadToString( loader_->DownloadToString(
......
...@@ -96,8 +96,8 @@ DiscoveryNetworkMonitor::DiscoveryNetworkMonitor(NetworkInfoFunction strategy) ...@@ -96,8 +96,8 @@ DiscoveryNetworkMonitor::DiscoveryNetworkMonitor(NetworkInfoFunction strategy)
: network_id_(kNetworkIdDisconnected), : network_id_(kNetworkIdDisconnected),
observers_(new base::ObserverListThreadSafe<Observer>( observers_(new base::ObserverListThreadSafe<Observer>(
base::ObserverListPolicy::EXISTING_ONLY)), base::ObserverListPolicy::EXISTING_ONLY)),
task_runner_(base::CreateSequencedTaskRunnerWithTraits( task_runner_(base::CreateSequencedTaskRunner(
{base::MayBlock(), {base::ThreadPool(), base::MayBlock(),
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})),
network_info_function_(strategy), network_info_function_(strategy),
metric_observer_(std::make_unique<DiscoveryNetworkMonitorMetricObserver>( metric_observer_(std::make_unique<DiscoveryNetworkMonitorMetricObserver>(
......
...@@ -46,8 +46,8 @@ base::TimeDelta IssueManager::GetAutoDismissTimeout( ...@@ -46,8 +46,8 @@ base::TimeDelta IssueManager::GetAutoDismissTimeout(
IssueManager::IssueManager() IssueManager::IssueManager()
: top_issue_(nullptr), : top_issue_(nullptr),
task_runner_(base::CreateSingleThreadTaskRunnerWithTraits( task_runner_(
{content::BrowserThread::UI})) {} base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})) {}
IssueManager::~IssueManager() { IssueManager::~IssueManager() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
......
...@@ -35,8 +35,9 @@ bool DoCanFirewallUseLocalPorts() { ...@@ -35,8 +35,9 @@ bool DoCanFirewallUseLocalPorts() {
} // namespace } // namespace
void CanFirewallUseLocalPorts(base::OnceCallback<void(bool)> callback) { void CanFirewallUseLocalPorts(base::OnceCallback<void(bool)> callback) {
auto task_runner = base::CreateCOMSTATaskRunnerWithTraits( auto task_runner = base::CreateCOMSTATaskRunner(
{base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}); {base::ThreadPool(), base::MayBlock(),
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN});
base::PostTaskAndReplyWithResult(task_runner.get(), FROM_HERE, base::PostTaskAndReplyWithResult(task_runner.get(), FROM_HERE,
base::BindOnce(&DoCanFirewallUseLocalPorts), base::BindOnce(&DoCanFirewallUseLocalPorts),
std::move(callback)); std::move(callback));
......
...@@ -736,7 +736,7 @@ void MediaRouterMojoImpl::RegisterMediaRoutesObserver( ...@@ -736,7 +736,7 @@ void MediaRouterMojoImpl::RegisterMediaRoutesObserver(
// Return to the event loop before notifying of a cached route list because // Return to the event loop before notifying of a cached route list because
// MediaRoutesObserver is calling this method from its constructor, and that // MediaRoutesObserver is calling this method from its constructor, and that
// must complete before invoking its virtual OnRoutesUpdated() method. // must complete before invoking its virtual OnRoutesUpdated() method.
base::PostTaskWithTraits( base::PostTask(
FROM_HERE, {content::BrowserThread::UI}, FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&MediaRouterMojoImpl::NotifyOfExistingRoutesIfRegistered, base::BindOnce(&MediaRouterMojoImpl::NotifyOfExistingRoutesIfRegistered,
weak_factory_.GetWeakPtr(), source_id, observer)); weak_factory_.GetWeakPtr(), source_id, observer));
......
...@@ -92,8 +92,8 @@ class CastActivityManagerTest : public testing::Test, ...@@ -92,8 +92,8 @@ class CastActivityManagerTest : public testing::Test,
public CastActivityRecordFactoryForTest { public CastActivityRecordFactoryForTest {
public: public:
CastActivityManagerTest() CastActivityManagerTest()
: socket_service_(base::CreateSingleThreadTaskRunnerWithTraits( : socket_service_(
{content::BrowserThread::UI})), base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})),
message_handler_(&socket_service_) { message_handler_(&socket_service_) {
media_sink_service_.AddOrUpdateSink(sink_); media_sink_service_.AddOrUpdateSink(sink_);
socket_.set_id(kChannelId); socket_.set_id(kChannelId);
......
...@@ -179,8 +179,7 @@ class CastActivityRecordTest : public testing::Test, ...@@ -179,8 +179,7 @@ class CastActivityRecordTest : public testing::Test,
MediaSinkInternal sink_ = CreateCastSink(kChannelId); MediaSinkInternal sink_ = CreateCastSink(kChannelId);
service_manager::TestConnectorFactory connector_factory_; service_manager::TestConnectorFactory connector_factory_;
cast_channel::MockCastSocketService socket_service_{ cast_channel::MockCastSocketService socket_service_{
base::CreateSingleThreadTaskRunnerWithTraits( base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})};
{content::BrowserThread::UI})};
cast_channel::MockCastMessageHandler message_handler_{&socket_service_}; cast_channel::MockCastMessageHandler message_handler_{&socket_service_};
std::unique_ptr<DataDecoder> data_decoder_ = std::unique_ptr<DataDecoder> data_decoder_ =
std::make_unique<DataDecoder>(connector_factory_.GetDefaultConnector()); std::make_unique<DataDecoder>(connector_factory_.GetDefaultConnector());
......
...@@ -40,8 +40,8 @@ class CastMediaRouteProviderTest : public testing::Test { ...@@ -40,8 +40,8 @@ class CastMediaRouteProviderTest : public testing::Test {
CastMediaRouteProviderTest() CastMediaRouteProviderTest()
: data_decoder_service_(connector_factory_.RegisterInstance( : data_decoder_service_(connector_factory_.RegisterInstance(
data_decoder::mojom::kServiceName)), data_decoder::mojom::kServiceName)),
socket_service_(base::CreateSingleThreadTaskRunnerWithTraits( socket_service_(
{content::BrowserThread::UI})), base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})),
message_handler_(&socket_service_) {} message_handler_(&socket_service_) {}
~CastMediaRouteProviderTest() override = default; ~CastMediaRouteProviderTest() override = default;
......
...@@ -92,8 +92,7 @@ class CastSessionClientImplTest : public testing::Test { ...@@ -92,8 +92,7 @@ class CastSessionClientImplTest : public testing::Test {
data_decoder::TestingJsonParser::ScopedFactoryOverride parser_override_; data_decoder::TestingJsonParser::ScopedFactoryOverride parser_override_;
service_manager::TestConnectorFactory connector_factory_; service_manager::TestConnectorFactory connector_factory_;
cast_channel::MockCastSocketService socket_service_{ cast_channel::MockCastSocketService socket_service_{
base::CreateSingleThreadTaskRunnerWithTraits( base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})};
{content::BrowserThread::UI})};
cast_channel::MockCastMessageHandler message_handler_{&socket_service_}; cast_channel::MockCastMessageHandler message_handler_{&socket_service_};
DataDecoder decoder_{connector_factory_.GetDefaultConnector()}; DataDecoder decoder_{connector_factory_.GetDefaultConnector()};
url::Origin origin_; url::Origin origin_;
......
...@@ -76,8 +76,8 @@ class MockCastSessionObserver : public CastSessionTracker::Observer { ...@@ -76,8 +76,8 @@ class MockCastSessionObserver : public CastSessionTracker::Observer {
class CastSessionTrackerTest : public testing::Test { class CastSessionTrackerTest : public testing::Test {
public: public:
CastSessionTrackerTest() CastSessionTrackerTest()
: socket_service_(base::CreateSingleThreadTaskRunnerWithTraits( : socket_service_(
{content::BrowserThread::UI})), base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})),
message_handler_(&socket_service_), message_handler_(&socket_service_),
session_tracker_(&media_sink_service_, session_tracker_(&media_sink_service_,
&message_handler_, &message_handler_,
......
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