Add a method to query IPP printers for attributes.

printing/backend/cups_jobs.cc

Index: printing/backend/cups_jobs.cc
diff --git a/printing/backend/cups_jobs.cc b/printing/backend/cups_jobs.cc
index e73ae8fb331041aaa5754074a924856980eca18d..964a14314ecbb63d85b52c7402b62ef4c40b9b23 100644
--- a/printing/backend/cups_jobs.cc
+++ b/printing/backend/cups_jobs.cc
@@ -11,8 +11,11 @@
 #include <memory>
 
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
+#include "printing/backend/cups_deleters.h"
 
 namespace printing {
 namespace {
@@ -26,6 +29,10 @@ const char kPrinterState[] = "printer-state";
 const char kPrinterStateReasons[] = "printer-state-reasons";
 const char kPrinterStateMessage[] = "printer-state-message";
 
+const char kPrinterMakeAndModel[] = "printer-make-and-model";
+const char kIppVersionsSupported[] = "ipp-versions-supported";
+const char kIppFeaturesSupported[] = "ipp-features-supported";
+
 // job attributes
 const char kJobUri[] = "job-uri";
 const char kJobId[] = "job-id";
@@ -44,6 +51,9 @@ const char kLimit[] = "limit";
 const char kCompleted[] = "completed";
 const char kNotCompleted[] = "not-completed";
 
+// ipp features
+const char kIppEverywhere[] = "ipp-everywhere";
+
 // printer state severities
 const char kSeverityReport[] = "report";
 const char kSeverityWarn[] = "warning";
@@ -88,10 +98,17 @@ const char kInterpreterResourceUnavailable[] =
 constexpr std::array<const char* const, 3> kPrinterAttributes{
     {kPrinterState, kPrinterStateReasons, kPrinterStateMessage}};
 
-std::unique_ptr<ipp_t, void (*)(ipp_t*)> WrapIpp(ipp_t* ipp) {
-  return std::unique_ptr<ipp_t, void (*)(ipp_t*)>(ipp, &ippDelete);
+constexpr std::array<const char* const, 3> kPrinterInfo{
+    {kPrinterMakeAndModel, kIppVersionsSupported, kIppFeaturesSupported}};
+
+using IppPtr = std::unique_ptr<ipp_t, void (*)(ipp_t*)>;
+
+IppPtr WrapIpp(ipp_t* ipp) {
+  return IppPtr(ipp, &ippDelete);
 }
 
+using HttpPtr = std::unique_ptr<http_t, HttpDeleter>;
+
 // Converts an IPP attribute |attr| to the appropriate JobState enum.
 CupsJob::JobState ToJobState(ipp_attribute_t* attr) {
   DCHECK_EQ(IPP_TAG_ENUM, ippGetValueTag(attr));
@@ -295,6 +312,10 @@ PrinterStatus::PrinterStatus(const PrinterStatus& other) = default;
 
 PrinterStatus::~PrinterStatus() = default;
 
+PrinterInfo::PrinterInfo() = default;
+
+PrinterInfo::~PrinterInfo() = default;
+
 void ParseJobsResponse(ipp_t* response,
                        const std::string& printer_id,
                        std::vector<CupsJob>* jobs) {
@@ -309,6 +330,80 @@ void ParseJobsResponse(ipp_t* response,
   }
 }
 
+IppPtr GetPrinterAttributes(http_t* http,
+                            const std::string& printer_uri,
+                            int num_attributes,
+                            const char* const* attributes,
+                            ipp_status_t* status) {
+  DCHECK(http);
+
+  auto request = WrapIpp(ippNewRequest(IPP_OP_GET_PRINTER_ATTRIBUTES));
+
+  ippAddString(request.get(), IPP_TAG_OPERATION, IPP_TAG_URI, kPrinterUri,
+               nullptr, printer_uri.data());
+
+  ippAddStrings(request.get(), IPP_TAG_OPERATION, IPP_TAG_KEYWORD,
+                kRequestedAttributes, num_attributes, nullptr, attributes);
+
+  auto response =
+      WrapIpp(cupsDoRequest(http, request.release(), printer_uri.c_str()));
+  *status = ippGetStatusCode(response.get());
+
+  return response;
+}
+
+std::pair<int, int> ToVersionNumber(const std::string& version, bool* success) {

[Carlson, 2017/05/25 19:04:36]

Not sure this is a formal thing, but it seems like the more common idiom for a
function like this is to return the bool and pass the std::pair<int, int> as a
pointer, not the reverse.

I don't have data to back that up, though.

[Carlson, 2017/05/25 19:04:36]

Function comment?  (Also, shouldn't these free functions be in the anonymous
namespace?)

[skau, 2017/05/27 02:01:20]

I've seen both but returning a bool has been more common for us.

+  std::vector<base::StringPiece> pieces = SplitStringPiece(
+      version, ".", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  DCHECK_EQ(2U, pieces.size());

[Carlson, 2017/05/25 19:04:36]

This is input validation, and so can't be a DCHECK, I think.  If a printer gives
you a stupid string back, it shouldn't crash the browser.

[skau, 2017/05/27 02:01:20]

Done.

+  int major;
+  int minor;
+  *success = base::StringToInt(pieces[0], &major) &&
+             base::StringToInt(pieces[1], &minor);
+
+  return {major, minor};

[Carlson, 2017/05/25 19:04:36]

Structure this so that we return a known bad major,minor tuple in the case of
lack of success?  The caller shouldn't use the values in that case, but
returning uninitialized memory gives me heeby-jeebies.

(This sort of function seems like a good use case for StatusOr, if we supported
that in the main browser code.  :P)

[skau, 2017/05/27 02:01:20]

Done.

+}
+
+// Extracts PrinterInfo fields from |response| and populates |printer_info|.
+// Returns true if at least printer-make-and-model and ipp-versions-supported
+// were read.
+bool ParsePrinterInfo(ipp_t* response, PrinterInfo* printer_info) {
+  for (ipp_attribute_t* attr = ippFirstAttribute(response); attr != nullptr;
+       attr = ippNextAttribute(response)) {
+    base::StringPiece name = ippGetName(attr);
+    if (name.empty()) {

[Carlson, 2017/05/25 19:04:36]

This case is not explicitly needed, but if you like it, cool.

[skau, 2017/05/27 02:01:19]

Done.

+      continue;
+    }
+
+    if (name == kPrinterMakeAndModel) {

[Carlson, 2017/05/25 19:04:36]

Paranoid favor, can you make the StringPiece conversion explicit when you're
using '=='?  i.e. if (name == StringPiece(kPrinterMakeAndModel))

  Otherwise, if someone changes the type of 'name' to const char * (or auto),
things will still compile but break in surprising ways.  

Not that that's ever happened to me.  Nope.  Never.  And it wasn't a real PITA
to debug, either.

[skau, 2017/05/27 02:01:20]

Sounds like a traumatic experience.  Let's avoid that.

+      DCHECK_EQ(IPP_TAG_TEXT, ippGetValueTag(attr));
+      printer_info->make_and_model = ippGetString(attr, 0, nullptr);
+    } else if (name == kIppVersionsSupported) {
+      std::vector<std::string> ipp_versions;
+      ParseCollection(attr, &ipp_versions);
+      for (const std::string& version : ipp_versions) {
+        bool success;
+        std::pair<int, int> major_minor = ToVersionNumber(version, &success);
+        if (success) {
+          printer_info->ipp_versions.push_back(major_minor);
+        }
+      }
+    } else if (name == kIppFeaturesSupported) {
+      std::vector<std::string> features;
+      ParseCollection(attr, &features);
+      for (const std::string& feature : features) {
+        if (feature == kIppEverywhere) {
+          printer_info->ipp_everywhere = true;
+          break;
+        }
+      }
+    }
+  }
+
+  return !printer_info->make_and_model.empty() &&
+         !printer_info->ipp_versions.empty();
+}
+
 void ParsePrinterStatus(ipp_t* response, PrinterStatus* printer_status) {
   for (ipp_attribute_t* attr = ippFirstAttribute(response); attr != nullptr;
        attr = ippNextAttribute(response)) {
@@ -332,25 +427,31 @@ void ParsePrinterStatus(ipp_t* response, PrinterStatus* printer_status) {
   }
 }
 
+bool GetPrinterInfo(const std::string& address,
+                    const int port,
+                    const std::string& resource,
+                    PrinterInfo* printer_info) {
+  ipp_status_t status;
+  HttpPtr http =
+      HttpPtr(httpConnect2(address.data(), port, nullptr, AF_INET,
+                           HTTP_ENCRYPTION_IF_REQUESTED, 0, 200, nullptr));
+  auto response = GetPrinterAttributes(
+      http.get(), resource, kPrinterInfo.size(), kPrinterInfo.data(), &status);
+  return status == IPP_STATUS_OK &&
+         ParsePrinterInfo(response.get(), printer_info);
+}
+
 bool GetPrinterStatus(http_t* http,
                       const std::string& printer_id,
                       PrinterStatus* printer_status) {
-  DCHECK(http);
-
-  auto request = WrapIpp(ippNewRequest(IPP_OP_GET_PRINTER_ATTRIBUTES));
-
+  ipp_status_t status;
   const std::string printer_uri = PrinterUriFromName(printer_id);
-  ippAddString(request.get(), IPP_TAG_OPERATION, IPP_TAG_URI, kPrinterUri,
-               nullptr, printer_uri.data());
-
-  ippAddStrings(request.get(), IPP_TAG_OPERATION, IPP_TAG_KEYWORD,
-                kRequestedAttributes, kPrinterAttributes.size(), nullptr,
-                kPrinterAttributes.data());
 
   auto response =
-      WrapIpp(cupsDoRequest(http, request.release(), printer_uri.c_str()));
+      GetPrinterAttributes(http, printer_uri, kPrinterAttributes.size(),
+                           kPrinterAttributes.data(), &status);
 
-  if (ippGetStatusCode(response.get()) != IPP_STATUS_OK)
+  if (status != IPP_STATUS_OK)
     return false;
 
   ParsePrinterStatus(response.get(), printer_status);