Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3158)

Unified Diff: chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc

Issue 2943843002: Convert to CupsPrintJobManagerImpl to TaskScheduler. (Closed)
Patch Set: add an io assertion Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/chromeos/printing/cups_print_job_manager_impl.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc
diff --git a/chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc b/chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc
index d03c07c5d950f5c7565c337f9aa3bab9a8d1487b..3cfdd7a9f4c338037001cfdd67f17809efc84f4f 100644
--- a/chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc
+++ b/chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc
@@ -14,10 +14,10 @@
#include "base/bind.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h"
+#include "base/sequenced_task_runner.h"
#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
-#include "base/threading/sequenced_task_runner_handle.h"
-#include "base/threading/sequenced_worker_pool.h"
+#include "base/task_scheduler/post_task.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/printing/cups_print_job.h"
#include "chrome/browser/chromeos/printing/printers_manager.h"
@@ -25,7 +25,6 @@
#include "chrome/browser/printing/print_job.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/browser_context.h"
-#include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h"
#include "printing/backend/cups_connection.h"
@@ -104,6 +103,7 @@ chromeos::CupsPrintJob::State ConvertState(printing::CupsJob::JobState state) {
chromeos::QueryResult QueryCups(::printing::CupsConnection* connection,
const std::vector<std::string>& printer_ids) {
+ base::ThreadRestrictions::AssertIOAllowed();
chromeos::QueryResult result;
result.success = connection->GetJobs(printer_ids, &result.queues);
return result;
@@ -185,6 +185,9 @@ QueryResult::~QueryResult() = default;
CupsPrintJobManagerImpl::CupsPrintJobManagerImpl(Profile* profile)
: CupsPrintJobManager(profile),
cups_connection_(GURL(), HTTP_ENCRYPT_NEVER, false),
+ query_runner_(base::CreateSequencedTaskRunnerWithTraits(
+ base::TaskTraits(base::TaskPriority::BACKGROUND,
+ base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN))),
weak_ptr_factory_(this) {
registrar_.Add(this, chrome::NOTIFICATION_PRINT_JOB_EVENT,
content::NotificationService::AllSources());
@@ -194,8 +197,6 @@ CupsPrintJobManagerImpl::~CupsPrintJobManagerImpl() {}
// Must be run from the UI thread.
void CupsPrintJobManagerImpl::CancelPrintJob(CupsPrintJob* job) {
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
Carlson 2017/06/20 19:02:09 Are you removing the requirement, or just the DCHE
skau 2017/06/24 01:03:44 The requirement still exits. It's now enforced as
-
// Copy job_id and printer_id. |job| is about to be freed.
const int job_id = job->job_id();
const std::string printer_id = job->printer().id();
@@ -205,8 +206,8 @@ void CupsPrintJobManagerImpl::CancelPrintJob(CupsPrintJob* job) {
// Be sure to copy out all relevant fields. |job| may be destroyed after we
// exit this scope.
- content::BrowserThread::GetBlockingPool()->PostTask(
- FROM_HERE,
+ base::PostTaskWithTraits(
+ FROM_HERE, base::TaskTraits(base::TaskPriority::USER_VISIBLE),
Carlson 2017/06/20 19:02:09 I don't understand how concurrency safety works wi
gab 2017/06/20 21:04:49 Right, seems this was racy before and still is..?
skau 2017/06/24 01:03:44 I've sequenced QueryJobs and CancelPrintJobImpl by
base::Bind(&CupsPrintJobManagerImpl::CancelJobImpl,
weak_ptr_factory_.GetWeakPtr(), printer_id, job_id));
}
@@ -284,7 +285,7 @@ void CupsPrintJobManagerImpl::ScheduleQuery(const base::TimeDelta& delay) {
if (!in_query_) {
in_query_ = true;
- base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+ query_runner_->PostDelayedTask(
FROM_HERE,
base::Bind(&CupsPrintJobManagerImpl::PostQuery,
weak_ptr_factory_.GetWeakPtr()),
@@ -300,9 +301,8 @@ void CupsPrintJobManagerImpl::PostQuery() {
}
std::vector<std::string> ids{printer_ids.begin(), printer_ids.end()};
- content::BrowserThread::PostTaskAndReplyWithResult(
- content::BrowserThread::FILE_USER_BLOCKING, FROM_HERE,
skau 2017/06/20 01:24:17 This was higher than necessary.
- base::Bind(&QueryCups, &cups_connection_, ids),
+ query_runner_->PostTaskAndReplyWithResult(
+ FROM_HERE, base::Bind(&QueryCups, &cups_connection_, ids),
base::Bind(&CupsPrintJobManagerImpl::UpdateJobs,
weak_ptr_factory_.GetWeakPtr()));
}
« no previous file with comments | « chrome/browser/chromeos/printing/cups_print_job_manager_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698