Remove out-parameter from CPDF_DefaultAppearance::GetFont()
- Make GetFont() return a struct instead.
- Also add GetFontSizeOrZero() for use with some existing callers.
- Remove useless code along the way.
Change-Id: Ic355ceddb216c470fd0e373ad3a6b8a36d3efa7d
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/131690
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfdoc/cpdf_bafontmap.cpp b/core/fpdfdoc/cpdf_bafontmap.cpp
index ca385af..b50b6b3 100644
--- a/core/fpdfdoc/cpdf_bafontmap.cpp
+++ b/core/fpdfdoc/cpdf_bafontmap.cpp
@@ -276,9 +276,12 @@
}
CPDF_DefaultAppearance appearance(sDA);
- float font_size;
- std::optional<ByteString> font = appearance.GetFont(&font_size);
- *sAlias = font.value_or(ByteString());
+ auto maybe_font_name_and_size = appearance.GetFont();
+ if (maybe_font_name_and_size.has_value()) {
+ *sAlias = maybe_font_name_and_size.value().name;
+ } else {
+ sAlias->clear();
+ }
RetainPtr<CPDF_Dictionary> pFontDict;
if (RetainPtr<CPDF_Dictionary> pAPDict =
diff --git a/core/fpdfdoc/cpdf_defaultappearance.cpp b/core/fpdfdoc/cpdf_defaultappearance.cpp
index 9c6c1f1..fefe6bb 100644
--- a/core/fpdfdoc/cpdf_defaultappearance.cpp
+++ b/core/fpdfdoc/cpdf_defaultappearance.cpp
@@ -63,21 +63,31 @@
CPDF_DefaultAppearance::~CPDF_DefaultAppearance() = default;
-std::optional<ByteString> CPDF_DefaultAppearance::GetFont(
- float* fFontSize) const {
- *fFontSize = 0.0f;
+std::optional<CPDF_DefaultAppearance::FontNameAndSize>
+CPDF_DefaultAppearance::GetFont() const {
if (da_.IsEmpty()) {
return std::nullopt;
}
CPDF_SimpleParser syntax(da_.AsStringView().unsigned_span());
if (!FindTagParamFromStart(&syntax, "Tf", 2)) {
- return ByteString();
+ return FontNameAndSize();
}
- ByteString name = PDF_NameDecode(syntax.GetWord().Substr(1));
- *fFontSize = StringToFloat(syntax.GetWord());
- return name;
+ // Deliberately using separate statements here to ensure the correct
+ // evaluation order.
+ FontNameAndSize result;
+ result.name = PDF_NameDecode(syntax.GetWord().Substr(1));
+ result.size = StringToFloat(syntax.GetWord());
+ return result;
+}
+
+float CPDF_DefaultAppearance::GetFontSizeOrZero() const {
+ auto maybe_font_name_and_size = GetFont();
+ if (!maybe_font_name_and_size.has_value()) {
+ return 0;
+ }
+ return maybe_font_name_and_size.value().size;
}
std::optional<CFX_Color> CPDF_DefaultAppearance::GetColor() const {
diff --git a/core/fpdfdoc/cpdf_defaultappearance.h b/core/fpdfdoc/cpdf_defaultappearance.h
index 147fab2..129b75f 100644
--- a/core/fpdfdoc/cpdf_defaultappearance.h
+++ b/core/fpdfdoc/cpdf_defaultappearance.h
@@ -16,12 +16,18 @@
class CPDF_DefaultAppearance {
public:
+ struct FontNameAndSize {
+ ByteString name;
+ float size = 0; // Defaults to 0 if not found.
+ };
+
explicit CPDF_DefaultAppearance(const ByteString& csDA);
CPDF_DefaultAppearance(const CPDF_DefaultAppearance&) = delete;
CPDF_DefaultAppearance& operator=(const CPDF_DefaultAppearance&) = delete;
~CPDF_DefaultAppearance();
- std::optional<ByteString> GetFont(float* fFontSize) const;
+ std::optional<FontNameAndSize> GetFont() const;
+ float GetFontSizeOrZero() const;
std::optional<CFX_Color> GetColor() const;
std::optional<CFX_Color::TypeAndARGB> GetColorARGB() const;
diff --git a/core/fpdfdoc/cpdf_formcontrol.cpp b/core/fpdfdoc/cpdf_formcontrol.cpp
index fb874ec..2f51e68 100644
--- a/core/fpdfdoc/cpdf_formcontrol.cpp
+++ b/core/fpdfdoc/cpdf_formcontrol.cpp
@@ -215,20 +215,21 @@
}
RetainPtr<CPDF_Font> CPDF_FormControl::GetDefaultControlFont() const {
- float fFontSize;
- CPDF_DefaultAppearance cDA = GetDefaultAppearance();
- std::optional<ByteString> csFontNameTag = cDA.GetFont(&fFontSize);
- if (!csFontNameTag.has_value() || csFontNameTag->IsEmpty()) {
+ CPDF_DefaultAppearance default_appearance = GetDefaultAppearance();
+ auto maybe_font_name_and_size = default_appearance.GetFont();
+ if (!maybe_font_name_and_size.has_value() ||
+ maybe_font_name_and_size.value().name.IsEmpty()) {
return nullptr;
}
+ const ByteString& font_name = maybe_font_name_and_size.value().name;
RetainPtr<CPDF_Dictionary> pDRDict = ToDictionary(
CPDF_FormField::GetMutableFieldAttrForDict(widget_dict_.Get(), "DR"));
if (pDRDict) {
RetainPtr<CPDF_Dictionary> pFonts = pDRDict->GetMutableDictFor("Font");
if (ValidateFontResourceDict(pFonts.Get())) {
RetainPtr<CPDF_Dictionary> pElement =
- pFonts->GetMutableDictFor(csFontNameTag.value());
+ pFonts->GetMutableDictFor(font_name);
if (pElement) {
RetainPtr<CPDF_Font> pFont =
form_->GetFontForElement(std::move(pElement));
@@ -238,7 +239,7 @@
}
}
}
- RetainPtr<CPDF_Font> pFormFont = form_->GetFormFont(csFontNameTag.value());
+ RetainPtr<CPDF_Font> pFormFont = form_->GetFormFont(font_name);
if (pFormFont) {
return pFormFont;
}
@@ -255,8 +256,7 @@
return nullptr;
}
- RetainPtr<CPDF_Dictionary> pElement =
- pFonts->GetMutableDictFor(csFontNameTag.value());
+ RetainPtr<CPDF_Dictionary> pElement = pFonts->GetMutableDictFor(font_name);
if (!pElement) {
return nullptr;
}
diff --git a/core/fpdfdoc/cpdf_generateap.cpp b/core/fpdfdoc/cpdf_generateap.cpp
index d190d60..829ebc1 100644
--- a/core/fpdfdoc/cpdf_generateap.cpp
+++ b/core/fpdfdoc/cpdf_generateap.cpp
@@ -268,21 +268,15 @@
std::optional<DefaultAppearanceInfo> GetDefaultAppearanceInfo(
const ByteString& default_appearance_string) {
- if (default_appearance_string.IsEmpty()) {
- return std::nullopt;
- }
-
CPDF_DefaultAppearance appearance(default_appearance_string);
-
- float font_size = 0;
- std::optional<ByteString> font = appearance.GetFont(&font_size);
- if (!font.has_value()) {
+ auto maybe_font_name_and_size = appearance.GetFont();
+ if (!maybe_font_name_and_size.has_value()) {
return std::nullopt;
}
return DefaultAppearanceInfo{
- .font_name = font.value(),
- .font_size = font_size,
+ .font_name = maybe_font_name_and_size.value().name,
+ .font_size = maybe_font_name_and_size.value().size,
.text_color = appearance.GetColor().value_or(CFX_Color())};
}
diff --git a/fpdfsdk/cpdfsdk_appstream.cpp b/fpdfsdk/cpdfsdk_appstream.cpp
index c4363d3..a6b4516 100644
--- a/fpdfsdk/cpdfsdk_appstream.cpp
+++ b/fpdfsdk/cpdfsdk_appstream.cpp
@@ -1204,14 +1204,10 @@
std::optional<CFX_Color> color = da.GetColor();
CFX_Color crText = color.value_or(CFX_Color(CFX_Color::Type::kGray, 0));
- float fFontSize;
- ByteString csNameTag;
- std::optional<ByteString> font = da.GetFont(&fFontSize);
- if (font.has_value()) {
- csNameTag = font.value();
- } else {
- fFontSize = 12.0f;
- }
+ const float font_size =
+ da.GetFont()
+ .value_or(CPDF_DefaultAppearance::FontNameAndSize{.size = 12.0f})
+ .size;
WideString csWCaption;
WideString csNormalCaption;
@@ -1258,7 +1254,7 @@
crRightBottom, nBorderStyle, dsBorder) +
GetPushButtonAppStream(iconFit.GetFittingBounds() ? rcWindow : rcClient,
&font_map, pNormalIcon, iconFit, csNormalCaption,
- crText, fFontSize, nLayout);
+ crText, font_size, nLayout);
Write("N", csAP, ByteString());
if (pNormalIcon) {
@@ -1286,7 +1282,7 @@
crRightBottom, nBorderStyle, dsBorder) +
GetPushButtonAppStream(iconFit.GetFittingBounds() ? rcWindow : rcClient,
&font_map, pRolloverIcon, iconFit,
- csRolloverCaption, crText, fFontSize, nLayout);
+ csRolloverCaption, crText, font_size, nLayout);
Write("R", csAP, ByteString());
if (pRolloverIcon) {
@@ -1323,7 +1319,7 @@
crRightBottom, nBorderStyle, dsBorder) +
GetPushButtonAppStream(iconFit.GetFittingBounds() ? rcWindow : rcClient,
&font_map, pDownIcon, iconFit, csDownCaption,
- crText, fFontSize, nLayout);
+ crText, font_size, nLayout);
Write("D", csAP, ByteString());
if (pDownIcon) {
diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp
index 9c6c823..c232fe6 100644
--- a/fpdfsdk/cpdfsdk_widget.cpp
+++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -503,11 +503,7 @@
}
float CPDFSDK_Widget::GetFontSize() const {
- CPDF_FormControl* pFormCtrl = GetFormControl();
- CPDF_DefaultAppearance pDa = pFormCtrl->GetDefaultAppearance();
- float fFontSize;
- pDa.GetFont(&fFontSize);
- return fFontSize;
+ return GetFormControl()->GetDefaultAppearance().GetFontSizeOrZero();
}
int CPDFSDK_Widget::GetSelectedIndex(int nIndex) const {
diff --git a/fxjs/cjs_field.cpp b/fxjs/cjs_field.cpp
index be5e336..9d8e347 100644
--- a/fxjs/cjs_field.cpp
+++ b/fxjs/cjs_field.cpp
@@ -2174,10 +2174,10 @@
return CJS_Result::Failure(JSMessage::kBadObjectError);
}
- float fFontSize;
- CPDF_DefaultAppearance FieldAppearance = pFormControl->GetDefaultAppearance();
- FieldAppearance.GetFont(&fFontSize);
- return CJS_Result::Success(pRuntime->NewNumber(static_cast<int>(fFontSize)));
+ CPDF_DefaultAppearance field_appearance =
+ pFormControl->GetDefaultAppearance();
+ return CJS_Result::Success(pRuntime->NewNumber(
+ static_cast<int>(field_appearance.GetFontSizeOrZero())));
}
CJS_Result CJS_Field::set_text_size(CJS_Runtime* pRuntime,
