Added CUPS error handling.

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/6250144

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@73706 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 31 insertions, 23 deletions

diff --git a/chrome/service/cloud_print/print_system_cups.cc b/chrome/service/cloud_print/print_system_cups.cc
index 0e40dff..6cb26e1 100644
--- a/chrome/service/cloud_print/print_system_cups.cc
+++ b/chrome/service/cloud_print/print_system_cups.cc
@@ -545,19 +545,21 @@ bool PrintSystemCUPS::GetJobDetails(const std::string& printer_name,
   cups_job_t* jobs = NULL;
   int num_jobs = GetJobs(&jobs, server_info->url,
                          short_printer_name.c_str(), 1, -1);
-
+  bool error = (num_jobs == 0) && (cupsLastError() > IPP_OK_EVENTS_COMPLETE);
+  if (error) {
+    VLOG(1) << "CP_CUPS: Error getting jobs from CUPS server. Printer:"
+            << printer_name
+            << " Error: "
+            << static_cast<int>(cupsLastError());
+    return false;
+  }
 
   // Check if the request is for dummy dry run job.
   // We check this after calling GetJobs API to see if this printer is actually
   // accessible through CUPS.
   if (job_id == kDryRunJobId) {
-    if (num_jobs >= 0) {
-      job_details->status = PRINT_JOB_STATUS_COMPLETED;
-      VLOG(1) << "CP_CUPS: Dry run job succeeded for: " << printer_name;
-    } else {
-      job_details->status = PRINT_JOB_STATUS_ERROR;
-      VLOG(1) << "CP_CUPS: Dry run job faield for: " << printer_name;
-    }
+    job_details->status = PRINT_JOB_STATUS_COMPLETED;
+    VLOG(1) << "CP_CUPS: Dry run job succeeded for: " << printer_name;
     return true;
   }
 
diff --git a/printing/backend/print_backend_cups.cc b/printing/backend/print_backend_cups.cc
index 3b38c3f..ee8907e 100644
--- a/printing/backend/print_backend_cups.cc
+++ b/printing/backend/print_backend_cups.cc
@@ -113,8 +113,13 @@ bool PrintBackendCUPS::EnumeratePrinters(PrinterList* printer_list) {
 
   cups_dest_t* destinations = NULL;
   int num_dests = GetDests(&destinations);
-  // TODO(gene): Figure out how to get an error code from cupsGetDests so we can
-  // differentiate between the enumeration failing and there being 0 printers.
+  if ((num_dests == 0) && (cupsLastError() > IPP_OK_EVENTS_COMPLETE)) {
+    VLOG(1) << "CP_CUPS: Error getting printers from CUPS server. Server: "
+            << print_server_url_
+            << " Error: "
+            << static_cast<int>(cupsLastError());
+    return false;
+  }
 
   for (int printer_index = 0; printer_index < num_dests; printer_index++) {
     const cups_dest_t& printer = destinations[printer_index];
@@ -238,27 +243,28 @@ FilePath PrintBackendCUPS::GetPPD(const char* name) {
     // Note: After looking at CUPS sources, it looks like non-blocking
     // connection will timeout after 10 seconds of no data period. And it will
     // return the same way as if data was completely and sucessfully downloaded.
-    // To distinguish error case from the normal return, will check result file
-    // size agains content length.
     HttpConnectionCUPS http(print_server_url_);
     http.SetBlocking(blocking_);
     ppd_file_path = cupsGetPPD2(http.http(), name);
     // Check if the get full PPD, since non-blocking call may simply return
     // normally after timeout expired.
     if (ppd_file_path) {
+      // There is no reliable way right now to detect full and complete PPD
+      // get downloaded. If we reach http timeout, it may simply return
+      // downloaded part as a full response. It might be good enough to check
+      // http->data_remaining or http->_data_remaining, unfortunately http_t
+      // is an internal structure and fields are not exposed in CUPS headers.
+      // httpGetLength or httpGetLength2 returning the full content size.
+      // Comparing file size against that content length might be unreliable
+      // since some http reponses are encoded and content_length > file size.
+      // Let's just check for the obvious CUPS and http errors here.
       ppd_path = FilePath(ppd_file_path);
-      off_t content_len = httpGetLength2(http.http());
-      int64 ppd_size = 0;
-      // This is a heuristic to detect if we reached timeout. If we see content
-      // length is larger that the actual file we downloaded it means timeout
-      // reached. Sometimes http can be compressed, and in that case the
-      // the content length will be smaller than the actual payload (not sure
-      // if CUPS support such responses).
-      if (!file_util::GetFileSize(ppd_path, &ppd_size) ||
-          content_len > ppd_size) {
+      ipp_status_t error_code = cupsLastError();
+      int http_error = httpError(http.http());
+      if (error_code > IPP_OK_EVENTS_COMPLETE || http_error != 0) {
         LOG(ERROR) << "Error downloading PPD file for: " << name
-                   << ", file size: "  << ppd_size
-                   << ", content length: " << content_len;
+                   << ", CUPS error: " << static_cast<int>(error_code)
+                   << ", HTTP error: " << http_error;
         file_util::Delete(ppd_path, false);
         ppd_path.clear();
       }