Refine notifications for print jobs in progress.

chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc

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 9806dd7b78c3109ecbe98cdaf53ebae273759849..49a8e9464c480074d8b166dbe06a1e5bcef4b9de 100644
--- a/chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc
+++ b/chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/printing/cups_print_job_manager_impl.h"
 
 #include <cups/cups.h>
+#include <algorithm>
 #include <set>
 #include <string>
 #include <utility>
@@ -33,35 +34,41 @@ namespace {
 // The rate in milliseconds at which we will poll CUPS for print job updates.
 const int kPollRate = 1000;
 
+// How long we'll wait to connect to a printer before declaring an error.
+const int kConnectingTimeout = 20;
+
 // Threshold for giving up on communicating with CUPS.
 const int kRetryMax = 6;
 
+using State = chromeos::CupsPrintJob::State;
+using ErrorCode = chromeos::CupsPrintJob::ErrorCode;
+
+using PrinterReason = printing::PrinterStatus::PrinterReason;
+
 // Returns the equivalient CupsPrintJob#State from a CupsJob#JobState.
 chromeos::CupsPrintJob::State ConvertState(printing::CupsJob::JobState state) {
-  using cpj = chromeos::CupsPrintJob::State;
-
   switch (state) {
     case printing::CupsJob::PENDING:
-      return cpj::STATE_WAITING;
+      return State::STATE_WAITING;
     case printing::CupsJob::HELD:
-      return cpj::STATE_SUSPENDED;
+      return State::STATE_SUSPENDED;
     case printing::CupsJob::PROCESSING:
-      return cpj::STATE_STARTED;
+      return State::STATE_STARTED;
     case printing::CupsJob::CANCELED:
-      return cpj::STATE_CANCELLED;
+      return State::STATE_CANCELLED;
     case printing::CupsJob::COMPLETED:
-      return cpj::STATE_DOCUMENT_DONE;
+      return State::STATE_DOCUMENT_DONE;
     case printing::CupsJob::STOPPED:
-      return cpj::STATE_SUSPENDED;
+      return State::STATE_SUSPENDED;
     case printing::CupsJob::ABORTED:
-      return cpj::STATE_ERROR;
+      return State::STATE_ERROR;
     case printing::CupsJob::UNKNOWN:
       break;
   }
 
   NOTREACHED();
 
-  return cpj::STATE_NONE;
+  return State::STATE_NONE;
 }
 
 chromeos::QueryResult QueryCups(::printing::CupsConnection* connection,
@@ -71,6 +78,108 @@ chromeos::QueryResult QueryCups(::printing::CupsConnection* connection,
   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();
+}
+
+// Returns true if the reason is paper related.
+bool IsPaperReason(const PrinterReason& reason) {
+  static std::set<PrinterReason::Reason> paper_reasons = {
+      PrinterReason::MEDIA_JAM, PrinterReason::MEDIA_EMPTY,
+      PrinterReason::MEDIA_NEEDED, PrinterReason::MEDIA_LOW};
+
+  return paper_reasons.find(reason.reason) != paper_reasons.end();
+}
+
+// Returns true if the reason is ink related.
+bool IsInkReason(const PrinterReason& reason) {
+  return reason.reason == PrinterReason::TONER_EMPTY ||
+         reason.reason == PrinterReason::TONER_LOW;
+}
+
+// Extracts an ErrorCode from PrinterStatus#reasons.  Returns NO_ERROR if there
+// are no reasons which indicate an error.
+chromeos::CupsPrintJob::ErrorCode ErrorCodeFromReasons(
+    const printing::PrinterStatus& printer_status) {
+  const auto begin = printer_status.reasons.begin();
+  const auto end = printer_status.reasons.end();
+
+  if (std::find_if(begin, end, &IsPaperReason) != end) {
+    return chromeos::CupsPrintJob::ErrorCode::PAPER_JAM;
+  } else if (std::find_if(begin, end, &IsInkReason) != end) {
+    return chromeos::CupsPrintJob::ErrorCode::OUT_OF_INK;
+  }
+
+  return chromeos::CupsPrintJob::ErrorCode::NO_ERROR;
+}
+
+// Update job in the processing state.  Returns true if the number of pages
+// changed.
+bool UpdateForProcessing(const printing::PrinterStatus& printer_status,
+                         const printing::CupsJob& job,
+                         chromeos::CupsPrintJob* print_job) {
+  if (ContainsReason(printer_status, PrinterReason::CONNECTING_TO_DEVICE)) {
+    // Check to see if we should time out.
+    base::TimeDelta time_waiting =
+        base::Time::Now() - base::Time::FromTimeT(job.processing_started);
+    if (time_waiting > base::TimeDelta::FromSeconds(kConnectingTimeout)) {
+      print_job->set_state(chromeos::CupsPrintJob::State::STATE_ERROR);
+      print_job->set_error_code(
+          chromeos::CupsPrintJob::ErrorCode::PRINTER_UNREACHABLE);
+      LOG(WARNING) << "Connecting to printer timed out";
+    }
+
+    return false;

[xdai1, 2017/03/13 18:48:37]

return in line 134 instead of here?

[skau, 2017/03/14 02:31:29]

The intention is that we always return false if where in the
CONNECTING_TO_DEVICE state since we know that pages are not being printed if we
can't communicate with the printer.  An appropriate comment has been added.

+  }
+
+  bool pages_updated = false;
+  if (job.current_pages <= 0) {
+    print_job->set_printed_page_number(0);
+    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;
+}
+
+// Returns true if the state of |print_job| changed.  Updates |print_job| with
+// the data in |printer_status| and |job|.
+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:
+      pages_updated = UpdateForProcessing(printer_status, 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 {
@@ -227,8 +336,10 @@ void CupsPrintJobManagerImpl::UpdateJobs(const QueryResult& result) {
 
       CupsPrintJob* print_job = entry->second.get();
 
-      // Update a job we're tracking.
-      JobStateUpdated(print_job, ConvertState(job.state));
+      if (UpdatePrintJob(queue.printer_status, job, print_job)) {
+        // The state of the job changed, notify observers.
+        NotifyJobStateUpdate(print_job);
+      }
 
       // Cleanup completed jobs.
       if (print_job->IsJobFinished()) {
@@ -254,46 +365,41 @@ void CupsPrintJobManagerImpl::UpdateJobs(const QueryResult& result) {
 void CupsPrintJobManagerImpl::PurgeJobs() {
   for (const auto& entry : jobs_) {
     // Declare all lost jobs errors.
-    JobStateUpdated(entry.second.get(), CupsPrintJob::State::STATE_ERROR);
+    CupsPrintJob* job = entry.second.get();
+    job->set_state(CupsPrintJob::State::STATE_ERROR);
+    NotifyJobStateUpdate(job);
   }
 
   jobs_.clear();
 }
 
-void CupsPrintJobManagerImpl::JobStateUpdated(CupsPrintJob* job,
-                                              CupsPrintJob::State new_state) {
-  if (job->state() == new_state)
-    return;
-
-  // We don't track state transitions because some of them might be missed due
-  // to how we query jobs.
-  job->set_state(new_state);
-  switch (new_state) {
-    case CupsPrintJob::State::STATE_NONE:
+void CupsPrintJobManagerImpl::NotifyJobStateUpdate(CupsPrintJob* job) {
+  switch (job->state()) {
+    case State::STATE_NONE:
       // State does not require notification.
       break;
-    case CupsPrintJob::State::STATE_WAITING:
+    case State::STATE_WAITING:
       NotifyJobUpdated(job);
       break;
-    case CupsPrintJob::State::STATE_STARTED:
+    case State::STATE_STARTED:
       NotifyJobStarted(job);
       break;
-    case CupsPrintJob::State::STATE_RESUMED:
+    case State::STATE_PAGE_DONE:
+      NotifyJobUpdated(job);
+      break;
+    case State::STATE_RESUMED:
       NotifyJobResumed(job);
       break;
-    case CupsPrintJob::State::STATE_SUSPENDED:
+    case State::STATE_SUSPENDED:
       NotifyJobSuspended(job);
       break;
-    case CupsPrintJob::State::STATE_CANCELLED:
+    case State::STATE_CANCELLED:
       NotifyJobCanceled(job);
       break;
-    case CupsPrintJob::State::STATE_ERROR:
+    case State::STATE_ERROR:
       NotifyJobError(job);
       break;
-    case CupsPrintJob::State::STATE_PAGE_DONE:
-      NOTREACHED() << "CUPS does not surface this state so it's not expected";
-      break;
-    case CupsPrintJob::State::STATE_DOCUMENT_DONE:
+    case State::STATE_DOCUMENT_DONE:
       NotifyJobDone(job);
       break;
   }