Convert to CupsPrintJobManagerImpl to TaskScheduler.

chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/printing/cups_print_job_manager_impl.h"
 
 #include <cups/cups.h>
 #include <algorithm>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #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_runner_util.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/threading/thread_task_runner_handle.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"
 #include "chrome/browser/chromeos/printing/printers_manager_factory.h"
 #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"
@@ skipping 64 matching lines @@
       return State::STATE_ERROR;
     case printing::CupsJob::UNKNOWN:
       break;
   }
 
   NOTREACHED();
 
   return State::STATE_NONE;
 }
 
-chromeos::QueryResult QueryCups(::printing::CupsConnection* connection,
-                                const std::vector<std::string>& printer_ids) {
-  chromeos::QueryResult result;
-  result.success = connection->GetJobs(printer_ids, &result.queues);
-  return result;
-}
-
 // Returns true if |printer_status|.reasons contains |reason|.
 bool ContainsReason(const printing::PrinterStatus printer_status,
                     PrinterReason::Reason reason) {
   return std::find_if(printer_status.reasons.begin(),
                       printer_status.reasons.end(),
                       [&reason](const PrinterReason& r) {
                         return r.reason == reason;
                       }) != printer_status.reasons.end();
 }
 
@@ skipping 43 matching lines @@
     print_job->set_state(State::STATE_STARTED);
   } else {
     pages_updated = job.current_pages != print_job->printed_page_number();
     print_job->set_printed_page_number(job.current_pages);
     print_job->set_state(State::STATE_PAGE_DONE);
   }
 
   return pages_updated;
 }
 
+// Updates the state of a print job based on |printer_status| and |job|.
+// Returns true if observers need to be notified of an update.
+bool UpdatePrintJob(const ::printing::PrinterStatus& printer_status,
+                    const ::printing::CupsJob& job,
+                    chromeos::CupsPrintJob* print_job) {
+  DCHECK_EQ(job.id, print_job->job_id());
+
+  State old_state = print_job->state();
+
+  bool pages_updated = false;
+  switch (job.state) {
+    case ::printing::CupsJob::PROCESSING:
+      if (ContainsReason(printer_status, PrinterReason::CONNECTING_TO_DEVICE)) {
+        if (EnforceTimeout(job, print_job)) {
+          LOG(WARNING) << "Connecting to printer timed out";
+          print_job->set_expired(true);
+        }
+      } else {
+        pages_updated = UpdateCurrentPage(job, print_job);
+      }
+      break;
+    case ::printing::CupsJob::COMPLETED:
+      DCHECK_GE(job.current_pages, print_job->total_page_number());
+      print_job->set_state(State::STATE_DOCUMENT_DONE);
+      break;
+    case ::printing::CupsJob::ABORTED:
+    case ::printing::CupsJob::CANCELED:
+      print_job->set_error_code(ErrorCodeFromReasons(printer_status));
+    // fall through
+    default:
+      print_job->set_state(ConvertState(job.state));
+      break;
+  }
+
+  return print_job->state() != old_state || pages_updated;
+}
+
 }  // namespace
 
 namespace chromeos {
 
 QueryResult::QueryResult() = default;
 
 QueryResult::QueryResult(const QueryResult& other) = default;
 
 QueryResult::~QueryResult() = default;
 
+CupsWrapper::CupsWrapper()
+    : cups_connection_(GURL(), HTTP_ENCRYPT_NEVER, false) {
+  DETACH_FROM_SEQUENCE(sequence_checker_);

[Carlson, 2017/06/26 18:42:01]

I don't understand what this does (which is probably just be being unfamiliar
with this mechanism).  Are constructors for sequence checkers normally
automatically required to be sequenced?  I don't understand why you need this
here (as opposed to just *not* having a DCHECK_CALLED_ON_VALID_SEQUENCE() call).

[skau, 2017/06/26 23:15:16]

This object is actually being created on a different sequence than the one it's
being used on.  This macro indicates that.  Otherwise, it associates with the
sequence it was created on.

+}
+
+CupsWrapper::~CupsWrapper() = default;
+
+void CupsWrapper::QueryCups(const std::vector<std::string>& printer_ids,
+                            QueryResult* result) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  base::ThreadRestrictions::AssertIOAllowed();
+
+  result->success = cups_connection_.GetJobs(printer_ids, &result->queues);
+}
+
+void CupsWrapper::CancelJobImpl(const std::string& printer_id,
+                                const int job_id) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  base::ThreadRestrictions::AssertIOAllowed();
+
+  std::unique_ptr<::printing::CupsPrinter> printer =
+      cups_connection_.GetPrinter(printer_id);
+  if (!printer) {
+    LOG(WARNING) << "Printer not found: " << printer_id;
+    return;
+  }
+
+  if (!printer->CancelJob(job_id)) {
+    // This is not expected to fail but log it if it does.
+    LOG(WARNING) << "Cancelling job failed.  Job may be stuck in queue.";
+  }
+}
+
 CupsPrintJobManagerImpl::CupsPrintJobManagerImpl(Profile* profile)
     : CupsPrintJobManager(profile),
