Read duplex modes exactly.

The previous version just checked whether long edge duplex mode
is available.

Bug: 842063
Change-Id: Ia36246bdb1bedad04a03c592dd4622264413c7d8
Reviewed-on: https://chromium-review.googlesource.com/1113189
Commit-Queue: Vladislav Kuzkokov <vkuzkokov@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589884}

chrome/browser/chromeos/arc/print/print_struct_traits.cc

@@ -20,6 +20,20 @@ arc::mojom::PrintMediaSizePtr ToMediaSize(
                                          size_mil.width(), size_mil.height());
 }
 
+arc::mojom::PrintDuplexMode ToArcDuplexMode(printing::DuplexMode mode) {
+  switch (mode) {
+    case printing::LONG_EDGE:
+      return arc::mojom::PrintDuplexMode::LONG_EDGE;
+    case printing::SHORT_EDGE:
+      return arc::mojom::PrintDuplexMode::SHORT_EDGE;
+    case printing::SIMPLEX:
+      return arc::mojom::PrintDuplexMode::NONE;
+    default:
+      NOTREACHED();
+  }
+  return arc::mojom::PrintDuplexMode::NONE;
+}
+
 }  // namespace
 
 std::string StructTraits<arc::mojom::PrintResolutionDataView, gfx::Size>::id(
@@ -68,31 +82,19 @@ arc::mojom::PrintDuplexMode
 StructTraits<arc::mojom::PrinterCapabilitiesDataView,
              printing::PrinterSemanticCapsAndDefaults>::
     duplex_modes(const printing::PrinterSemanticCapsAndDefaults& caps) {
-  arc::mojom::PrintDuplexMode duplex_modes = arc::mojom::PrintDuplexMode::NONE;
-  if (caps.duplex_capable) {
-    duplex_modes = static_cast<arc::mojom::PrintDuplexMode>(
-        static_cast<uint32_t>(duplex_modes) |
-        static_cast<uint32_t>(arc::mojom::PrintDuplexMode::LONG_EDGE) |
-        static_cast<uint32_t>(arc::mojom::PrintDuplexMode::SHORT_EDGE));
+  uint32_t duplex_modes = 0;
+  for (printing::DuplexMode mode : caps.duplex_modes) {
+    duplex_modes |= static_cast<uint32_t>(ToArcDuplexMode(mode));
   }
-  return duplex_modes;
+  return static_cast<arc::mojom::PrintDuplexMode>(duplex_modes);
 }
 
 arc::mojom::PrintAttributesPtr
 StructTraits<arc::mojom::PrinterCapabilitiesDataView,
              printing::PrinterSemanticCapsAndDefaults>::
     defaults(const printing::PrinterSemanticCapsAndDefaults& caps) {
-  arc::mojom::PrintDuplexMode default_duplex_mode;
-  switch (caps.duplex_default) {
-    case printing::LONG_EDGE:
-      default_duplex_mode = arc::mojom::PrintDuplexMode::LONG_EDGE;
-      break;
-    case printing::SHORT_EDGE:
-      default_duplex_mode = arc::mojom::PrintDuplexMode::SHORT_EDGE;
-      break;
-    default:
-      default_duplex_mode = arc::mojom::PrintDuplexMode::NONE;
-  }
+  arc::mojom::PrintDuplexMode default_duplex_mode =
+      ToArcDuplexMode(caps.duplex_default);
   return arc::mojom::PrintAttributes::New(
       ToMediaSize(caps.default_paper), caps.default_dpi,
       arc::mojom::PrintMargins::New(0, 0, 0, 0),

chrome/common/chrome_utility_printing_messages.h

@@ -37,7 +37,7 @@ IPC_STRUCT_TRAITS_BEGIN(printing::PrinterSemanticCapsAndDefaults)
   IPC_STRUCT_TRAITS_MEMBER(collate_capable)
   IPC_STRUCT_TRAITS_MEMBER(collate_default)
   IPC_STRUCT_TRAITS_MEMBER(copies_capable)
-  IPC_STRUCT_TRAITS_MEMBER(duplex_capable)
+  IPC_STRUCT_TRAITS_MEMBER(duplex_modes)
   IPC_STRUCT_TRAITS_MEMBER(duplex_default)
   IPC_STRUCT_TRAITS_MEMBER(color_changeable)
   IPC_STRUCT_TRAITS_MEMBER(color_default)

components/printing/common/cloud_print_cdd_conversion.cc

@@ -15,6 +15,25 @@
 
 namespace cloud_print {
 
+namespace {
+
+cloud_devices::printer::DuplexType ToCloudDuplexType(
+    printing::DuplexMode mode) {
+  switch (mode) {
+    case printing::SIMPLEX:
+      return cloud_devices::printer::NO_DUPLEX;
+    case printing::LONG_EDGE:
+      return cloud_devices::printer::LONG_EDGE;
+    case printing::SHORT_EDGE:
+      return cloud_devices::printer::SHORT_EDGE;
+    default:
+      NOTREACHED();
+  }
+  return cloud_devices::printer::NO_DUPLEX;
+}
+
+}  // namespace
+
 std::unique_ptr<base::DictionaryValue> PrinterSemanticCapsAndDefaultsToCdd(
     const printing::PrinterSemanticCapsAndDefaults& semantic_info) {
   using namespace cloud_devices::printer;
@@ -35,14 +54,12 @@ std::unique_ptr<base::DictionaryValue> PrinterSemanticCapsAndDefaultsToCdd(
     copies.SaveTo(&description);
   }
 
-  if (semantic_info.duplex_capable) {
+  if (semantic_info.duplex_modes.size() > 1) {
     DuplexCapability duplex;
-    duplex.AddDefaultOption(NO_DUPLEX,
-                            semantic_info.duplex_default == printing::SIMPLEX);
-    duplex.AddDefaultOption(
-        LONG_EDGE, semantic_info.duplex_default == printing::LONG_EDGE);
-    duplex.AddDefaultOption(
-        SHORT_EDGE, semantic_info.duplex_default == printing::SHORT_EDGE);
+    for (printing::DuplexMode mode : semantic_info.duplex_modes) {
+      duplex.AddDefaultOption(ToCloudDuplexType(mode),
+                              semantic_info.duplex_default == mode);
+    }
     duplex.SaveTo(&description);
   }
 

printing/BUILD.gn

@@ -283,6 +283,7 @@ test("printing_unittests") {
     "//base/test:run_all_unittests",
     "//base/test:test_support",
     "//printing/common",
+    "//testing/gmock",
     "//testing/gtest",
     "//ui/base",
     "//ui/gfx",

printing/backend/cups_helper.cc

@@ -40,6 +40,7 @@ const char kHighGray[] = "High.Gray";
 
 constexpr char kDuplex[] = "Duplex";
 constexpr char kDuplexNone[] = "None";
+constexpr char kDuplexNoTumble[] = "DuplexNoTumble";
 constexpr char kDuplexTumble[] = "DuplexTumble";
 constexpr char kPageSize[] = "PageSize";
 
@@ -132,23 +133,31 @@ void MarkLpOptions(base::StringPiece printer_name, ppd_file_t** ppd) {
 }
 
 void GetDuplexSettings(ppd_file_t* ppd,
-                       bool* duplex_capable,
+                       std::vector<DuplexMode>* duplex_modes,
                        DuplexMode* duplex_default) {
   ppd_choice_t* duplex_choice = ppdFindMarkedChoice(ppd, kDuplex);
-  if (!duplex_choice) {
-    ppd_option_t* option = ppdFindOption(ppd, kDuplex);
-    if (!option)
-      option = ppdFindOption(ppd, kBrotherDuplex);
-    if (!option)
-      return;
+  ppd_option_t* option = ppdFindOption(ppd, kDuplex);
+  if (!option)
+    option = ppdFindOption(ppd, kBrotherDuplex);
 
+  if (!option)
+    return;
+
+  if (!duplex_choice)
     duplex_choice = ppdFindChoice(option, option->defchoice);
-  }
+
+  if (ppdFindChoice(option, kDuplexNone))
+    duplex_modes->push_back(SIMPLEX);
+
+  if (ppdFindChoice(option, kDuplexNoTumble))
+    duplex_modes->push_back(LONG_EDGE);
+
+  if (ppdFindChoice(option, kDuplexTumble))
+    duplex_modes->push_back(SHORT_EDGE);
 
   if (!duplex_choice)
     return;
 
-  *duplex_capable = true;
   const char* choice = duplex_choice->choice;
   if (EqualsCaseInsensitiveASCII(choice, kDuplexNone)) {
     *duplex_default = SIMPLEX;
@@ -452,7 +461,7 @@ bool ParsePpdCapabilities(base::StringPiece printer_name,
   caps.collate_default = true;
   caps.copies_capable = true;
 
-  GetDuplexSettings(ppd, &caps.duplex_capable, &caps.duplex_default);
+  GetDuplexSettings(ppd, &caps.duplex_modes, &caps.duplex_default);
 
   bool is_color = false;
   ColorModel cm_color = UNKNOWN_COLOR_MODEL, cm_black = UNKNOWN_COLOR_MODEL;

printing/backend/cups_helper_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "printing/backend/cups_helper.h"
 #include "printing/backend/print_backend.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 TEST(PrintBackendCupsHelperTest, TestPpdParsingNoColorDuplexShortEdge) {
@@ -35,7 +36,9 @@ TEST(PrintBackendCupsHelperTest, TestPpdParsingNoColorDuplexShortEdge) {
   EXPECT_TRUE(caps.collate_capable);
   EXPECT_TRUE(caps.collate_default);
   EXPECT_TRUE(caps.copies_capable);
-  EXPECT_TRUE(caps.duplex_capable);
+  EXPECT_THAT(caps.duplex_modes, testing::UnorderedElementsAre(
+                                     printing::SIMPLEX, printing::LONG_EDGE,
+                                     printing::SHORT_EDGE));
   EXPECT_EQ(printing::SHORT_EDGE, caps.duplex_default);
   EXPECT_FALSE(caps.color_changeable);
   EXPECT_FALSE(caps.color_default);
@@ -62,7 +65,9 @@ TEST(PrintBackendCupsHelperTest, TestPpdParsingNoColorDuplexSimples) {
   EXPECT_TRUE(caps.collate_capable);
   EXPECT_TRUE(caps.collate_default);
   EXPECT_TRUE(caps.copies_capable);
-  EXPECT_TRUE(caps.duplex_capable);
+  EXPECT_THAT(caps.duplex_modes, testing::UnorderedElementsAre(
+                                     printing::SIMPLEX, printing::LONG_EDGE,
+                                     printing::SHORT_EDGE));
   EXPECT_EQ(printing::SIMPLEX, caps.duplex_default);
   EXPECT_FALSE(caps.color_changeable);
   EXPECT_FALSE(caps.color_default);
@@ -88,7 +93,7 @@ TEST(PrintBackendCupsHelperTest, TestPpdParsingNoColorNoDuplex) {
   EXPECT_TRUE(caps.collate_capable);
   EXPECT_TRUE(caps.collate_default);
   EXPECT_TRUE(caps.copies_capable);
-  EXPECT_FALSE(caps.duplex_capable);
+  EXPECT_THAT(caps.duplex_modes, testing::UnorderedElementsAre());
   EXPECT_EQ(printing::UNKNOWN_DUPLEX_MODE, caps.duplex_default);
   EXPECT_FALSE(caps.color_changeable);
   EXPECT_FALSE(caps.color_default);
@@ -123,7 +128,9 @@ TEST(PrintBackendCupsHelperTest, TestPpdParsingColorTrueDuplexShortEdge) {
   EXPECT_TRUE(caps.collate_capable);
   EXPECT_TRUE(caps.collate_default);
   EXPECT_TRUE(caps.copies_capable);
-  EXPECT_TRUE(caps.duplex_capable);
+  EXPECT_THAT(caps.duplex_modes, testing::UnorderedElementsAre(
+                                     printing::SIMPLEX, printing::LONG_EDGE,
+                                     printing::SHORT_EDGE));
   EXPECT_EQ(printing::SHORT_EDGE, caps.duplex_default);
   EXPECT_TRUE(caps.color_changeable);
   EXPECT_TRUE(caps.color_default);
@@ -162,7 +169,9 @@ TEST(PrintBackendCupsHelperTest, TestPpdParsingColorFalseDuplexLongEdge) {
   EXPECT_TRUE(caps.collate_capable);
   EXPECT_TRUE(caps.collate_default);
   EXPECT_TRUE(caps.copies_capable);
-  EXPECT_TRUE(caps.duplex_capable);
+  EXPECT_THAT(caps.duplex_modes, testing::UnorderedElementsAre(
+                                     printing::SIMPLEX, printing::LONG_EDGE,
+                                     printing::SHORT_EDGE));
   EXPECT_EQ(printing::LONG_EDGE, caps.duplex_default);
   EXPECT_TRUE(caps.color_changeable);
   EXPECT_FALSE(caps.color_default);
@@ -249,7 +258,9 @@ TEST(PrintBackendCupsHelperTest, TestPpdParsingBrotherPrinters) {
 
     printing::PrinterSemanticCapsAndDefaults caps;
     EXPECT_TRUE(printing::ParsePpdCapabilities("test", kTestPpdData, &caps));
-    EXPECT_TRUE(caps.duplex_capable);
+    EXPECT_THAT(caps.duplex_modes, testing::UnorderedElementsAre(
+                                       printing::SIMPLEX, printing::LONG_EDGE,
+                                       printing::SHORT_EDGE));
     EXPECT_EQ(printing::SHORT_EDGE, caps.duplex_default);
   }
 }

printing/backend/cups_ipp_util.cc

@@ -52,11 +52,22 @@ struct ColorMap {
   ColorModel model;
 };
 
+struct DuplexMap {
+  const char* name;
+  DuplexMode mode;
+};
+
 const ColorMap kColorList[]{
     {CUPS_PRINT_COLOR_MODE_COLOR, COLORMODE_COLOR},
     {CUPS_PRINT_COLOR_MODE_MONOCHROME, COLORMODE_MONOCHROME},
 };
 
+const DuplexMap kDuplexList[]{
+    {CUPS_SIDES_ONE_SIDED, SIMPLEX},
+    {CUPS_SIDES_TWO_SIDED_PORTRAIT, LONG_EDGE},
+    {CUPS_SIDES_TWO_SIDED_LANDSCAPE, SHORT_EDGE},
+};
+
 ColorModel ColorModelFromIppColor(base::StringPiece ippColor) {
   for (const ColorMap& color : kColorList) {
     if (ippColor.compare(color.color) == 0) {
@@ -67,29 +78,11 @@ ColorModel ColorModelFromIppColor(base::StringPiece ippColor) {
   return UNKNOWN_COLOR_MODEL;
 }
 
-bool PrinterSupportsValue(const CupsOptionProvider& printer,
-                          const char* name,
-                          const char* value) {
-  std::vector<base::StringPiece> values =
-      printer.GetSupportedOptionValueStrings(name);
-  return base::ContainsValue(values, value);
-}
-
-DuplexMode PrinterDefaultDuplex(const CupsOptionProvider& printer) {
-  ipp_attribute_t* attr = printer.GetDefaultOptionValue(kIppDuplex);
-  if (!attr)
-    return UNKNOWN_DUPLEX_MODE;
-
-  const char* value = ippGetString(attr, 0, nullptr);
-  if (base::EqualsCaseInsensitiveASCII(value, CUPS_SIDES_ONE_SIDED))
-    return SIMPLEX;
-
-  if (base::EqualsCaseInsensitiveASCII(value, CUPS_SIDES_TWO_SIDED_PORTRAIT))
-    return LONG_EDGE;
-
-  if (base::EqualsCaseInsensitiveASCII(value, CUPS_SIDES_TWO_SIDED_LANDSCAPE))
-    return SHORT_EDGE;
-
+DuplexMode DuplexModeFromIpp(base::StringPiece ipp_duplex) {
+  for (const DuplexMap& entry : kDuplexList) {
+    if (base::EqualsCaseInsensitiveASCII(ipp_duplex, entry.name))
+      return entry.mode;
+  }
   return UNKNOWN_DUPLEX_MODE;
 }
 
@@ -192,6 +185,21 @@ void ExtractColor(const CupsOptionProvider& printer,
   printer_info->color_default = DefaultColorModel(printer) == COLORMODE_COLOR;
 }
 
+void ExtractDuplexModes(const CupsOptionProvider& printer,
+                        PrinterSemanticCapsAndDefaults* printer_info) {
+  std::vector<base::StringPiece> duplex_modes =
+      printer.GetSupportedOptionValueStrings(kIppDuplex);
+  for (base::StringPiece duplex : duplex_modes) {
+    DuplexMode duplex_mode = DuplexModeFromIpp(duplex);
+    if (duplex_mode != UNKNOWN_DUPLEX_MODE)
+      printer_info->duplex_modes.push_back(duplex_mode);
+  }
+  ipp_attribute_t* attr = printer.GetDefaultOptionValue(kIppDuplex);
+  printer_info->duplex_default =
+      attr ? DuplexModeFromIpp(ippGetString(attr, 0, nullptr))
+           : UNKNOWN_DUPLEX_MODE;
+}
+
 void ExtractCopies(const CupsOptionProvider& printer,
                    PrinterSemanticCapsAndDefaults* printer_info) {
   // copies
@@ -316,11 +324,6 @@ bool CollateDefault(const CupsOptionProvider& printer) {
 
 void CapsAndDefaultsFromPrinter(const CupsOptionProvider& printer,
                                 PrinterSemanticCapsAndDefaults* printer_info) {
-  // duplex
-  printer_info->duplex_default = PrinterDefaultDuplex(printer);
-  printer_info->duplex_capable =
-      PrinterSupportsValue(printer, kIppDuplex, CUPS_SIDES_TWO_SIDED_PORTRAIT);
-
   // collate
   printer_info->collate_default = CollateDefault(printer);
   printer_info->collate_capable = CollateCapable(printer);
@@ -331,6 +334,7 @@ void CapsAndDefaultsFromPrinter(const CupsOptionProvider& printer,
 
   ExtractCopies(printer, printer_info);
   ExtractColor(printer, printer_info);
+  ExtractDuplexModes(printer, printer_info);
   ExtractResolutions(printer, printer_info);
 }
 

printing/backend/cups_ipp_util_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "printing/backend/cups_ipp_util.h"
 #include "printing/backend/cups_printer.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace printing {
@@ -147,8 +148,9 @@ TEST_F(PrintBackendCupsIppUtilTest, DuplexSupported) {
   PrinterSemanticCapsAndDefaults caps;
   CapsAndDefaultsFromPrinter(*printer_, &caps);
 
-  EXPECT_TRUE(caps.duplex_capable);
-  EXPECT_FALSE(caps.duplex_default);
+  EXPECT_THAT(caps.duplex_modes,
+              testing::UnorderedElementsAre(SIMPLEX, LONG_EDGE));
+  EXPECT_EQ(SIMPLEX, caps.duplex_default);
 }
 
 TEST_F(PrintBackendCupsIppUtilTest, DuplexNotSupported) {
@@ -159,8 +161,8 @@ TEST_F(PrintBackendCupsIppUtilTest, DuplexNotSupported) {
   PrinterSemanticCapsAndDefaults caps;
   CapsAndDefaultsFromPrinter(*printer_, &caps);
 
-  EXPECT_FALSE(caps.duplex_capable);
-  EXPECT_FALSE(caps.duplex_default);
+  EXPECT_THAT(caps.duplex_modes, testing::UnorderedElementsAre(SIMPLEX));
+  EXPECT_EQ(SIMPLEX, caps.duplex_default);
 }
 
 TEST_F(PrintBackendCupsIppUtilTest, A4PaperSupported) {

printing/backend/print_backend.cc

@@ -25,7 +25,6 @@ PrinterSemanticCapsAndDefaults::PrinterSemanticCapsAndDefaults()
     : collate_capable(false),
       collate_default(false),
       copies_capable(false),
-      duplex_capable(false),
       duplex_default(UNKNOWN_DUPLEX_MODE),
       color_changeable(false),
       color_default(false),

printing/backend/print_backend.h

@@ -48,7 +48,7 @@ struct PRINTING_EXPORT PrinterSemanticCapsAndDefaults {
 
   bool copies_capable;
 
-  bool duplex_capable;
+  std::vector<DuplexMode> duplex_modes;
   DuplexMode duplex_default;
 
   bool color_changeable;

printing/backend/print_backend_win.cc

@@ -291,8 +291,11 @@ bool PrintBackendWin::GetPrinterSemanticCapsAndDefaults(
   caps.color_model = printing::COLOR;
   caps.bw_model = printing::GRAY;
 
-  caps.duplex_capable =
-      (DeviceCapabilities(name, port, DC_DUPLEX, nullptr, nullptr) == 1);
+  caps.duplex_modes.push_back(SIMPLEX);
+  if (DeviceCapabilities(name, port, DC_DUPLEX, nullptr, nullptr) == 1) {
+    caps.duplex_modes.push_back(LONG_EDGE);
+    caps.duplex_modes.push_back(SHORT_EDGE);
+  }
 
   caps.collate_capable =
       (DeviceCapabilities(name, port, DC_COLLATE, nullptr, nullptr) == 1);