Skip to content

Commit

Permalink
Read basic printer info differently for macOS in CUPS IPP code
Browse files Browse the repository at this point in the history
CUPS destination names and info are organized differently of macOS and
other CUPS platforms. On macOS, the destination name is stored in the
"printer-info" option, while the description (usually the make and
model) is stored in the "printer-make-and-model" option. In contrast,
on other platforms, the name is reliably stored in the |name|
attribute of the cups_dest_t struct, and the description is stored in
the "printer-info" option.

Additionally, populate the |device_name| attribute of |printer_info|
in CupsPrinter::ToPrinterInfo(), so the name gets plumbed up to the
print preview UI.

Meanwhile, consolidate some common CUPS consts into
print_backend_consts.h.

Bug: 226176
Change-Id: Idb71bf7ad7602dcf185d4f49d81294b13d70365d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066297
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746974}
  • Loading branch information
Daniel Hosseinian authored and Commit Bot committed Mar 4, 2020
1 parent e83b7c0 commit 920bb45
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 36 deletions.
49 changes: 28 additions & 21 deletions printing/backend/cups_printer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,11 @@
#include <utility>

#include "base/strings/string_number_conversions.h"
#include "build/build_config.h"
#include "printing/backend/cups_connection.h"
#include "printing/backend/print_backend.h"
#include "printing/backend/print_backend_consts.h"

namespace {

const char kCUPSDeviceUri[] = "device-uri";
const char kCUPSPrinterInfoOpt[] = "printer-info";
const char kCUPSPrinterStateOpt[] = "printer-state";

} // namespace