-      cups_connection_(GURL(), HTTP_ENCRYPT_NEVER, false),
+      cups_wrapper_(),
+      cups_wrapper_ptr_factory_(&cups_wrapper_),
+      query_runner_(base::CreateSequencedTaskRunnerWithTraits(
+          base::TaskTraits(base::TaskPriority::BACKGROUND,
+                           base::MayBlock(),
+                           base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN))),
+      browser_thread_runner_(content::BrowserThread::GetTaskRunnerForThread(
+          content::BrowserThread::UI)),
       weak_ptr_factory_(this) {
   registrar_.Add(this, chrome::NOTIFICATION_PRINT_JOB_EVENT,
                  content::NotificationService::AllSources());
 }
 
 CupsPrintJobManagerImpl::~CupsPrintJobManagerImpl() {}
 
 // Must be run from the UI thread.
 void CupsPrintJobManagerImpl::CancelPrintJob(CupsPrintJob* job) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(browser_thread_runner_->RunsTasksInCurrentSequence());
 
   // 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();
 
   // Stop montioring jobs after we cancel them.  The user no longer cares.
   jobs_.erase(job->GetUniqueId());
 
   // Be sure to copy out all relevant fields.  |job| may be destroyed after we

[Carlson, 2017/06/26 18:42:01]

Is this comment still relevant?

[skau, 2017/06/26 23:15:16]

It's still true but it duplicates the comment on #271.  Deleted.

   // exit this scope.
-  content::BrowserThread::GetBlockingPool()->PostTask(
+  query_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&CupsPrintJobManagerImpl::CancelJobImpl,
-                 weak_ptr_factory_.GetWeakPtr(), printer_id, job_id));
+      base::Bind(&CupsWrapper::CancelJobImpl,
+                 cups_wrapper_ptr_factory_.GetWeakPtr(), printer_id, job_id));
 }
 
 bool CupsPrintJobManagerImpl::SuspendPrintJob(CupsPrintJob* job) {
   NOTREACHED() << "Pause printer is not implemented";
   return false;
 }
 
 bool CupsPrintJobManagerImpl::ResumePrintJob(CupsPrintJob* job) {
   NOTREACHED() << "Resume printer is not implemented";
   return false;
@@ skipping 15 matching lines @@
     CreatePrintJob(base::UTF16ToUTF8(document->settings().device_name()),
                    base::UTF16ToUTF8(document->settings().title()),
                    job_details->job_id(), document->page_count());
   }
 }
 
 bool CupsPrintJobManagerImpl::CreatePrintJob(const std::string& printer_name,
                                              const std::string& title,
                                              int job_id,
                                              int total_page_number) {
+  DCHECK(browser_thread_runner_->RunsTasksInCurrentSequence());
+
   auto printer =
       PrintersManagerFactory::GetForBrowserContext(profile_)->GetPrinter(
           printer_name);
   if (!printer) {
     LOG(WARNING) << "Printer was removed while job was in progress.  It cannot "
                     "be tracked";
     return false;
   }
 
   // Records the number of jobs we're currently tracking when a new job is
@@ skipping 17 matching lines @@
   ScheduleQuery(base::TimeDelta());
 
   return true;
 }
 
 void CupsPrintJobManagerImpl::ScheduleQuery() {
   ScheduleQuery(base::TimeDelta::FromMilliseconds(kPollRate));
 }
 
 void CupsPrintJobManagerImpl::ScheduleQuery(const base::TimeDelta& delay) {
+  DCHECK(browser_thread_runner_->RunsTasksInCurrentSequence());
+
   if (!in_query_) {
     in_query_ = true;
 
-    base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+    browser_thread_runner_->PostDelayedTask(
         FROM_HERE,
         base::Bind(&CupsPrintJobManagerImpl::PostQuery,
                    weak_ptr_factory_.GetWeakPtr()),
         delay);
   }
 }
 
 void CupsPrintJobManagerImpl::PostQuery() {
+  DCHECK(browser_thread_runner_->RunsTasksInCurrentSequence());
+
   // The set of active printers is expected to be small.
   std::set<std::string> printer_ids;
   for (const auto& entry : jobs_) {
     printer_ids.insert(entry.second->printer().id());
   }
   std::vector<std::string> ids{printer_ids.begin(), printer_ids.end()};
 
-  content::BrowserThread::PostTaskAndReplyWithResult(
-      content::BrowserThread::FILE_USER_BLOCKING, FROM_HERE,
-      base::Bind(&QueryCups, &cups_connection_, ids),
+  auto result = base::MakeUnique<QueryResult>();
+  QueryResult* result_ptr = result.get();
+  // Runs a query on query_runner_ which will rejoin browser_thread_runner_.
+  query_runner_->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&CupsWrapper::QueryCups,
+                 cups_wrapper_ptr_factory_.GetWeakPtr(), ids, result_ptr),
       base::Bind(&CupsPrintJobManagerImpl::UpdateJobs,
-                 weak_ptr_factory_.GetWeakPtr()));
-}
-
-bool CupsPrintJobManagerImpl::UpdatePrintJob(
-    const ::printing::PrinterStatus& printer_status,
-    const ::printing::CupsJob& job,
-    CupsPrintJob* print_job) {
-  DCHECK_EQ(job.id, print_job->job_id());
-
-  State old_state = print_job->state();
-
-  bool pages_updated = false;
-  switch (job.state) {
-    case ::printing::CupsJob::PROCESSING:
-      if (ContainsReason(printer_status, PrinterReason::CONNECTING_TO_DEVICE)) {
-        if (EnforceTimeout(job, print_job)) {
-          LOG(WARNING) << "Connecting to printer timed out";
-          print_job->set_expired(true);
-        }
-      } else {
-        pages_updated = UpdateCurrentPage(job, print_job);
-      }
-      break;
-    case ::printing::CupsJob::COMPLETED:
-      DCHECK_GE(job.current_pages, print_job->total_page_number());
-      print_job->set_state(State::STATE_DOCUMENT_DONE);
-      break;
-    case ::printing::CupsJob::ABORTED:
-    case ::printing::CupsJob::CANCELED:
-      print_job->set_error_code(ErrorCodeFromReasons(printer_status));
-    // fall through
-    default:
-      print_job->set_state(ConvertState(job.state));
-      break;
-  }
-
-  return print_job->state() != old_state || pages_updated;
+                 weak_ptr_factory_.GetWeakPtr(), base::Passed(&result)));
 }
 
 // Use job information to update local job states.  Previously completed jobs
