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,