Get rid of non-const ref parameter in CPDFSDK_InterForm::OnFormat(). Return Optional<WideString> instead. Change the call stack to accept Optional<WideString>. Change-Id: I020589fac8a1de4974070268a19b4acad64c4092 Reviewed-on: https://pdfium-review.googlesource.com/c/43601 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_interform.cpp b/fpdfsdk/cpdfsdk_interform.cpp index 549caaf..2cc20f9 100644 --- a/fpdfsdk/cpdfsdk_interform.cpp +++ b/fpdfsdk/cpdfsdk_interform.cpp
@@ -323,14 +323,11 @@ } } -WideString CPDFSDK_InterForm::OnFormat(CPDF_FormField* pFormField, - bool& bFormatted) { - WideString sValue = pFormField->GetValue(); - if (!m_pFormFillEnv->IsJSPlatformPresent()) { - bFormatted = false; - return sValue; - } +Optional<WideString> CPDFSDK_InterForm::OnFormat(CPDF_FormField* pFormField) { + if (!m_pFormFillEnv->IsJSPlatformPresent()) + return {}; + WideString sValue = pFormField->GetValue(); IJS_Runtime* pRuntime = m_pFormFillEnv->GetIJSRuntime(); if (pFormField->GetFieldType() == FormFieldType::kComboBox && pFormField->CountSelectedItems() > 0) { @@ -339,30 +336,25 @@ sValue = pFormField->GetOptionLabel(index); } - bFormatted = false; - CPDF_AAction aAction = pFormField->GetAdditionalAction(); if (aAction.GetDict() && aAction.ActionExist(CPDF_AAction::Format)) { CPDF_Action action = aAction.GetAction(CPDF_AAction::Format); if (action.GetDict()) { WideString script = action.GetJavaScript(); if (!script.IsEmpty()) { - WideString Value = sValue; IJS_Runtime::ScopedEventContext pContext(pRuntime); - pContext->OnField_Format(pFormField, &Value, true); + pContext->OnField_Format(pFormField, &sValue, true); Optional<IJS_Runtime::JS_Error> err = pContext->RunScript(script); - if (!err) { - sValue = std::move(Value); - bFormatted = true; - } + if (!err) + return sValue; } } } - return sValue; + return {}; } void CPDFSDK_InterForm::ResetFieldAppearance(CPDF_FormField* pFormField, - const WideString* sValue, + Optional<WideString> sValue, bool bValueChanged) { for (int i = 0, sz = pFormField->CountControls(); i < sz; i++) { CPDF_FormControl* pFormCtrl = pFormField->GetControl(i); @@ -591,9 +583,7 @@ return; OnCalculate(pField); - bool bFormatted = false; - WideString sValue = OnFormat(pField, bFormatted); - ResetFieldAppearance(pField, bFormatted ? &sValue : nullptr, true); + ResetFieldAppearance(pField, OnFormat(pField), true); UpdateField(pField); } @@ -611,7 +601,7 @@ return; OnCalculate(pField); - ResetFieldAppearance(pField, nullptr, true); + ResetFieldAppearance(pField, pdfium::nullopt, true); UpdateField(pField); }
diff --git a/fpdfsdk/cpdfsdk_interform.h b/fpdfsdk/cpdfsdk_interform.h index 1862fdb..2a34c23 100644 --- a/fpdfsdk/cpdfsdk_interform.h +++ b/fpdfsdk/cpdfsdk_interform.h
@@ -16,6 +16,7 @@ #include "core/fxcrt/unowned_ptr.h" #include "core/fxge/fx_dib.h" #include "fpdfsdk/cpdfsdk_widget.h" +#include "third_party/base/optional.h" class CPDF_Dictionary; class CPDF_FormControl; @@ -66,10 +67,10 @@ bool OnKeyStrokeCommit(CPDF_FormField* pFormField, const WideString& csValue); bool OnValidate(CPDF_FormField* pFormField, const WideString& csValue); void OnCalculate(CPDF_FormField* pFormField); - WideString OnFormat(CPDF_FormField* pFormField, bool& bFormatted); + Optional<WideString> OnFormat(CPDF_FormField* pFormField); void ResetFieldAppearance(CPDF_FormField* pFormField, - const WideString* sValue, + Optional<WideString> sValue, bool bValueChanged); void UpdateField(CPDF_FormField* pFormField);
diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp index c2a4754..fc848d5 100644 --- a/fpdfsdk/cpdfsdk_widget.cpp +++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -585,19 +585,17 @@ switch (GetFieldType()) { case FormFieldType::kTextField: case FormFieldType::kComboBox: { - bool bFormatted = false; - WideString sValue = OnFormat(bFormatted); - ResetAppearance(bFormatted ? &sValue : nullptr, true); + ResetAppearance(OnFormat(), true); break; } default: - ResetAppearance(nullptr, false); + ResetAppearance(pdfium::nullopt, false); break; } } #endif // PDF_ENABLE_XFA -void CPDFSDK_Widget::ResetAppearance(const WideString* sValue, +void CPDFSDK_Widget::ResetAppearance(Optional<WideString> sValue, bool bValueChanged) { SetAppModified(); @@ -632,16 +630,17 @@ m_pAnnot->ClearCachedAP(); } -WideString CPDFSDK_Widget::OnFormat(bool& bFormatted) { +Optional<WideString> CPDFSDK_Widget::OnFormat() { CPDF_FormField* pFormField = GetFormField(); ASSERT(pFormField); - return m_pInterForm->OnFormat(pFormField, bFormatted); + return m_pInterForm->OnFormat(pFormField); } void CPDFSDK_Widget::ResetFieldAppearance(bool bValueChanged) { CPDF_FormField* pFormField = GetFormField(); ASSERT(pFormField); - m_pInterForm->ResetFieldAppearance(pFormField, nullptr, bValueChanged); + m_pInterForm->ResetFieldAppearance(pFormField, pdfium::nullopt, + bValueChanged); } void CPDFSDK_Widget::DrawAppearance(CFX_RenderDevice* pDevice, @@ -700,7 +699,7 @@ CFX_FloatRect CPDFSDK_Widget::GetClientRect() const { CFX_FloatRect rcWindow = GetRotatedRect(); - float fBorderWidth = (float)GetBorderWidth(); + float fBorderWidth = GetBorderWidth(); switch (GetBorderStyle()) { case BorderStyle::BEVELED: case BorderStyle::INSET:
diff --git a/fpdfsdk/cpdfsdk_widget.h b/fpdfsdk/cpdfsdk_widget.h index fb83ceb..e3f9204 100644 --- a/fpdfsdk/cpdfsdk_widget.h +++ b/fpdfsdk/cpdfsdk_widget.h
@@ -89,10 +89,10 @@ #ifdef PDF_ENABLE_XFA void ResetAppearance(bool bValueChanged); #endif // PDF_ENABLE_XFA - void ResetAppearance(const WideString* sValue, bool bValueChanged); + void ResetAppearance(Optional<WideString> sValue, bool bValueChanged); void ResetFieldAppearance(bool bValueChanged); void UpdateField(); - WideString OnFormat(bool& bFormatted); + Optional<WideString> OnFormat(); bool OnAAction(CPDF_AAction::AActionType type, CPDFSDK_FieldAction* data,
diff --git a/fpdfsdk/cpdfsdk_widgethandler.cpp b/fpdfsdk/cpdfsdk_widgethandler.cpp index b67a92d..c63d915 100644 --- a/fpdfsdk/cpdfsdk_widgethandler.cpp +++ b/fpdfsdk/cpdfsdk_widgethandler.cpp
@@ -63,8 +63,7 @@ pInterForm->AddMap(pCtrl, pWidget); CPDF_InterForm* pPDFInterForm = pInterForm->GetInterForm(); if (pPDFInterForm->NeedConstructAP()) - pWidget->ResetAppearance(nullptr, false); - + pWidget->ResetAppearance(pdfium::nullopt, false); return pWidget; } @@ -223,19 +222,18 @@ CPDFSDK_Widget* pWidget = ToCPDFSDKWidget(pAnnot); if (!pWidget->IsAppearanceValid()) - pWidget->ResetAppearance(nullptr, false); + pWidget->ResetAppearance(pdfium::nullopt, false); FormFieldType fieldType = pWidget->GetFieldType(); if (fieldType == FormFieldType::kTextField || fieldType == FormFieldType::kComboBox) { - bool bFormatted = false; CPDFSDK_Annot::ObservedPtr pObserved(pWidget); - WideString sValue = pWidget->OnFormat(bFormatted); + Optional<WideString> sValue = pWidget->OnFormat(); if (!pObserved) return; - if (bFormatted && fieldType == FormFieldType::kComboBox) - pWidget->ResetAppearance(&sValue, false); + if (sValue.has_value() && fieldType == FormFieldType::kComboBox) + pWidget->ResetAppearance(sValue, false); } #ifdef PDF_ENABLE_XFA
diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp index 329224d..cb36de7 100644 --- a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp +++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
@@ -718,13 +718,12 @@ ASSERT(pWidget); CPDFSDK_InterForm* pInterForm = pPageView->GetFormFillEnv()->GetInterForm(); - bool bFormatted = false; - WideString sValue = pInterForm->OnFormat(pWidget->GetFormField(), bFormatted); + Optional<WideString> sValue = pInterForm->OnFormat(pWidget->GetFormField()); if (!(*pAnnot)) return; - if (bFormatted) { - pInterForm->ResetFieldAppearance(pWidget->GetFormField(), &sValue, true); + if (sValue.has_value()) { + pInterForm->ResetFieldAppearance(pWidget->GetFormField(), sValue, true); pInterForm->UpdateField(pWidget->GetFormField()); }
diff --git a/fpdfsdk/pwl/cpwl_appstream.cpp b/fpdfsdk/pwl/cpwl_appstream.cpp index 8ced563..d2ffd2a 100644 --- a/fpdfsdk/pwl/cpwl_appstream.cpp +++ b/fpdfsdk/pwl/cpwl_appstream.cpp
@@ -1580,7 +1580,7 @@ widget_->SetAppState("Off"); } -void CPWL_AppStream::SetAsComboBox(const WideString* sValue) { +void CPWL_AppStream::SetAsComboBox(Optional<WideString> sValue) { CPDF_FormControl* pControl = widget_->GetFormControl(); CPDF_FormField* pField = pControl->GetField(); std::ostringstream sBody; @@ -1614,8 +1614,8 @@ pEdit->Initialize(); - if (sValue) { - pEdit->SetText(*sValue); + if (sValue.has_value()) { + pEdit->SetText(sValue.value()); } else { int32_t nCurSel = pField->GetSelectedIndex(0); if (nCurSel < 0) @@ -1736,7 +1736,7 @@ ""); } -void CPWL_AppStream::SetAsTextField(const WideString* sValue) { +void CPWL_AppStream::SetAsTextField(Optional<WideString> sValue) { CPDF_FormControl* pControl = widget_->GetFormControl(); CPDF_FormField* pField = pControl->GetField(); std::ostringstream sBody; @@ -1775,11 +1775,8 @@ float fFontSize = widget_->GetFontSize(); #ifdef PDF_ENABLE_XFA - WideString sValueTmp; - if (!sValue && widget_->GetMixXFAWidget()) { - sValueTmp = widget_->GetValue(); - sValue = &sValueTmp; - } + if (!sValue.has_value() && widget_->GetMixXFAWidget()) + sValue = widget_->GetValue(); #endif // PDF_ENABLE_XFA if (nMaxLen > 0) { @@ -1791,8 +1788,8 @@ rcClient, nMaxLen); } } else { - if (sValue) - nMaxLen = sValue->GetLength(); + if (sValue.has_value()) + nMaxLen = sValue.value().GetLength(); pEdit->SetLimitChar(nMaxLen); } } @@ -1803,7 +1800,7 @@ pEdit->SetFontSize(fFontSize); pEdit->Initialize(); - pEdit->SetText(sValue ? *sValue : pField->GetValue()); + pEdit->SetText(sValue.value_or(pField->GetValue())); CFX_FloatRect rcContent = pEdit->GetContentRect(); ByteString sEdit =
diff --git a/fpdfsdk/pwl/cpwl_appstream.h b/fpdfsdk/pwl/cpwl_appstream.h index 810e79c..1c3afab 100644 --- a/fpdfsdk/pwl/cpwl_appstream.h +++ b/fpdfsdk/pwl/cpwl_appstream.h
@@ -9,6 +9,7 @@ #include "core/fxcrt/fx_string.h" #include "core/fxcrt/unowned_ptr.h" +#include "third_party/base/optional.h" class CPDFSDK_Widget; class CPDF_Dictionary; @@ -22,9 +23,9 @@ void SetAsPushButton(); void SetAsCheckBox(); void SetAsRadioButton(); - void SetAsComboBox(const WideString* sValue); + void SetAsComboBox(Optional<WideString> sValue); void SetAsListBox(); - void SetAsTextField(const WideString* sValue); + void SetAsTextField(Optional<WideString> sValue); private: void AddImage(const ByteString& sAPType, CPDF_Stream* pImage); @@ -36,8 +37,8 @@ ByteString GetBackgroundAppStream() const; ByteString GetBorderAppStream() const; - UnownedPtr<CPDFSDK_Widget> widget_; - UnownedPtr<CPDF_Dictionary> dict_; + UnownedPtr<CPDFSDK_Widget> const widget_; + UnownedPtr<CPDF_Dictionary> const dict_; }; #endif // FPDFSDK_PWL_CPWL_APPSTREAM_H_
diff --git a/fxjs/cjs_field.cpp b/fxjs/cjs_field.cpp index 9c47e70..ef2a252 100644 --- a/fxjs/cjs_field.cpp +++ b/fxjs/cjs_field.cpp
@@ -54,19 +54,17 @@ if (IsComboBoxOrTextField(pFormField)) { for (auto& pObserved : widgets) { if (pObserved) { - bool bFormatted = false; - WideString sValue = - ToCPDFSDKWidget(pObserved.Get())->OnFormat(bFormatted); + Optional<WideString> sValue = + ToCPDFSDKWidget(pObserved.Get())->OnFormat(); if (pObserved) { // Not redundant, may be clobbered by OnFormat. - ToCPDFSDKWidget(pObserved.Get())->ResetAppearance( - bFormatted ? &sValue : nullptr, false); + ToCPDFSDKWidget(pObserved.Get())->ResetAppearance(sValue, false); } } } } else { for (auto& pObserved : widgets) { if (pObserved) - ToCPDFSDKWidget(pObserved.Get())->ResetAppearance(nullptr, false); + ToCPDFSDKWidget(pObserved.Get())->ResetAppearance({}, false); } } } @@ -110,13 +108,12 @@ FormFieldType fieldType = pWidget->GetFieldType(); if (fieldType == FormFieldType::kComboBox || fieldType == FormFieldType::kTextField) { - bool bFormatted = false; - WideString sValue = pWidget->OnFormat(bFormatted); + Optional<WideString> sValue = pWidget->OnFormat(); if (!observed_widget) return; - pWidget->ResetAppearance(bFormatted ? &sValue : nullptr, false); + pWidget->ResetAppearance(sValue, false); } else { - pWidget->ResetAppearance(nullptr, false); + pWidget->ResetAppearance({}, false); } if (!observed_widget) return;