Skip to content

Commit

Permalink
Revert "Remove Epson generic PPD handling"
Browse files Browse the repository at this point in the history
This reverts commit 636b9b4.

Reason for revert: Causing failures on Linux ChromiumOS MSan bots
https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/18309

Original change's description:
> Remove Epson generic PPD handling
> 
> We previously implemented this generic ppd as a fallback for Epson
> printers that failed ppd matching. This led to some unforeseen issues
> detailed in the bug below; as such this special handling is no
> longer needed and this change removes it.
> 
> Bug: chromium:1049850
> Test: tba
> Change-Id: I45c9dce72205f8ed9255d983cac8b1a739fa388a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087959
> Commit-Queue: Luum Habtemariam <luum@chromium.org>
> Reviewed-by: Sean Kau <skau@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#752210}

TBR=skau@chromium.org,luum@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:1049850
Change-Id: Idb01fcafbf7acc9a87eff86361acc29142448f17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2113399
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752351}
  • Loading branch information
nik3daz authored and Commit Bot committed Mar 23, 2020
1 parent 9e431aa commit 4bb8464
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 25 deletions.
6 changes: 6 additions & 0 deletions chrome/browser/chromeos/printing/usb_printer_detector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ namespace chromeos {
namespace {

// Given a usb device, guesses the make and model for a driver lookup.
//
// TODO(https://crbug.com/895037): Possibly go deeper and query the IEEE1284
// fields for make and model if we determine those are more likely to contain
// what we want. Strings currently come from udev.
// TODO(https://crbug.com/895037): When above is added, parse out document
// formats and add to DetectedPrinter
std::string GuessEffectiveMakeAndModel(
const device::mojom::UsbDeviceInfo& device) {
return base::UTF16ToUTF8(GetManufacturerName(device)) + " " +
Expand Down
38 changes: 26 additions & 12 deletions chromeos/printing/epson_driver_matching.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@
namespace chromeos {

bool CanUseEpsonGenericPPD(const PrinterSearchData& sd) {
// Only matches USB printers.
if (sd.discovery_type != PrinterSearchData::PrinterDiscoveryType::kUsb) {
return false;
}

// Needed to check if its an Epson printer.
if (sd.make_and_model.empty()) {
return false;
Expand All @@ -31,15 +26,34 @@ bool CanUseEpsonGenericPPD(const PrinterSearchData& sd) {
return false;
}

// The command set is retrieved from the 'CMD' field of the printer's IEEE
// 1284 Device ID.
for (base::StringPiece format : sd.printer_id.command_set()) {
if (format.starts_with("ESCPR")) {
return true;
switch (sd.discovery_type) {
case PrinterSearchData::PrinterDiscoveryType::kManual:
// For manually discovered printers, supported_document_formats is
// retrieved via an ippGetAttributes query to the printer.
return base::Contains(sd.supported_document_formats,
"application/octet-stream");

case PrinterSearchData::PrinterDiscoveryType::kUsb: {
// For USB printers, the command set is retrieved from the 'CMD' field of
// the printer's IEEE 1284 Device ID.
for (base::StringPiece format : sd.printer_id.command_set()) {
if (format.starts_with("ESCPR")) {
return true;
}
}
return false;
}
}

return false;
case PrinterSearchData::PrinterDiscoveryType::kZeroconf:
// For printers found through mDNS/DNS-SD discovery,
// supported_document_formats is retrieved via the Printer Description TXT
// Record(from the key 'pdl').
return base::Contains(sd.supported_document_formats,
"application/vnd.epson.escpr");

default:
return false;
}
}

} // namespace chromeos
2 changes: 1 addition & 1 deletion chromeos/printing/epson_driver_matching.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace chromeos {
struct PrinterSearchData;

// Implements PPD matching rules obtained from Epson. Returns true when this
// printer can be safely setup using the generic Epson PPD.
// printer can be saftely setup using the generic Epson PPD.
bool CHROMEOS_EXPORT CanUseEpsonGenericPPD(const PrinterSearchData& sd);

} // namespace chromeos
Expand Down
85 changes: 73 additions & 12 deletions chromeos/printing/epson_driver_matching_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,49 @@
namespace chromeos {
namespace {

const char kOctetStream[] = "application/octet-stream";
const char kEscPr[] = "ESCPR1";
const char kEpsonEscpr[] = "application/vnd.epson.escpr";

using PrinterDiscoveryType = PrinterSearchData::PrinterDiscoveryType;

// Returns PrinterSearchData that will match the Epson Generic PPD.
PrinterSearchData GetTestPrinterSearchData() {
PrinterSearchData GetTestPrinterSearchData(
PrinterDiscoveryType type = PrinterDiscoveryType::kManual) {
PrinterSearchData sd;
sd.discovery_type = PrinterDiscoveryType::kUsb;
sd.make_and_model.push_back("epson");
sd.printer_id.set_command_set({kEscPr});

switch (type) {
case PrinterDiscoveryType::kManual:
sd.discovery_type = PrinterDiscoveryType::kManual;
sd.supported_document_formats.push_back(kOctetStream);
break;

case PrinterDiscoveryType::kUsb:
sd.discovery_type = PrinterDiscoveryType::kUsb;
sd.printer_id.set_command_set({kEscPr});
break;

case PrinterDiscoveryType::kZeroconf:
sd.discovery_type = PrinterDiscoveryType::kZeroconf;
sd.supported_document_formats.push_back(kEpsonEscpr);
break;

default:
sd.discovery_type = type;
break;
}

return sd;
}

// Ensuring simple good cases generated above pass.
TEST(EpsonDriverMatchingTest, SimpleSanityTest) {
EXPECT_TRUE(CanUseEpsonGenericPPD(GetTestPrinterSearchData()));
EXPECT_TRUE(CanUseEpsonGenericPPD(
GetTestPrinterSearchData(PrinterDiscoveryType::kManual)));
EXPECT_TRUE(CanUseEpsonGenericPPD(
GetTestPrinterSearchData(PrinterDiscoveryType::kUsb)));
EXPECT_TRUE(CanUseEpsonGenericPPD(
GetTestPrinterSearchData(PrinterDiscoveryType::kZeroconf)));
}

// Always fails printers missing make and model information.
Expand All @@ -38,12 +65,10 @@ TEST(EpsonDriverMatchingTest, EmptyMakeAndModels) {

// Always fails printers with invalid discovery types.
TEST(EpsonDriverMatchingTest, InvalidPrinterDiscoveryType) {
PrinterSearchData sd(GetTestPrinterSearchData());
sd.discovery_type = PrinterDiscoveryType::kUnknown;
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));

sd.discovery_type = PrinterDiscoveryType::kDiscoveryTypeMax;
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));
EXPECT_FALSE(CanUseEpsonGenericPPD(
GetTestPrinterSearchData(PrinterDiscoveryType::kUnknown)));
EXPECT_FALSE(CanUseEpsonGenericPPD(
GetTestPrinterSearchData(PrinterDiscoveryType::kDiscoveryTypeMax)));
}

// Confirms an Epson printer if any make and models have 'epson'.
Expand All @@ -61,9 +86,27 @@ TEST(EpsonDriverMatchingTest, ChecksAllMakeAndModels) {
EXPECT_TRUE(CanUseEpsonGenericPPD(sd));
}

// Simple PrinterDiscoveryType::kManual checks.
TEST(EpsonDriverMatchingTest, ManualDiscovery) {
PrinterSearchData sd(GetTestPrinterSearchData(PrinterDiscoveryType::kManual));
sd.supported_document_formats.clear();

sd.supported_document_formats.push_back("application/");
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));

sd.supported_document_formats.push_back(std::string(kOctetStream) + "afds");
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));

sd.printer_id.set_command_set({kOctetStream});
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));