-// could be in |jobs| but those are ignored as we will not emit updates for them
-// after they are completed.
-void CupsPrintJobManagerImpl::UpdateJobs(const QueryResult& result) {
-  const std::vector<::printing::QueueStatus>& queues = result.queues;
+// could be in |jobs| but those are ignored as we will not emit updates for
+// them after they are completed.
+void CupsPrintJobManagerImpl::UpdateJobs(std::unique_ptr<QueryResult> result) {
+  DCHECK(browser_thread_runner_->RunsTasksInCurrentSequence());
+
+  const std::vector<::printing::QueueStatus>& queues = result->queues;
 
   // Query has completed.  Allow more queries.
   in_query_ = false;
 
   // If the query failed, either retry or purge.
-  if (!result.success) {
+  if (!result->success) {
     retry_count_++;
     LOG(WARNING) << "Failed to query CUPS for queue status.  Schedule retry ("
                  << retry_count_ << ")";
     if (retry_count_ > kRetryMax) {
       LOG(ERROR) << "CUPS is unreachable.  Giving up on all jobs.";
       PurgeJobs();
     } else {
       // Schedule another query with a larger delay.
       DCHECK_GE(1, retry_count_);
       ScheduleQuery(
@@ skipping 33 matching lines @@
         active_jobs.push_back(key);
       }
     }
   }
 
   // Keep polling until all jobs complete or error.
   if (!active_jobs.empty()) {
     // During normal operations, we poll at the default rate.
     ScheduleQuery();
   } else if (!jobs_.empty()) {
-    // We're tracking jobs that we didn't receive an update for.  Something bad
-    // has happened.
+    // We're tracking jobs that we didn't receive an update for.  Something
+    // bad has happened.
     LOG(ERROR) << "Lost track of (" << jobs_.size() << ") jobs";
     PurgeJobs();
   }
 }
 
 void CupsPrintJobManagerImpl::PurgeJobs() {
+  DCHECK(browser_thread_runner_->RunsTasksInCurrentSequence());
+
   for (const auto& entry : jobs_) {
     // Declare all lost jobs errors.
     RecordJobResult(LOST);
     CupsPrintJob* job = entry.second.get();
     job->set_state(CupsPrintJob::State::STATE_ERROR);
     NotifyJobStateUpdate(job);
   }
 
   jobs_.clear();
 }
 
-void CupsPrintJobManagerImpl::CancelJobImpl(const std::string& printer_id,
-                                            const int job_id) {
-  std::unique_ptr<::printing::CupsPrinter> printer =
-      cups_connection_.GetPrinter(printer_id);
-  if (!printer) {
-    LOG(WARNING) << "Printer not found: " << printer_id;
-    return;
-  }
+void CupsPrintJobManagerImpl::NotifyJobStateUpdate(CupsPrintJob* job) {
+  DCHECK(browser_thread_runner_->RunsTasksInCurrentSequence());
 
-  if (!printer->CancelJob(job_id)) {
-    // This is not expected to fail but log it if it does.
-    LOG(WARNING) << "Cancelling job failed.  Job may be stuck in queue.";
-  }
-}
-
-void CupsPrintJobManagerImpl::NotifyJobStateUpdate(CupsPrintJob* job) {
   switch (job->state()) {
     case State::STATE_NONE:
       // State does not require notification.
       break;
     case State::STATE_WAITING:
       NotifyJobUpdated(job);
       break;
     case State::STATE_STARTED:
       NotifyJobStarted(job);
       break;
@@ skipping 17 matching lines @@
       break;
   }
 }
 
 // static
 CupsPrintJobManager* CupsPrintJobManager::CreateInstance(Profile* profile) {
   return new CupsPrintJobManagerImpl(profile);
 }
 
 }  // namespace chromeos