Use Optional<> return from CXFA_LocaleMgr::GetConfigLocaleName().
Its only caller returns an optional, too.
Change-Id: I700f15b69c7eee75d73a1a7c271e5482951e224e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/74110
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/xfa/fxfa/parser/cxfa_localemgr.cpp b/xfa/fxfa/parser/cxfa_localemgr.cpp
index e1b7b5c..b5da74e 100644
--- a/xfa/fxfa/parser/cxfa_localemgr.cpp
+++ b/xfa/fxfa/parser/cxfa_localemgr.cpp
@@ -1231,32 +1231,40 @@
m_pDefLocale = pLocale;
}
-WideString CXFA_LocaleMgr::GetConfigLocaleName(CXFA_Node* pConfig) {
- if (m_hasSetLocaleName)
+Optional<WideString> CXFA_LocaleMgr::GetConfigLocaleName(
+ CXFA_Node* pConfig) const {
+ if (m_bConfigLocaleCached)
return m_wsConfigLocale;
- m_hasSetLocaleName = true;
- m_wsConfigLocale.clear();
+ ASSERT(!m_wsConfigLocale.has_value());
+ m_bConfigLocaleCached = true;
if (!pConfig)
return m_wsConfigLocale;
- CXFA_Node* pChildfConfig =
+ CXFA_Node* pChildConfig =
pConfig->GetFirstChildByClass<CXFA_Acrobat>(XFA_Element::Acrobat);
- if (!pChildfConfig) {
- pChildfConfig =
+ if (!pChildConfig) {
+ pChildConfig =
pConfig->GetFirstChildByClass<CXFA_Present>(XFA_Element::Present);
+ if (!pChildConfig)
+ return m_wsConfigLocale;
}
- CXFA_Common* pCommon = pChildfConfig
- ? pChildfConfig->GetFirstChildByClass<CXFA_Common>(
- XFA_Element::Common)
- : nullptr;
+
+ CXFA_Common* pCommon =
+ pChildConfig->GetFirstChildByClass<CXFA_Common>(XFA_Element::Common);
+ if (!pCommon)
+ return m_wsConfigLocale;
+
CXFA_Locale* pLocale =
- pCommon ? pCommon->GetFirstChildByClass<CXFA_Locale>(XFA_Element::Locale)
- : nullptr;
- if (pLocale) {
- m_wsConfigLocale = pLocale->JSObject()
- ->TryCData(XFA_Attribute::Value, false)
- .value_or(WideString());
- }
+ pCommon->GetFirstChildByClass<CXFA_Locale>(XFA_Element::Locale);
+ if (!pLocale)
+ return m_wsConfigLocale;
+
+ Optional<WideString> wsMaybeLocale =
+ pLocale->JSObject()->TryCData(XFA_Attribute::Value, false);
+ if (!wsMaybeLocale.has_value() || wsMaybeLocale.value().IsEmpty())
+ return m_wsConfigLocale;
+
+ m_wsConfigLocale = wsMaybeLocale;
return m_wsConfigLocale;
}
diff --git a/xfa/fxfa/parser/cxfa_localemgr.h b/xfa/fxfa/parser/cxfa_localemgr.h
index 8cb6f04..e9fdf44 100644
--- a/xfa/fxfa/parser/cxfa_localemgr.h
+++ b/xfa/fxfa/parser/cxfa_localemgr.h
@@ -13,6 +13,7 @@
#include "core/fxcrt/unowned_ptr.h"
#include "core/fxcrt/widestring.h"
#include "fxjs/gc/heap.h"
+#include "third_party/base/optional.h"
#include "v8/include/cppgc/garbage-collected.h"
#include "v8/include/cppgc/member.h"
#include "xfa/fgas/crt/locale_mgr_iface.h"
@@ -34,7 +35,7 @@
GCedLocaleIface* GetLocaleByName(const WideString& wsLocaleName) override;
void SetDefLocale(GCedLocaleIface* pLocale);
- WideString GetConfigLocaleName(CXFA_Node* pConfig);
+ Optional<WideString> GetConfigLocaleName(CXFA_Node* pConfig) const;
private:
CXFA_LocaleMgr(cppgc::Heap* pHeap,
@@ -49,9 +50,17 @@
std::vector<cppgc::Member<CXFA_XMLLocale>> m_XMLLocaleArray;
cppgc::Member<GCedLocaleIface> m_pDefLocale;
- WideString m_wsConfigLocale;
+ // Note: three possiblities
+ // 1. we might never have tried to determine |m_wsConfigLocale|.
+ // 2. we might have tried but gotten nothing and want to continue
+ // to return nothing without ever trying again.
+ // 3. we might have tried and gotten something.
+ // So |m_bConfigLocaleCached| indicates whether we've already tried,
+ // and |m_wsConfigLocale| is the possibly nothing we got if we tried.
+ mutable Optional<WideString> m_wsConfigLocale;
+ mutable bool m_bConfigLocaleCached = false;
+
uint16_t m_dwDeflcid;
- bool m_hasSetLocaleName = false;
};
#endif // XFA_FXFA_PARSER_CXFA_LOCALEMGR_H_
diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 55dc84a..9b67810 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -1403,39 +1403,39 @@
Optional<WideString> CXFA_Node::GetLocaleName() {
CXFA_Node* pForm = ToNode(GetDocument()->GetXFAObject(XFA_HASHCODE_Form));
if (!pForm)
- return {};
+ return pdfium::nullopt;
CXFA_Subform* pTopSubform =
pForm->GetFirstChildByClass<CXFA_Subform>(XFA_Element::Subform);
if (!pTopSubform)
- return {};
+ return pdfium::nullopt;
+ Optional<WideString> localeName;
CXFA_Node* pLocaleNode = this;
do {
- Optional<WideString> localeName =
+ localeName =
pLocaleNode->JSObject()->TryCData(XFA_Attribute::Locale, false);
- if (localeName)
+ if (localeName.has_value())
return localeName;
pLocaleNode = pLocaleNode->GetParent();
} while (pLocaleNode && pLocaleNode != pTopSubform);
CXFA_Node* pConfig = ToNode(GetDocument()->GetXFAObject(XFA_HASHCODE_Config));
- WideString wsLocaleName =
- GetDocument()->GetLocaleMgr()->GetConfigLocaleName(pConfig);
- if (!wsLocaleName.IsEmpty())
- return wsLocaleName;
+ localeName = GetDocument()->GetLocaleMgr()->GetConfigLocaleName(pConfig);
+ if (localeName.has_value())
+ return localeName;
if (pTopSubform) {
- Optional<WideString> localeName =
+ localeName =
pTopSubform->JSObject()->TryCData(XFA_Attribute::Locale, false);
- if (localeName)
+ if (localeName.has_value())
return localeName;
}
LocaleIface* pLocale = GetDocument()->GetLocaleMgr()->GetDefLocale();
if (!pLocale)
- return {};
+ return pdfium::nullopt;
return pLocale->GetName();
}