sd.supported_document_formats.push_back(kOctetStream);
EXPECT_TRUE(CanUseEpsonGenericPPD(sd));
}

// Simple PrinterDiscoveryType::kUsb checks.
TEST(EpsonDriverMatchingTest, UsbDiscovery) {
PrinterSearchData sd(GetTestPrinterSearchData());
PrinterSearchData sd(GetTestPrinterSearchData(PrinterDiscoveryType::kUsb));
std::vector<std::string> command_set;

command_set.push_back("ESC");
Expand All @@ -82,5 +125,23 @@ TEST(EpsonDriverMatchingTest, UsbDiscovery) {
EXPECT_TRUE(CanUseEpsonGenericPPD(sd));
}

TEST(EpsonDriverMatchingTest, ZerconfDiscovery) {
PrinterSearchData sd(
GetTestPrinterSearchData(PrinterDiscoveryType::kZeroconf));
sd.supported_document_formats.clear();

sd.supported_document_formats.push_back("application/");
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));

sd.supported_document_formats.push_back(std::string(kEpsonEscpr) + ":asfd");
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));

sd.printer_id.set_command_set({kEpsonEscpr});
EXPECT_FALSE(CanUseEpsonGenericPPD(sd));

sd.supported_document_formats.push_back(kEpsonEscpr);
EXPECT_TRUE(CanUseEpsonGenericPPD(sd));
}

} // namespace
} // namespace chromeos

0 comments on commit 4bb8464

Please sign in to comment.