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

chrome/browser/printing: 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=rbpotter@chromium.org

Bug: 968047
Change-Id: I1849e7c28ee90650814e7a7f8e53b0075511da63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729081
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarRebekah Potter <rbpotter@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683301}
parent f0d4ef6c
......@@ -49,9 +49,8 @@ void OnGetNetworkList(
"lo", "lo", 0, net::NetworkChangeNotifier::CONNECTION_UNKNOWN,
localhost_prefix, 8, net::IP_ADDRESS_ATTRIBUTE_NONE));
base::PostTaskWithTraits(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(std::move(callback), std::move(ip4_networks)));
base::PostTask(FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(std::move(callback), std::move(ip4_networks)));
}
void GetNetworkListOnUIThread(
......@@ -83,10 +82,9 @@ PrivetTrafficDetector::PrivetTrafficDetector(
: helper_(new Helper(profile, on_traffic_detected)) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
base::PostTaskWithTraits(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&PrivetTrafficDetector::Helper::ScheduleRestart,
base::Unretained(helper_)));
base::PostTask(FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&PrivetTrafficDetector::Helper::ScheduleRestart,
base::Unretained(helper_)));
}
PrivetTrafficDetector::~PrivetTrafficDetector() {
......@@ -99,7 +97,7 @@ PrivetTrafficDetector::~PrivetTrafficDetector() {
void PrivetTrafficDetector::OnConnectionChanged(
network::mojom::ConnectionType type) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&PrivetTrafficDetector::Helper::HandleConnectionChanged,
base::Unretained(helper_), type));
......@@ -132,7 +130,7 @@ void PrivetTrafficDetector::Helper::ScheduleRestart() {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
ResetConnection();
weak_ptr_factory_.InvalidateWeakPtrs();
base::PostDelayedTaskWithTraits(
base::PostDelayedTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(
&GetNetworkListOnUIThread,
......@@ -154,7 +152,7 @@ void PrivetTrafficDetector::Helper::Bind() {
network::mojom::UDPSocketReceiverRequest receiver_request =
mojo::MakeRequest(&receiver_ptr);
receiver_binding_.Bind(std::move(receiver_request));
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&CreateUDPSocketOnUIThread, profile_,
mojo::MakeRequest(&socket_), std::move(receiver_ptr)));
......@@ -256,8 +254,8 @@ void PrivetTrafficDetector::Helper::OnReceived(
recv_addr_ = src_addr.value();
if (IsPrivetPacket(data.value())) {
ResetConnection();
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI},
on_traffic_detected_);
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
on_traffic_detected_);
} else {
socket_->ReceiveMoreWithBufferSize(1, net::dns_protocol::kMaxMulticastSize);
}
......
......@@ -288,7 +288,7 @@ class CloudPrintProxyPolicyStartupTest : public base::MultiProcessTest,
void TearDown() override;
scoped_refptr<base::SingleThreadTaskRunner> IOTaskRunner() {
return base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::IO});
return base::CreateSingleThreadTaskRunner({BrowserThread::IO});
}
base::Process Launch(const std::string& name);
void WaitForConnect(mojo::IsolatedConnection* mojo_connection);
......@@ -438,8 +438,9 @@ void CloudPrintProxyPolicyStartupTest::WaitForConnect(
EXPECT_TRUE(base::ThreadTaskRunnerHandle::Get().get());
mojo::MessagePipe pipe;
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::PostTask(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&ConnectAsync, std::move(pipe.handle1),
GetServiceProcessServerName(), mojo_connection));
ServiceProcessControl::GetInstance()->SetMojoHandle(
......@@ -478,7 +479,7 @@ base::CommandLine CloudPrintProxyPolicyStartupTest::MakeCmdLine(
TEST_F(CloudPrintProxyPolicyStartupTest, StartAndShutdown) {
mojo::core::Init();
mojo::core::ScopedIPCSupport ipc_support(
base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::IO}),
base::CreateSingleThreadTaskRunner({BrowserThread::IO}),
mojo::core::ScopedIPCSupport::ShutdownPolicy::FAST);
base::Process process =
......
......@@ -130,8 +130,9 @@ void CreatePrintDialogForFile(content::BrowserContext* browser_context,
const base::string16& print_ticket,
const std::string& file_type) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::PostTaskWithTraitsAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
base::PostTaskAndReplyWithResult(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_BLOCKING},
base::BindOnce(&ReadFile, path_to_file),
base::BindOnce(&CreatePrintDialog, browser_context, print_job_title,
print_ticket, file_type));
......
......@@ -28,6 +28,6 @@ void ShowPrintErrorDialogTask() {
void ShowPrintErrorDialog() {
// Nested loop may destroy caller.
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&ShowPrintErrorDialogTask));
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&ShowPrintErrorDialogTask));
}
......@@ -219,8 +219,8 @@ bool PrintJob::FlushJob(base::TimeDelta timeout) {
base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
quit_closure_ = loop.QuitClosure();
base::PostDelayedTaskWithTraits(FROM_HERE, {content::BrowserThread::UI},
loop.QuitClosure(), timeout);
base::PostDelayedTask(FROM_HERE, {content::BrowserThread::UI},
loop.QuitClosure(), timeout);
loop.Run();
......@@ -451,8 +451,8 @@ void PrintJob::OnNotifyPrintJobEvent(const JobEventDetails& event_details) {
}
case JobEventDetails::DOC_DONE: {
// This will call Stop() and broadcast a JOB_DONE message.
base::PostTaskWithTraits(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&PrintJob::OnDocumentDone, this));
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&PrintJob::OnDocumentDone, this));
break;
}
#if defined(OS_WIN)
......@@ -508,7 +508,7 @@ void PrintJob::ControlledWorkerShutdown() {
// Delay shutdown until the worker terminates. We want this code path
// to wait on the thread to quit before continuing.
if (worker_->IsRunning()) {
base::PostDelayedTaskWithTraits(
base::PostDelayedTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&PrintJob::ControlledWorkerShutdown, this),
base::TimeDelta::FromMilliseconds(100));
......@@ -518,9 +518,9 @@ void PrintJob::ControlledWorkerShutdown() {
// Now make sure the thread object is cleaned up. Do this on a worker
// thread because it may block.
base::PostTaskWithTraitsAndReply(
base::PostTaskAndReply(
FROM_HERE,
{base::MayBlock(), base::WithBaseSyncPrimitives(),
{base::ThreadPool(), base::MayBlock(), base::WithBaseSyncPrimitives(),
base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
base::BindOnce(&PrintJobWorker::Stop, base::Unretained(worker_.get())),
......@@ -533,8 +533,8 @@ void PrintJob::ControlledWorkerShutdown() {
bool PrintJob::PostTask(const base::Location& from_here,
base::OnceClosure task) {
return base::PostTaskWithTraits(from_here, {content::BrowserThread::UI},
std::move(task));
return base::PostTask(from_here, {content::BrowserThread::UI},
std::move(task));
}
void PrintJob::HoldUntilStopIsCalled() {
......
......@@ -165,16 +165,15 @@ void PrintJobWorker::GetSettings(bool ask_user_for_settings,
// When we delegate to a destination, we don't ask the user for settings.
// TODO(mad): Ask the destination for settings.
if (ask_user_for_settings) {
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintJobWorker::GetSettingsWithUI,
base::Unretained(this), document_page_count,
has_selection, is_scripted, std::move(callback)));
} else {
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintJobWorker::UseDefaultSettings,
base::Unretained(this), std::move(callback)));
base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintJobWorker::UseDefaultSettings,
base::Unretained(this), std::move(callback)));
}
}
......@@ -182,11 +181,10 @@ void PrintJobWorker::SetSettings(base::Value new_settings,
SettingsCallback callback) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintJobWorker::UpdatePrintSettings,
base::Unretained(this), std::move(new_settings),
std::move(callback)));
base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintJobWorker::UpdatePrintSettings,
base::Unretained(this), std::move(new_settings),
std::move(callback)));
}
#if defined(OS_CHROMEOS)
......@@ -195,11 +193,10 @@ void PrintJobWorker::SetSettingsFromPOD(
SettingsCallback callback) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintJobWorker::UpdatePrintSettingsFromPOD,
base::Unretained(this), std::move(new_settings),
std::move(callback)));
base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintJobWorker::UpdatePrintSettingsFromPOD,
base::Unretained(this), std::move(new_settings),
std::move(callback)));
}
#endif
......
......@@ -51,7 +51,7 @@ void StopWorker(int document_cookie) {
std::unique_ptr<PrinterQuery> printer_query =
queue->PopPrinterQuery(document_cookie);
if (printer_query) {
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {BrowserThread::IO},
base::BindOnce(&PrinterQuery::StopWorker, std::move(printer_query)));
}
......
......@@ -79,7 +79,7 @@ void OnPrintSettingsDoneWrapper(PrintSettingsCallback settings_callback,
std::unique_ptr<PrinterQuery> query) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(std::move(settings_callback), std::move(query)));
}
......@@ -143,7 +143,7 @@ void PrintViewManagerBase::PrintForPrintPreview(
weak_ptr_factory_.GetWeakPtr(), print_data,
job_settings.FindIntKey(kSettingPreviewPageCount).value(),
std::move(callback));
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(CreateQueryWithSettings, std::move(job_settings),
rfh->GetProcess()->GetID(), rfh->GetRoutingID(), queue_,
......@@ -184,17 +184,16 @@ void PrintViewManagerBase::OnPrintSettingsDone(
if (printer_query->last_status() == PrintingContext::CANCEL) {
queue_->QueuePrinterQuery(std::move(printer_query));
#if defined(OS_WIN)
base::PostTaskWithTraits(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&PrintViewManagerBase::SystemDialogCancelled,
weak_ptr_factory_.GetWeakPtr()));
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&PrintViewManagerBase::SystemDialogCancelled,
weak_ptr_factory_.GetWeakPtr()));
#endif
std::move(callback).Run(base::Value());
return;
}
if (!printer_query->cookie() || !printer_query->settings().dpi()) {
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&PrinterQuery::StopWorker, std::move(printer_query)));
std::move(callback).Run(base::Value("Update settings failed"));
......@@ -205,11 +204,10 @@ void PrintViewManagerBase::OnPrintSettingsDone(
// OnDidGetPrintedPagesCount().
int cookie = printer_query->cookie();
queue_->QueuePrinterQuery(std::move(printer_query));
base::PostTaskWithTraits(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&PrintViewManagerBase::StartLocalPrintJob,
weak_ptr_factory_.GetWeakPtr(), print_data, page_count,
cookie, std::move(callback)));
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&PrintViewManagerBase::StartLocalPrintJob,
weak_ptr_factory_.GetWeakPtr(), print_data,
page_count, cookie, std::move(callback)));
}
void PrintViewManagerBase::StartLocalPrintJob(
......@@ -704,7 +702,7 @@ void PrintViewManagerBase::ReleasePrinterQuery() {
std::unique_ptr<PrinterQuery> printer_query = queue_->PopPrinterQuery(cookie);
if (!printer_query)
return;
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&PrinterQuery::StopWorker, std::move(printer_query)));
}
......
......@@ -88,8 +88,9 @@ void DetectAndOpenPrinterConfigDialog() {
namespace printing {
void PrinterManagerDialog::ShowPrinterManagerDialog(Profile* profile) {
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
base::PostTask(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_BLOCKING},
base::BindOnce(&DetectAndOpenPrinterConfigDialog));
}
......
......@@ -39,9 +39,10 @@ void PrinterManagerDialog::ShowPrinterManagerDialog(Profile* profile) {
if (base::win::GetVersion() >= base::win::Version::WIN10_RS1) {
platform_util::OpenExternal(profile, GURL("ms-settings:printers"));
} else {
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
base::BindOnce(OpenPrintersDialogCallback));
base::PostTask(FROM_HERE,
{base::ThreadPool(), base::MayBlock(),
base::TaskPriority::USER_BLOCKING},
base::BindOnce(OpenPrintersDialogCallback));
}
}
......
......@@ -57,7 +57,7 @@ void PrinterQuery::PostSettingsDoneToIO(base::OnceClosure callback,
const PrintSettings& new_settings,
PrintingContext::Result result) {
// |this| is owned by |callback|, so |base::Unretained()| is safe.
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&PrinterQuery::GetSettingsDone, base::Unretained(this),
std::move(callback), new_settings, result));
......@@ -161,8 +161,8 @@ void PrinterQuery::StopWorker() {
bool PrinterQuery::PostTask(const base::Location& from_here,
base::OnceClosure task) {
return base::PostTaskWithTraits(from_here, {content::BrowserThread::IO},
std::move(task));
return base::PostTask(from_here, {content::BrowserThread::IO},
std::move(task));
}
bool PrinterQuery::is_valid() const {
......
......@@ -102,7 +102,7 @@ PrintingMessageFilter::PrintingMessageFilter(int render_process_id,
base::Unretained(this)));
is_printing_enabled_.Init(prefs::kPrintingEnabled, profile->GetPrefs());
is_printing_enabled_.MoveToSequence(
base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::IO}));
base::CreateSingleThreadTaskRunner({BrowserThread::IO}));
}
PrintingMessageFilter::~PrintingMessageFilter() {
......@@ -269,7 +269,7 @@ void PrintingMessageFilter::OnUpdatePrintSettingsReply(
#if defined(OS_WIN) && BUILDFLAG(ENABLE_PRINT_PREVIEW)
if (canceled) {
int routing_id = reply_msg->routing_id();
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {BrowserThread::UI},
base::BindOnce(&PrintingMessageFilter::NotifySystemDialogCancelled,
this, routing_id));
......
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