diff --git a/chrome/browser/chromeos/arc/print_spooler/print_session_impl.cc b/chrome/browser/chromeos/arc/print_spooler/print_session_impl.cc index b941b1401313a..842d98c418301 100644 --- a/chrome/browser/chromeos/arc/print_spooler/print_session_impl.cc +++ b/chrome/browser/chromeos/arc/print_spooler/print_session_impl.cc @@ -47,7 +47,8 @@ constexpr int kMinimumPdfSize = 50; // Converts a color mode to its Mojo type. mojom::PrintColorMode ToArcColorMode(int color_mode) { - base::Optional<bool> is_color = printing::IsColorModelSelected(color_mode); + base::Optional<bool> is_color = printing::IsColorModelSelected( + printing::ColorModeToColorModel(color_mode)); return is_color.value() ? mojom::PrintColorMode::COLOR : mojom::PrintColorMode::MONOCHROME; } diff --git a/chrome/browser/chromeos/extensions/printing/printing_api_utils.cc b/chrome/browser/chromeos/extensions/printing/printing_api_utils.cc index d62318453a265..38441c46227ee 100644 --- a/chrome/browser/chromeos/extensions/printing/printing_api_utils.cc +++ b/chrome/browser/chromeos/extensions/printing/printing_api_utils.cc @@ -232,7 +232,7 @@ bool CheckSettingsAndCapabilitiesCompatibility( return false; base::Optional<bool> is_color = - ::printing::IsColorModelSelected(static_cast<int>(settings.color())); + ::printing::IsColorModelSelected(settings.color()); bool color_mode_selected = is_color.has_value() && is_color.value(); if (!color_mode_selected && capabilities.bw_model == diff --git a/chrome/browser/chromeos/printing/history/print_job_info_proto_conversions.cc b/chrome/browser/chromeos/printing/history/print_job_info_proto_conversions.cc index ce177a46fa3b8..35b33d06222a7 100644 --- a/chrome/browser/chromeos/printing/history/print_job_info_proto_conversions.cc +++ b/chrome/browser/chromeos/printing/history/print_job_info_proto_conversions.cc @@ -18,8 +18,7 @@ namespace { proto::PrintSettings_ColorMode ColorModelToProto( ::printing::mojom::ColorModel color) { - base::Optional<bool> is_color = - ::printing::IsColorModelSelected(static_cast<int>(color)); + base::Optional<bool> is_color = ::printing::IsColorModelSelected(color); return is_color.value() ? proto::PrintSettings_ColorMode_COLOR : proto::PrintSettings_ColorMode_BLACK_AND_WHITE; } diff --git a/chrome/browser/ui/webui/print_preview/print_preview_metrics.cc b/chrome/browser/ui/webui/print_preview/print_preview_metrics.cc index 377c9a41e81a3..d00cef7992ddb 100644 --- a/chrome/browser/ui/webui/print_preview/print_preview_metrics.cc +++ b/chrome/browser/ui/webui/print_preview/print_preview_metrics.cc @@ -104,12 +104,12 @@ void ReportPrintSettingsStats(const base::Value& print_settings, base::Optional<int> color_mode_opt = print_settings.FindIntKey(kSettingColor); if (color_mode_opt.has_value()) { + mojom::ColorModel color_model = + ColorModeToColorModel(color_mode_opt.value()); bool unknown_color_model = - color_mode_opt.value() == - static_cast<int>(mojom::ColorModel::kUnknownColorModel); + color_model == mojom::ColorModel::kUnknownColorModel; if (!unknown_color_model) { - base::Optional<bool> is_color = - IsColorModelSelected(color_mode_opt.value()); + base::Optional<bool> is_color = IsColorModelSelected(color_model); ReportPrintSettingHistogram(is_color.value() ? PrintSettingsBuckets::kColor : PrintSettingsBuckets::kBlackAndWhite); diff --git a/printing/backend/cups_helper_unittest.cc b/printing/backend/cups_helper_unittest.cc index 8adaff3383677..7135afc6d0802 100644 --- a/printing/backend/cups_helper_unittest.cc +++ b/printing/backend/cups_helper_unittest.cc @@ -25,11 +25,10 @@ bool PapersEqual(const PrinterSemanticCapsAndDefaults::Paper& lhs, } void VerifyCapabilityColorModels(const PrinterSemanticCapsAndDefaults& caps) { - base::Optional<bool> maybe_color = - IsColorModelSelected(static_cast<int>(caps.color_model)); + base::Optional<bool> maybe_color = IsColorModelSelected(caps.color_model); ASSERT_TRUE(maybe_color.has_value()); EXPECT_TRUE(maybe_color.value()); - maybe_color = IsColorModelSelected(static_cast<int>(caps.bw_model)); + maybe_color = IsColorModelSelected(caps.bw_model); ASSERT_TRUE(maybe_color.has_value()); EXPECT_FALSE(maybe_color.value()); } diff --git a/printing/print_settings.cc b/printing/print_settings.cc index 2f6016c480ade..a4a9efda11a7c 100644 --- a/printing/print_settings.cc +++ b/printing/print_settings.cc @@ -19,13 +19,6 @@ namespace { base::LazyInstance<std::string>::Leaky g_user_agent; -base::Optional<mojom::ColorModel> ColorModeToColorModel(int color_mode) { - if (color_mode < static_cast<int>(mojom::ColorModel::kUnknownColorModel) || - color_mode > static_cast<int>(mojom::ColorModel::kColorModelLast)) - return base::nullopt; - return static_cast<mojom::ColorModel>(color_mode); -} - } // namespace void SetAgent(const std::string& user_agent) { @@ -36,10 +29,17 @@ const std::string& GetAgent() { return g_user_agent.Get(); } +mojom::ColorModel ColorModeToColorModel(int color_mode) { + if (color_mode < static_cast<int>(mojom::ColorModel::kUnknownColorModel) || + color_mode > static_cast<int>(mojom::ColorModel::kColorModelLast)) + return mojom::ColorModel::kUnknownColorModel; + return static_cast<mojom::ColorModel>(color_mode); +} + #if defined(USE_CUPS) -void GetColorModelForMode(int color_mode, - std::string* color_setting_name, - std::string* color_value) { +void GetColorModelForModel(mojom::ColorModel color_model, + std::string* color_setting_name, + std::string* color_value) { #if defined(OS_MAC) constexpr char kCUPSColorMode[] = "ColorMode"; constexpr char kCUPSColorModel[] = "ColorModel"; @@ -64,14 +64,7 @@ void GetColorModelForMode(int color_mode, *color_setting_name = kCUPSColorModel; - base::Optional<mojom::ColorModel> color_model = - ColorModeToColorModel(color_mode); - if (!color_model.has_value()) { - NOTREACHED(); - return; - } - - switch (color_model.value()) { + switch (color_model) { case mojom::ColorModel::kUnknownColorModel: *color_value = kGrayscale; break; @@ -190,12 +183,12 @@ void GetColorModelForMode(int color_mode, } #if defined(OS_MAC) || defined(OS_CHROMEOS) -std::string GetIppColorModelForMode(int color_mode) { - // Accept |UNKNOWN_COLOR_MODEL| for consistency with GetColorModelForMode(). - if (color_mode == static_cast<int>(mojom::ColorModel::kUnknownColorModel)) +std::string GetIppColorModelForModel(mojom::ColorModel color_model) { + // Accept |kUnknownColorModel| for consistency with GetColorModelForModel(). + if (color_model == mojom::ColorModel::kUnknownColorModel) return CUPS_PRINT_COLOR_MODE_MONOCHROME; - base::Optional<bool> is_color = IsColorModelSelected(color_mode); + base::Optional<bool> is_color = IsColorModelSelected(color_model); if (!is_color.has_value()) { NOTREACHED(); return std::string(); @@ -207,15 +200,8 @@ std::string GetIppColorModelForMode(int color_mode) { #endif // defined(OS_MAC) || defined(OS_CHROMEOS) #endif // defined(USE_CUPS) -base::Optional<bool> IsColorModelSelected(int color_mode) { - base::Optional<mojom::ColorModel> color_model = - ColorModeToColorModel(color_mode); - if (!color_model.has_value()) { - NOTREACHED(); - return base::nullopt; - } - - switch (color_model.value()) { +base::Optional<bool> IsColorModelSelected(mojom::ColorModel color_model) { + switch (color_model) { case mojom::ColorModel::kColor: case mojom::ColorModel::kCMYK: case mojom::ColorModel::kCMY: diff --git a/printing/print_settings.h b/printing/print_settings.h index 6e548b9a1a542..cad8c3a8e8235 100644 --- a/printing/print_settings.h +++ b/printing/print_settings.h @@ -27,19 +27,26 @@ namespace printing { -// Returns true if |color_mode| is color and not B&W. Must be called with a -// |color_mode| from printing::ColorModel, excluding UNKNOWN_COLOR_MODEL. -PRINTING_EXPORT base::Optional<bool> IsColorModelSelected(int color_mode); +// Convert from |color_mode| into a |color_model|. An invalid |color_mode| +// will give a result of |mojom::ColorModel::kUnknownColorModel|. +PRINTING_EXPORT mojom::ColorModel ColorModeToColorModel(int color_mode); + +// Returns true if |color_model| is color and false if it is B&W. Callers +// are not supposed to pass in |mojom::ColorModel::kUnknownColorModel|, but +// if they do then the result will be base::nullopt. +PRINTING_EXPORT base::Optional<bool> IsColorModelSelected( + mojom::ColorModel color_model); #if defined(USE_CUPS) -// Get the color model setting name and value for the |color_mode|. -PRINTING_EXPORT void GetColorModelForMode(int color_mode, - std::string* color_setting_name, - std::string* color_value); +// Get the color model setting name and value for the |color_model|. +PRINTING_EXPORT void GetColorModelForModel(mojom::ColorModel color_model, + std::string* color_setting_name, + std::string* color_value); #if defined(OS_MAC) || defined(OS_CHROMEOS) -// Convert from |color_mode| to a print-color-mode value from PWG 5100.13. -PRINTING_EXPORT std::string GetIppColorModelForMode(int color_mode); +// Convert from |color_model| to a print-color-mode value from PWG 5100.13. +PRINTING_EXPORT std::string GetIppColorModelForModel( + mojom::ColorModel color_model); #endif #endif // defined(USE_CUPS) diff --git a/printing/print_settings_unittest.cc b/printing/print_settings_unittest.cc index 7321f42164739..90168d4a67b12 100644 --- a/printing/print_settings_unittest.cc +++ b/printing/print_settings_unittest.cc @@ -11,29 +11,43 @@ namespace printing { +TEST(PrintSettingsTest, ColorModeToColorModel) { + for (int mode = static_cast<int>(mojom::ColorModel::kUnknownColorModel); + mode <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++mode) { + EXPECT_EQ(ColorModeToColorModel(mode), + static_cast<mojom::ColorModel>(mode)); + } + + // Check edge cases. + EXPECT_EQ(ColorModeToColorModel( + static_cast<int>(mojom::ColorModel::kUnknownColorModel) - 1), + mojom::ColorModel::kUnknownColorModel); + EXPECT_EQ(ColorModeToColorModel( + static_cast<int>(mojom::ColorModel::kColorModelLast) + 1), + mojom::ColorModel::kUnknownColorModel); +} + TEST(PrintSettingsTest, IsColorModelSelected) { for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel) + 1; - model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) - EXPECT_TRUE(IsColorModelSelected(IsColorModelSelected(model).has_value())); + model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) { + EXPECT_TRUE(IsColorModelSelected(static_cast<mojom::ColorModel>(model)) + .has_value()); + } } TEST(PrintSettingsDeathTest, IsColorModelSelectedEdges) { ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - EXPECT_DCHECK_DEATH(IsColorModelSelected( - static_cast<int>(mojom::ColorModel::kUnknownColorModel))); - EXPECT_DCHECK_DEATH(IsColorModelSelected( - static_cast<int>(mojom::ColorModel::kUnknownColorModel) - 1)); - EXPECT_DCHECK_DEATH(IsColorModelSelected( - static_cast<int>(mojom::ColorModel::kColorModelLast) + 1)); + EXPECT_DCHECK_DEATH( + IsColorModelSelected(mojom::ColorModel::kUnknownColorModel)); } - #if defined(USE_CUPS) -TEST(PrintSettingsTest, GetColorModelForMode) { +TEST(PrintSettingsTest, GetColorModelForModel) { std::string color_setting_name; std::string color_value; for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel); model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) { - GetColorModelForMode(model, &color_setting_name, &color_value); + GetColorModelForModel(static_cast<mojom::ColorModel>(model), + &color_setting_name, &color_value); EXPECT_FALSE(color_setting_name.empty()); EXPECT_FALSE(color_value.empty()); color_setting_name.clear(); @@ -41,31 +55,13 @@ TEST(PrintSettingsTest, GetColorModelForMode) { } } -TEST(PrintSettingsDeathTest, GetColorModelForModeEdges) { - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - std::string color_setting_name; - std::string color_value; - EXPECT_DCHECK_DEATH(GetColorModelForMode( - static_cast<int>(mojom::ColorModel::kUnknownColorModel) - 1, - &color_setting_name, &color_value)); - EXPECT_DCHECK_DEATH(GetColorModelForMode( - static_cast<int>(mojom::ColorModel::kColorModelLast) + 1, - &color_setting_name, &color_value)); -} - #if defined(OS_MAC) || defined(OS_CHROMEOS) -TEST(PrintSettingsTest, GetIppColorModelForMode) { +TEST(PrintSettingsTest, GetIppColorModelForModel) { for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel); - model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) - EXPECT_FALSE(GetIppColorModelForMode(model).empty()); -} - -TEST(PrintSettingsDeathTest, GetIppColorModelForModeEdges) { - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - EXPECT_DCHECK_DEATH(GetIppColorModelForMode( - static_cast<int>(mojom::ColorModel::kUnknownColorModel) - 1)); - EXPECT_DCHECK_DEATH(GetIppColorModelForMode( - static_cast<int>(mojom::ColorModel::kColorModelLast) + 1)); + model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) { + EXPECT_FALSE(GetIppColorModelForModel(static_cast<mojom::ColorModel>(model)) + .empty()); + } } #endif // defined(OS_MAC) || defined(OS_CHROMEOS) #endif // defined(USE_CUPS) diff --git a/printing/printing_context_chromeos.cc b/printing/printing_context_chromeos.cc index 9b5a3ab158e10..e6299ee3c5003 100644 --- a/printing/printing_context_chromeos.cc +++ b/printing/printing_context_chromeos.cc @@ -122,9 +122,9 @@ std::vector<ScopedCupsOption> SettingsToCupsOptions( } std::vector<ScopedCupsOption> options; - options.push_back(ConstructOption( - kIppColor, - GetIppColorModelForMode(static_cast<int>(settings.color())))); // color + options.push_back( + ConstructOption(kIppColor, + GetIppColorModelForModel(settings.color()))); // color options.push_back(ConstructOption(kIppDuplex, sides)); // duplexing options.push_back( ConstructOption(kIppMedia, diff --git a/printing/printing_context_mac.mm b/printing/printing_context_mac.mm index 15831e1064dbc..51c8ff31497fd 100644 --- a/printing/printing_context_mac.mm +++ b/printing/printing_context_mac.mm @@ -391,11 +391,12 @@ bool PrintingContextMac::SetOutputColor(int color_mode) { static_cast<PMPrintSettings>([print_info_.get() PMPrintSettings]); std::string color_setting_name; std::string color_value; + mojom::ColorModel color_model = ColorModeToColorModel(color_mode); if (base::FeatureList::IsEnabled(features::kCupsIppPrintingBackend)) { color_setting_name = CUPS_PRINT_COLOR_MODE; - color_value = GetIppColorModelForMode(color_mode); + color_value = GetIppColorModelForModel(color_model); } else { - GetColorModelForMode(color_mode, &color_setting_name, &color_value); + GetColorModelForModel(color_model, &color_setting_name, &color_value); } base::ScopedCFTypeRef<CFStringRef> color_setting( base::SysUTF8ToCFStringRef(color_setting_name)); diff --git a/ui/gtk/printing/print_dialog_gtk.cc b/ui/gtk/printing/print_dialog_gtk.cc index 361f52bb11148..f2ed36e1258f4 100644 --- a/ui/gtk/printing/print_dialog_gtk.cc +++ b/ui/gtk/printing/print_dialog_gtk.cc @@ -290,8 +290,8 @@ void PrintDialogGtk::UpdateSettings( std::string color_value; std::string color_setting_name; - printing::GetColorModelForMode(static_cast<int>(settings->color()), - &color_setting_name, &color_value); + printing::GetColorModelForModel(settings->color(), &color_setting_name, + &color_value); gtk_print_settings_set(gtk_settings_, color_setting_name.c_str(), color_value.c_str());