namespace printing {

CupsPrinter::CupsPrinter(http_t* http, ScopedDestination dest)
Expand Down Expand Up @@ -89,20 +82,26 @@ bool CupsPrinter::ToPrinterInfo(PrinterBasicInfo* printer_info) const {
printer_info->printer_name = printer->name;
printer_info->is_default = printer->is_default;

const char* info = cupsGetOption(kCUPSPrinterInfoOpt, printer->num_options,
printer->options);
if (info)
printer_info->printer_description = info;

const char* state = cupsGetOption(kCUPSPrinterStateOpt, printer->num_options,
const std::string info = GetInfo();
const std::string make_and_model = GetMakeAndModel();

#if defined(OS_MACOSX)
// On Mac, "printer-info" option specifies the human-readable printer name,
// while "printer-make-and-model" specifies the printer description.
printer_info->display_name = info;
printer_info->printer_description = make_and_model;
#else
// On other platforms, "printer-info" specifies the printer description.
printer_info->display_name = printer->name;
printer_info->printer_description = info;
#endif // defined(OS_MACOSX)

const char* state = cupsGetOption(kCUPSOptPrinterState, printer->num_options,
printer->options);
if (state)
base::StringToInt(state, &printer_info->printer_status);

const char* drv_info =
cupsGetOption(kDriverNameTagName, printer->num_options, printer->options);
if (drv_info)
printer_info->options[kDriverInfoTagName] = *drv_info;
printer_info->options[kDriverInfoTagName] = make_and_model;

// Store printer options.
for (int opt_index = 0; opt_index < printer->num_options; ++opt_index) {
Expand All @@ -118,14 +117,22 @@ std::string CupsPrinter::GetName() const {
}

std::string CupsPrinter::GetMakeAndModel() const {
const char* make_and_model = cupsGetOption(
kDriverNameTagName, destination_->num_options, destination_->options);
const char* make_and_model =
cupsGetOption(kCUPSOptPrinterMakeAndModel, destination_->num_options,
destination_->options);

return make_and_model ? std::string(make_and_model) : std::string();
}

std::string CupsPrinter::GetInfo() const {
const char* info = cupsGetOption(
kCUPSOptPrinterInfo, destination_->num_options, destination_->options);

return info ? std::string(info) : std::string();
}

std::string CupsPrinter::GetUri() const {
const char* uri = cupsGetOption(kCUPSDeviceUri, destination_->num_options,
const char* uri = cupsGetOption(kCUPSOptDeviceUri, destination_->num_options,
destination_->options);
return uri ? std::string(uri) : std::string();
}
Expand Down
3 changes: 3 additions & 0 deletions printing/backend/cups_printer.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ class PRINTING_EXPORT CupsPrinter : public CupsOptionProvider {

std::string GetMakeAndModel() const;

// Returns the "printer-info" option of the printer as configured in CUPS.
std::string GetInfo() const;

std::string GetUri() const;

// Lazily initialize dest info as it can require a network call
Expand Down
14 changes: 12 additions & 2 deletions printing/backend/print_backend_consts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,22 @@

#include "printing/backend/print_backend_consts.h"

// TODO(dhoss): Evaluate removing the strings used as keys for
// |PrinterBasicInfo.options| in favor of fields in PrinterBasicInfo.
const char kCUPSBlocking[] = "cups_blocking";
const char kCUPSEncryption[] = "cups_encryption";
const char kCUPSEnterprisePrinter[] = "cupsEnterprisePrinter";
const char kCUPSPrintServerURL[] = "print_server_url";
const char kDriverInfoTagName[] = "system_driverinfo";
const char kDriverNameTagName[] = "printer-make-and-model"; // Match CUPS.
const char kLocationTagName[] = "printer-location"; // Match CUPS.
const char kDriverNameTagName[] = "printer-make-and-model";
const char kLocationTagName[] = "printer-location";
const char kValueFalse[] = "false";
const char kValueTrue[] = "true";

// The following values must match those defined in CUPS.
const char kCUPSOptDeviceUri[] = "device-uri";
const char kCUPSOptPrinterInfo[] = "printer-info";
const char kCUPSOptPrinterLocation[] = "printer-location";
const char kCUPSOptPrinterMakeAndModel[] = "printer-make-and-model";
const char kCUPSOptPrinterState[] = "printer-state";
const char kCUPSOptPrinterType[] = "printer-type";
8 changes: 8 additions & 0 deletions printing/backend/print_backend_consts.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,12 @@ PRINTING_EXPORT extern const char kLocationTagName[];
PRINTING_EXPORT extern const char kValueFalse[];
PRINTING_EXPORT extern const char kValueTrue[];

// CUPS destination option names.
PRINTING_EXPORT extern const char kCUPSOptDeviceUri[];
PRINTING_EXPORT extern const char kCUPSOptPrinterInfo[];
PRINTING_EXPORT extern const char kCUPSOptPrinterLocation[];
PRINTING_EXPORT extern const char kCUPSOptPrinterMakeAndModel[];
PRINTING_EXPORT extern const char kCUPSOptPrinterState[];
PRINTING_EXPORT extern const char kCUPSOptPrinterType[];

#endif // PRINTING_BACKEND_PRINT_BACKEND_CONSTS_H_
18 changes: 5 additions & 13 deletions printing/backend/print_backend_cups.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@

namespace printing {

namespace {

const char kCUPSPrinterInfoOpt[] = "printer-info";
const char kCUPSPrinterStateOpt[] = "printer-state";
const char kCUPSPrinterTypeOpt[] = "printer-type";

} // namespace

PrintBackendCUPS::PrintBackendCUPS(const GURL& print_server_url,
http_encryption_t encryption,
bool blocking,
Expand All @@ -50,7 +42,7 @@ bool PrintBackendCUPS::PrinterBasicInfoFromCUPS(
// CUPS can have 'printers' that are actually scanners. (not MFC)
// At least on Mac. Check for scanners and skip them.
const char* type_str =
cupsGetOption(kCUPSPrinterTypeOpt, printer.num_options, printer.options);
cupsGetOption(kCUPSOptPrinterType, printer.num_options, printer.options);
if (type_str) {
int type;
if (base::StringToInt(type_str, &type) && (type & CUPS_PRINTER_SCANNER))
Expand All @@ -61,15 +53,15 @@ bool PrintBackendCUPS::PrinterBasicInfoFromCUPS(
printer_info->is_default = printer.is_default;

const char* info =
cupsGetOption(kCUPSPrinterInfoOpt, printer.num_options, printer.options);
cupsGetOption(kCUPSOptPrinterInfo, printer.num_options, printer.options);

const char* state =
cupsGetOption(kCUPSPrinterStateOpt, printer.num_options, printer.options);
cupsGetOption(kCUPSOptPrinterState, printer.num_options, printer.options);
if (state)
base::StringToInt(state, &printer_info->printer_status);

const char* drv_info =
cupsGetOption(kDriverNameTagName, printer.num_options, printer.options);
const char* drv_info = cupsGetOption(kCUPSOptPrinterMakeAndModel,
printer.num_options, printer.options);
if (drv_info)
printer_info->options[kDriverInfoTagName] = *drv_info;

Expand Down

0 comments on commit 920bb45

Please sign in to comment.