Clean up cloud_devices::printer::Media code.

- Use gfx::Size instead of 2 separate ints to represent the size. All
  non-test callers already have a gfx::Size ready to pass in. Adjust
  Media's implementation to plumb gfx::Size down where applicable.
- Use delegating constructors, instead of having similar code in the
  various Media constructors.
- Use range-based for-loops when possible.
- While updating components/cloud_devices/common/BUILD.gn and DEPS,
  also delete some obsolete entries.

Change-Id: I93957579c8338123e2cbdeaeccffe73f19c1b4a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584378
Reviewed-by: Alan Screen <awscreen@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992227}

chrome/browser/extensions/api/printing/printing_api_utils.cc

@@ -196,12 +196,11 @@ std::unique_ptr<printing::PrintSettings> ParsePrintTicket(base::Value ticket) {
     return nullptr;
   cloud_devices::printer::Media media_value = media.value();
   printing::PrintSettings::RequestedMedia requested_media;
-  if (media_value.width_um <= 0 || media_value.height_um <= 0 ||
+  if (media_value.size_um.width() <= 0 || media_value.size_um.height() <= 0 ||
       media_value.vendor_id.empty()) {
     return nullptr;
   }
-  requested_media.size_microns =
-      gfx::Size(media_value.width_um, media_value.height_um);
+  requested_media.size_microns = media_value.size_um;
   requested_media.vendor_id = media_value.vendor_id;
   settings->set_requested_media(requested_media);
 

chrome/browser/ui/webui/print_preview/pdf_printer_handler.cc

@@ -132,8 +132,7 @@ base::Value GetPdfCapabilities(
       MediaType::NA_LEGAL, MediaType::NA_LETTER, MediaType::NA_LEDGER};
   const gfx::Size default_media_size = GetDefaultPdfMediaSizeMicrons();
   cloud_devices::printer::Media default_media(std::string(), std::string(),
-                                              default_media_size.width(),
-                                              default_media_size.height());
+                                              default_media_size);
   if (!default_media.MatchBySize() ||
       !base::Contains(kPdfMedia, default_media.type)) {
     default_media = cloud_devices::printer::Media(
@@ -146,9 +145,8 @@ base::Value GetPdfCapabilities(
                            default_media.type == media_option.type);
   }
   for (const PrinterSemanticCapsAndDefaults::Paper& paper : custom_papers) {
-    cloud_devices::printer::Media media_option(
-        paper.display_name, paper.vendor_id, paper.size_um.width(),
-        paper.size_um.height());
+    cloud_devices::printer::Media media_option(paper.display_name,
+                                               paper.vendor_id, paper.size_um);
     media.AddOption(media_option);
   }
   media.SaveTo(&description);

components/cloud_devices/common/BUILD.gn

@@ -17,9 +17,7 @@ static_library("common") {
   deps = [
     "//base",
     "//build:chromeos_buildflags",
-    "//google_apis",
-    "//net",
-    "//url",
+    "//ui/gfx/geometry",
   ]
 }
 

components/cloud_devices/common/DEPS

@@ -1,4 +1,3 @@
 include_rules = [
-  "+google_apis/gaia",
-  "+net/base",
+  "+ui/gfx/geometry",
 ]

components/cloud_devices/common/printer_description.cc

@@ -24,6 +24,7 @@
 #include "build/chromeos_buildflags.h"
 #include "components/cloud_devices/common/cloud_device_description_consts.h"
 #include "components/cloud_devices/common/description_items_inl.h"
+#include "ui/gfx/geometry/size.h"
 
 namespace cloud_devices {
 
@@ -242,17 +243,16 @@ constexpr size_t kEnumClassPrefixLen = std::size("MediaType::") - 1;
 #define MAP_CLOUD_PRINT_MEDIA_TYPE(type, width, height, unit_um) \
   {                                                              \
     type, &#type[kEnumClassPrefixLen],                           \
-        static_cast<int>(width * unit_um + 0.5),                 \
-        static_cast<int>(height * unit_um + 0.5)                 \
+        gfx::Size(static_cast<int>(width * unit_um + 0.5),       \
+                  static_cast<int>(height * unit_um + 0.5))      \
   }
 
 constexpr struct MediaDefinition {
   MediaType id;
   const char* const json_name;
-  int width_um;
-  int height_um;
+  gfx::Size size_um;
 } kMediaDefinitions[] = {
-    {MediaType::CUSTOM_MEDIA, "CUSTOM", 0, 0},
+    {MediaType::CUSTOM_MEDIA, "CUSTOM", gfx::Size()},
     MAP_CLOUD_PRINT_MEDIA_TYPE(MediaType::NA_INDEX_3X5, 3, 5, kInchToUm),
     MAP_CLOUD_PRINT_MEDIA_TYPE(MediaType::NA_PERSONAL, 3.625f, 6.5f, kInchToUm),
     MAP_CLOUD_PRINT_MEDIA_TYPE(MediaType::NA_MONARCH, 3.875f, 7.5f, kInchToUm),
@@ -437,42 +437,42 @@ constexpr struct MediaDefinition {
     MAP_CLOUD_PRINT_MEDIA_TYPE(MediaType::OM_INVITE, 220, 220, kMmToUm)};
 #undef MAP_CLOUD_PRINT_MEDIA_TYPE
 
-const MediaDefinition& FindMediaByType(MediaType type) {
-  for (size_t i = 0; i < std::size(kMediaDefinitions); ++i) {
-    if (kMediaDefinitions[i].id == type)
-      return kMediaDefinitions[i];
+const gfx::Size& FindMediaSizeByType(MediaType type) {
+  for (const auto& media : kMediaDefinitions) {
+    if (media.id == type)
+      return media.size_um;
   }
   NOTREACHED();
-  return kMediaDefinitions[0];
+  return kMediaDefinitions[0].size_um;
 }
 
-const MediaDefinition* FindMediaBySize(int32_t width_um, int32_t height_um) {
+const MediaDefinition* FindMediaBySize(const gfx::Size& size_um) {
   const MediaDefinition* result = nullptr;
-  for (size_t i = 0; i < std::size(kMediaDefinitions); ++i) {
+  for (const auto& media : kMediaDefinitions) {
     int32_t diff =
-        std::max(std::abs(width_um - kMediaDefinitions[i].width_um),
-                 std::abs(height_um - kMediaDefinitions[i].height_um));
+        std::max(std::abs(size_um.width() - media.size_um.width()),
+                 std::abs(size_um.height() - media.size_um.height()));
     if (diff < kSizeThresholdUm)
-      result = &kMediaDefinitions[i];
+      result = &media;
   }
   return result;
 }
 
 template <class T, class IdType>
 std::string TypeToString(const T& names, IdType id) {
-  for (size_t i = 0; i < std::size(names); ++i) {
-    if (id == names[i].id)
-      return names[i].json_name;
+  for (const auto& name : names) {
+    if (id == name.id)
+      return name.json_name;
   }
   NOTREACHED();
   return std::string();
 }
 
 template <class T, class IdType>
 bool TypeFromString(const T& names, const std::string& type, IdType* id) {
-  for (size_t i = 0; i < std::size(names); ++i) {
-    if (type == names[i].json_name) {
-      *id = names[i].id;
+  for (const auto& name : names) {
+    if (type == name.json_name) {
+      *id = name.id;
       return true;
     }
   }
@@ -926,37 +926,21 @@ bool Dpi::operator==(const Dpi& other) const {
   return horizontal == other.horizontal && vertical == other.vertical;
 }
 
-Media::Media()
-    : type(MediaType::CUSTOM_MEDIA),
-      width_um(0),
-      height_um(0),
-      is_continuous_feed(false) {}
+Media::Media() : type(MediaType::CUSTOM_MEDIA), is_continuous_feed(false) {}
 
-Media::Media(MediaType type)
-    : type(type),
-      width_um(0),
-      height_um(0),
-      is_continuous_feed(false) {
-  const MediaDefinition& media = FindMediaByType(type);
-  width_um = media.width_um;
-  height_um = media.height_um;
-  is_continuous_feed = width_um <= 0 || height_um <= 0;
-}
+Media::Media(MediaType type) : Media(type, FindMediaSizeByType(type)) {}
 
-Media::Media(MediaType type, int32_t width_um, int32_t height_um)
+Media::Media(MediaType type, const gfx::Size& size_um)
     : type(type),
-      width_um(width_um),
-      height_um(height_um),
-      is_continuous_feed(width_um <= 0 || height_um <= 0) {}
+      size_um(size_um),
+      is_continuous_feed(size_um.width() <= 0 || size_um.height() <= 0) {}
 
 Media::Media(const std::string& custom_display_name,
              const std::string& vendor_id,
-             int32_t width_um,
-             int32_t height_um)
+             const gfx::Size& size_um)
     : type(MediaType::CUSTOM_MEDIA),
-      width_um(width_um),
-      height_um(height_um),
-      is_continuous_feed(width_um <= 0 || height_um <= 0),
+      size_um(size_um),
+      is_continuous_feed(size_um.width() <= 0 || size_um.height() <= 0),
       custom_display_name(custom_display_name),
       vendor_id(vendor_id) {}
 
@@ -965,7 +949,7 @@ Media::Media(const Media& other) = default;
 Media& Media::operator=(const Media& other) = default;
 
 bool Media::MatchBySize() {
-  const MediaDefinition* media = FindMediaBySize(width_um, height_um);
+  const MediaDefinition* media = FindMediaBySize(size_um);
   if (!media)
     return false;
   type = media->id;
@@ -974,18 +958,17 @@ bool Media::MatchBySize() {
 
 bool Media::IsValid() const {
   if (is_continuous_feed) {
-    if (width_um <= 0 && height_um <= 0)
+    if (size_um.width() <= 0 && size_um.height() <= 0)
       return false;
   } else {
-    if (width_um <= 0 || height_um <= 0)
+    if (size_um.width() <= 0 || size_um.height() <= 0)
       return false;
   }
   return true;
 }
 
 bool Media::operator==(const Media& other) const {
-  return type == other.type && width_um == other.width_um &&
-         height_um == other.height_um &&
+  return type == other.type && size_um == other.size_um &&
          is_continuous_feed == other.is_continuous_feed;
 }
 
@@ -1320,8 +1303,8 @@ class FitToPageTraits : public NoValueValidation,
 class PageRangeTraits : public ItemsTraits<kOptionPageRange> {
  public:
   static bool IsValid(const PageRange& option) {
-    for (size_t i = 0; i < option.size(); ++i) {
-      if (option[i].start < 1 || option[i].end < 1) {
+    for (const auto& item : option) {
+      if (item.start < 1 || item.end < 1) {
         return false;
       }
     }
@@ -1345,11 +1328,11 @@ class PageRangeTraits : public ItemsTraits<kOptionPageRange> {
   static void Save(const PageRange& option, base::Value* dict) {
     if (!option.empty()) {
       base::Value list(base::Value::Type::LIST);
-      for (size_t i = 0; i < option.size(); ++i) {
+      for (const auto& item : option) {
         base::Value interval(base::Value::Type::DICTIONARY);
-        interval.SetKey(kPageRangeStart, base::Value(option[i].start));
-        if (option[i].end < kMaxPageNumber)
-          interval.SetKey(kPageRangeEnd, base::Value(option[i].end));
+        interval.SetKey(kPageRangeStart, base::Value(item.start));
+        if (item.end < kMaxPageNumber)
+          interval.SetKey(kPageRangeEnd, base::Value(item.end));
         list.Append(std::move(interval));
       }
       dict->SetKey(kPageRangeInterval, std::move(list));
@@ -1367,10 +1350,10 @@ class MediaTraits : public ItemsTraits<kOptionMediaSize> {
       return false;
     absl::optional<int> width_um = dict.FindIntKey(kMediaWidth);
     if (width_um)
-      option->width_um = width_um.value();
+      option->size_um.set_width(width_um.value());
     absl::optional<int> height_um = dict.FindIntKey(kMediaHeight);
     if (height_um)
-      option->height_um = height_um.value();
+      option->size_um.set_height(height_um.value());
     absl::optional<bool> is_continuous_feed =
         dict.FindBoolKey(kMediaIsContinuous);
     if (is_continuous_feed)
@@ -1397,10 +1380,10 @@ class MediaTraits : public ItemsTraits<kOptionMediaSize> {
     }
     if (!option.vendor_id.empty())
       dict->SetKey(kKeyVendorId, base::Value(option.vendor_id));
-    if (option.width_um > 0)
-      dict->SetKey(kMediaWidth, base::Value(option.width_um));
-    if (option.height_um > 0)
-      dict->SetKey(kMediaHeight, base::Value(option.height_um));
+    if (option.size_um.width() > 0)
+      dict->SetKey(kMediaWidth, base::Value(option.size_um.width()));
+    if (option.size_um.height() > 0)
+      dict->SetKey(kMediaHeight, base::Value(option.size_um.height()));
     if (option.is_continuous_feed)
       dict->SetKey(kMediaIsContinuous, base::Value(true));
   }

components/cloud_devices/common/printer_description.h

@@ -13,6 +13,7 @@
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
 #include "components/cloud_devices/common/description_items.h"
+#include "ui/gfx/geometry/size.h"
 
 // Defines printer options, CDD and CJT items.
 // https://developers.google.com/cloud-print/docs/cdd
@@ -463,12 +464,11 @@ struct Media {
 
   explicit Media(MediaType type);
 
-  Media(MediaType type, int32_t width_um, int32_t height_um);
+  Media(MediaType type, const gfx::Size& size_um);
 
   Media(const std::string& custom_display_name,
         const std::string& vendor_id,
-        int32_t width_um,
-        int32_t height_um);
+        const gfx::Size& size_um);
 
   Media(const Media& other);
   Media& operator=(const Media& other);
@@ -480,8 +480,7 @@ struct Media {
   bool operator!=(const Media& other) const { return !(*this == other); }
 
   MediaType type;
-  int32_t width_um;
-  int32_t height_um;
+  gfx::Size size_um;
   bool is_continuous_feed;
   std::string custom_display_name;
   std::string vendor_id;

components/cloud_devices/common/printer_description_unittest.cc

@@ -718,10 +718,11 @@ TEST(PrinterDescriptionTest, CddSetAll) {
   fit_to_page.AddOption(FitToPageType::SHRINK_TO_PAGE);
   fit_to_page.AddOption(FitToPageType::FILL_PAGE);
 
-  media.AddDefaultOption(Media(MediaType::NA_LETTER, 2222, 3333), true);
-  media.AddOption(Media(MediaType::ISO_A6, 4444, 5555));
-  media.AddOption(Media(MediaType::JPN_YOU4, 6666, 7777));
-  media.AddOption(Media("Feed", "FEED", 1111, 0));
+  media.AddDefaultOption(Media(MediaType::NA_LETTER, gfx::Size(2222, 3333)),
+                         true);
+  media.AddOption(Media(MediaType::ISO_A6, gfx::Size(4444, 5555)));
+  media.AddOption(Media(MediaType::JPN_YOU4, gfx::Size(6666, 7777)));
+  media.AddOption(Media("Feed", "FEED", gfx::Size(1111, 0)));
 
   collate.set_default_value(false);
   reverse.set_default_value(true);
@@ -1210,11 +1211,14 @@ TEST(PrinterDescriptionTest, CddGetAll) {
   EXPECT_TRUE(fit_to_page.Contains(FitToPageType::FILL_PAGE));
   EXPECT_EQ(FitToPageType::NO_FITTING, fit_to_page.GetDefault());
 
-  EXPECT_TRUE(media.Contains(Media(MediaType::NA_LETTER, 2222, 3333)));
-  EXPECT_TRUE(media.Contains(Media(MediaType::ISO_A6, 4444, 5555)));
-  EXPECT_TRUE(media.Contains(Media(MediaType::JPN_YOU4, 6666, 7777)));
-  EXPECT_TRUE(media.Contains(Media("Feed", "FEED", 1111, 0)));
-  EXPECT_EQ(Media(MediaType::NA_LETTER, 2222, 3333), media.GetDefault());
+  EXPECT_TRUE(
+      media.Contains(Media(MediaType::NA_LETTER, gfx::Size(2222, 3333))));
+  EXPECT_TRUE(media.Contains(Media(MediaType::ISO_A6, gfx::Size(4444, 5555))));
+  EXPECT_TRUE(
+      media.Contains(Media(MediaType::JPN_YOU4, gfx::Size(6666, 7777))));
+  EXPECT_TRUE(media.Contains(Media("Feed", "FEED", gfx::Size(1111, 0))));
+  EXPECT_EQ(Media(MediaType::NA_LETTER, gfx::Size(2222, 3333)),
+            media.GetDefault());
 
   EXPECT_FALSE(collate.default_value());
   EXPECT_TRUE(reverse.default_value());
@@ -1291,7 +1295,7 @@ TEST(PrinterDescriptionTest, CjtSetAll) {
   page_ranges.push_back(Interval(1, 99));
   page_ranges.push_back(Interval(150));
   page_range.set_value(page_ranges);
-  media.set_value(Media(MediaType::ISO_C7C6, 4261, 334));
+  media.set_value(Media(MediaType::ISO_C7C6, gfx::Size(4261, 334)));
   collate.set_value(false);
   reverse.set_value(true);
 
@@ -1357,7 +1361,7 @@ TEST(PrinterDescriptionTest, CjtGetAll) {
   page_ranges.push_back(Interval(1, 99));
   page_ranges.push_back(Interval(150));
   EXPECT_EQ(page_range.value(), page_ranges);
-  EXPECT_EQ(media.value(), Media(MediaType::ISO_C7C6, 4261, 334));
+  EXPECT_EQ(media.value(), Media(MediaType::ISO_C7C6, gfx::Size(4261, 334)));
   EXPECT_FALSE(collate.value());
   EXPECT_TRUE(reverse.value());
 

components/printing/common/cloud_print_cdd_conversion.cc

@@ -63,8 +63,7 @@ printer::Media ConvertPaperToMedia(
   gfx::Size paper_size = paper.size_um;
   if (paper_size.width() > paper_size.height())
     paper_size.SetSize(paper_size.height(), paper_size.width());
-  printer::Media new_media(paper.display_name, paper.vendor_id,
-                           paper_size.width(), paper_size.height());
+  printer::Media new_media(paper.display_name, paper.vendor_id, paper_size);
   new_media.MatchBySize();
   return new_media;
 }
@@ -76,8 +75,7 @@ printer::MediaCapability GetMediaCapabilities(
 
   printer::Media default_media(semantic_info.default_paper.display_name,
                                semantic_info.default_paper.vendor_id,
-                               semantic_info.default_paper.size_um.width(),
-                               semantic_info.default_paper.size_um.height());
+                               semantic_info.default_paper.size_um);
   default_media.MatchBySize();
 
   for (const auto& paper : semantic_info.papers) {