0

Reduce casting for printing ColorModel APIs

Cleanup born from move of printing::ColorModel enum into mojom in
r799398.  Reduce the number of static_cast<int> in the code by updating
the API to accept printing::mojom::ColorModel in as many places as
possible.

Bug: 809738
Change-Id: Ib1d0024a94e21b9bf982174d99e85b04616f1009
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2368082
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802847}
This commit is contained in:
Alan Screen
2020-08-28 23:18:58 +00:00
committed by Commit Bot
parent 59afe0e256
commit 558045a938
11 changed files with 81 additions and 92 deletions

@@ -47,7 +47,8 @@ constexpr int kMinimumPdfSize = 50;
// Converts a color mode to its Mojo type. // Converts a color mode to its Mojo type.
mojom::PrintColorMode ToArcColorMode(int color_mode) { 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 return is_color.value() ? mojom::PrintColorMode::COLOR
: mojom::PrintColorMode::MONOCHROME; : mojom::PrintColorMode::MONOCHROME;
} }

@@ -232,7 +232,7 @@ bool CheckSettingsAndCapabilitiesCompatibility(
return false; return false;
base::Optional<bool> is_color = 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(); bool color_mode_selected = is_color.has_value() && is_color.value();
if (!color_mode_selected && if (!color_mode_selected &&
capabilities.bw_model == capabilities.bw_model ==

@@ -18,8 +18,7 @@ namespace {
proto::PrintSettings_ColorMode ColorModelToProto( proto::PrintSettings_ColorMode ColorModelToProto(
::printing::mojom::ColorModel color) { ::printing::mojom::ColorModel color) {
base::Optional<bool> is_color = base::Optional<bool> is_color = ::printing::IsColorModelSelected(color);
::printing::IsColorModelSelected(static_cast<int>(color));
return is_color.value() ? proto::PrintSettings_ColorMode_COLOR return is_color.value() ? proto::PrintSettings_ColorMode_COLOR
: proto::PrintSettings_ColorMode_BLACK_AND_WHITE; : proto::PrintSettings_ColorMode_BLACK_AND_WHITE;
} }

@@ -104,12 +104,12 @@ void ReportPrintSettingsStats(const base::Value& print_settings,
base::Optional<int> color_mode_opt = print_settings.FindIntKey(kSettingColor); base::Optional<int> color_mode_opt = print_settings.FindIntKey(kSettingColor);
if (color_mode_opt.has_value()) { if (color_mode_opt.has_value()) {
mojom::ColorModel color_model =
ColorModeToColorModel(color_mode_opt.value());
bool unknown_color_model = bool unknown_color_model =
color_mode_opt.value() == color_model == mojom::ColorModel::kUnknownColorModel;
static_cast<int>(mojom::ColorModel::kUnknownColorModel);
if (!unknown_color_model) { if (!unknown_color_model) {
base::Optional<bool> is_color = base::Optional<bool> is_color = IsColorModelSelected(color_model);
IsColorModelSelected(color_mode_opt.value());
ReportPrintSettingHistogram(is_color.value() ReportPrintSettingHistogram(is_color.value()
? PrintSettingsBuckets::kColor ? PrintSettingsBuckets::kColor
: PrintSettingsBuckets::kBlackAndWhite); : PrintSettingsBuckets::kBlackAndWhite);

@@ -25,11 +25,10 @@ bool PapersEqual(const PrinterSemanticCapsAndDefaults::Paper& lhs,
} }
void VerifyCapabilityColorModels(const PrinterSemanticCapsAndDefaults& caps) { void VerifyCapabilityColorModels(const PrinterSemanticCapsAndDefaults& caps) {
base::Optional<bool> maybe_color = base::Optional<bool> maybe_color = IsColorModelSelected(caps.color_model);
IsColorModelSelected(static_cast<int>(caps.color_model));
ASSERT_TRUE(maybe_color.has_value()); ASSERT_TRUE(maybe_color.has_value());
EXPECT_TRUE(maybe_color.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()); ASSERT_TRUE(maybe_color.has_value());
EXPECT_FALSE(maybe_color.value()); EXPECT_FALSE(maybe_color.value());
} }

@@ -19,13 +19,6 @@ namespace {
base::LazyInstance<std::string>::Leaky g_user_agent; 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 } // namespace
void SetAgent(const std::string& user_agent) { void SetAgent(const std::string& user_agent) {
@@ -36,10 +29,17 @@ const std::string& GetAgent() {
return g_user_agent.Get(); 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) #if defined(USE_CUPS)
void GetColorModelForMode(int color_mode, void GetColorModelForModel(mojom::ColorModel color_model,
std::string* color_setting_name, std::string* color_setting_name,
std::string* color_value) { std::string* color_value) {
#if defined(OS_MAC) #if defined(OS_MAC)
constexpr char kCUPSColorMode[] = "ColorMode"; constexpr char kCUPSColorMode[] = "ColorMode";
constexpr char kCUPSColorModel[] = "ColorModel"; constexpr char kCUPSColorModel[] = "ColorModel";
@@ -64,14 +64,7 @@ void GetColorModelForMode(int color_mode,
*color_setting_name = kCUPSColorModel; *color_setting_name = kCUPSColorModel;
base::Optional<mojom::ColorModel> color_model = switch (color_model) {
ColorModeToColorModel(color_mode);
if (!color_model.has_value()) {
NOTREACHED();
return;
}
switch (color_model.value()) {
case mojom::ColorModel::kUnknownColorModel: case mojom::ColorModel::kUnknownColorModel:
*color_value = kGrayscale; *color_value = kGrayscale;
break; break;
@@ -190,12 +183,12 @@ void GetColorModelForMode(int color_mode,
} }
#if defined(OS_MAC) || defined(OS_CHROMEOS) #if defined(OS_MAC) || defined(OS_CHROMEOS)
std::string GetIppColorModelForMode(int color_mode) { std::string GetIppColorModelForModel(mojom::ColorModel color_model) {
// Accept |UNKNOWN_COLOR_MODEL| for consistency with GetColorModelForMode(). // Accept |kUnknownColorModel| for consistency with GetColorModelForModel().
if (color_mode == static_cast<int>(mojom::ColorModel::kUnknownColorModel)) if (color_model == mojom::ColorModel::kUnknownColorModel)
return CUPS_PRINT_COLOR_MODE_MONOCHROME; 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()) { if (!is_color.has_value()) {
NOTREACHED(); NOTREACHED();
return std::string(); return std::string();
@@ -207,15 +200,8 @@ std::string GetIppColorModelForMode(int color_mode) {
#endif // defined(OS_MAC) || defined(OS_CHROMEOS) #endif // defined(OS_MAC) || defined(OS_CHROMEOS)
#endif // defined(USE_CUPS) #endif // defined(USE_CUPS)
base::Optional<bool> IsColorModelSelected(int color_mode) { base::Optional<bool> IsColorModelSelected(mojom::ColorModel color_model) {
base::Optional<mojom::ColorModel> color_model = switch (color_model) {
ColorModeToColorModel(color_mode);
if (!color_model.has_value()) {
NOTREACHED();
return base::nullopt;
}
switch (color_model.value()) {
case mojom::ColorModel::kColor: case mojom::ColorModel::kColor:
case mojom::ColorModel::kCMYK: case mojom::ColorModel::kCMYK:
case mojom::ColorModel::kCMY: case mojom::ColorModel::kCMY:

@@ -27,19 +27,26 @@
namespace printing { namespace printing {
// Returns true if |color_mode| is color and not B&W. Must be called with a // Convert from |color_mode| into a |color_model|. An invalid |color_mode|
// |color_mode| from printing::ColorModel, excluding UNKNOWN_COLOR_MODEL. // will give a result of |mojom::ColorModel::kUnknownColorModel|.
PRINTING_EXPORT base::Optional<bool> IsColorModelSelected(int color_mode); 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) #if defined(USE_CUPS)
// Get the color model setting name and value for the |color_mode|. // Get the color model setting name and value for the |color_model|.
PRINTING_EXPORT void GetColorModelForMode(int color_mode, PRINTING_EXPORT void GetColorModelForModel(mojom::ColorModel color_model,
std::string* color_setting_name, std::string* color_setting_name,
std::string* color_value); std::string* color_value);
#if defined(OS_MAC) || defined(OS_CHROMEOS) #if defined(OS_MAC) || defined(OS_CHROMEOS)
// Convert from |color_mode| to a print-color-mode value from PWG 5100.13. // Convert from |color_model| to a print-color-mode value from PWG 5100.13.
PRINTING_EXPORT std::string GetIppColorModelForMode(int color_mode); PRINTING_EXPORT std::string GetIppColorModelForModel(
mojom::ColorModel color_model);
#endif #endif
#endif // defined(USE_CUPS) #endif // defined(USE_CUPS)

@@ -11,29 +11,43 @@
namespace printing { 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) { TEST(PrintSettingsTest, IsColorModelSelected) {
for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel) + 1; for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel) + 1;
model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) {
EXPECT_TRUE(IsColorModelSelected(IsColorModelSelected(model).has_value())); EXPECT_TRUE(IsColorModelSelected(static_cast<mojom::ColorModel>(model))
.has_value());
}
} }
TEST(PrintSettingsDeathTest, IsColorModelSelectedEdges) { TEST(PrintSettingsDeathTest, IsColorModelSelectedEdges) {
::testing::FLAGS_gtest_death_test_style = "threadsafe"; ::testing::FLAGS_gtest_death_test_style = "threadsafe";
EXPECT_DCHECK_DEATH(IsColorModelSelected( EXPECT_DCHECK_DEATH(
static_cast<int>(mojom::ColorModel::kUnknownColorModel))); IsColorModelSelected(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));
} }
#if defined(USE_CUPS) #if defined(USE_CUPS)
TEST(PrintSettingsTest, GetColorModelForMode) { TEST(PrintSettingsTest, GetColorModelForModel) {
std::string color_setting_name; std::string color_setting_name;
std::string color_value; std::string color_value;
for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel); for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel);
model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) { 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_setting_name.empty());
EXPECT_FALSE(color_value.empty()); EXPECT_FALSE(color_value.empty());
color_setting_name.clear(); 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) #if defined(OS_MAC) || defined(OS_CHROMEOS)
TEST(PrintSettingsTest, GetIppColorModelForMode) { TEST(PrintSettingsTest, GetIppColorModelForModel) {
for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel); for (int model = static_cast<int>(mojom::ColorModel::kUnknownColorModel);
model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) model <= static_cast<int>(mojom::ColorModel::kColorModelLast); ++model) {
EXPECT_FALSE(GetIppColorModelForMode(model).empty()); EXPECT_FALSE(GetIppColorModelForModel(static_cast<mojom::ColorModel>(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));
} }
#endif // defined(OS_MAC) || defined(OS_CHROMEOS) #endif // defined(OS_MAC) || defined(OS_CHROMEOS)
#endif // defined(USE_CUPS) #endif // defined(USE_CUPS)

@@ -122,9 +122,9 @@ std::vector<ScopedCupsOption> SettingsToCupsOptions(
} }
std::vector<ScopedCupsOption> options; std::vector<ScopedCupsOption> options;
options.push_back(ConstructOption( options.push_back(
kIppColor, ConstructOption(kIppColor,
GetIppColorModelForMode(static_cast<int>(settings.color())))); // color GetIppColorModelForModel(settings.color()))); // color
options.push_back(ConstructOption(kIppDuplex, sides)); // duplexing options.push_back(ConstructOption(kIppDuplex, sides)); // duplexing
options.push_back( options.push_back(
ConstructOption(kIppMedia, ConstructOption(kIppMedia,

@@ -391,11 +391,12 @@ bool PrintingContextMac::SetOutputColor(int color_mode) {
static_cast<PMPrintSettings>([print_info_.get() PMPrintSettings]); static_cast<PMPrintSettings>([print_info_.get() PMPrintSettings]);
std::string color_setting_name; std::string color_setting_name;
std::string color_value; std::string color_value;
mojom::ColorModel color_model = ColorModeToColorModel(color_mode);
if (base::FeatureList::IsEnabled(features::kCupsIppPrintingBackend)) { if (base::FeatureList::IsEnabled(features::kCupsIppPrintingBackend)) {
color_setting_name = CUPS_PRINT_COLOR_MODE; color_setting_name = CUPS_PRINT_COLOR_MODE;
color_value = GetIppColorModelForMode(color_mode); color_value = GetIppColorModelForModel(color_model);
} else { } else {
GetColorModelForMode(color_mode, &color_setting_name, &color_value); GetColorModelForModel(color_model, &color_setting_name, &color_value);
} }
base::ScopedCFTypeRef<CFStringRef> color_setting( base::ScopedCFTypeRef<CFStringRef> color_setting(
base::SysUTF8ToCFStringRef(color_setting_name)); base::SysUTF8ToCFStringRef(color_setting_name));

@@ -290,8 +290,8 @@ void PrintDialogGtk::UpdateSettings(
std::string color_value; std::string color_value;
std::string color_setting_name; std::string color_setting_name;
printing::GetColorModelForMode(static_cast<int>(settings->color()), printing::GetColorModelForModel(settings->color(), &color_setting_name,
&color_setting_name, &color_value); &color_value);
gtk_print_settings_set(gtk_settings_, color_setting_name.c_str(), gtk_print_settings_set(gtk_settings_, color_setting_name.c_str(),
color_value.c_str()); color_value.c_str());