Remove xfa_basic_data.cpp representation dependence on XFA_SCRIPTATTRIBUTEINFO XFA_SCRIPTATTRIBUTEINFO becomes a means of communicating back a result, so that the underlying table structure can evolve. Rename some structs for clarity now that they are in an anonymous namespace. Tidy a test while we're at it. Change-Id: I70ac8ce80b64569fb7494610ae4c938d37ad293a Reviewed-on: https://pdfium-review.googlesource.com/c/46970 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/cfxjse_engine.cpp b/fxjs/cfxjse_engine.cpp index 38db6a5..ec492cb 100644 --- a/fxjs/cfxjse_engine.cpp +++ b/fxjs/cfxjse_engine.cpp
@@ -164,14 +164,11 @@ pValue->Assign(GetJSValueFromMap(resolveRs.objects.front().Get())); return true; } - if (resolveRs.dwFlags == XFA_ResolveNode_RSType_Attribute) { - const XFA_SCRIPTATTRIBUTEINFO* lpAttributeInfo = - resolveRs.pScriptAttribute.Get(); - if (lpAttributeInfo) { - CJX_Object* jsObject = resolveRs.objects.front()->JSObject(); - (jsObject->*(lpAttributeInfo->callback))(pValue, bSetting, - lpAttributeInfo->attribute); - } + if (resolveRs.dwFlags == XFA_ResolveNode_RSType_Attribute && + resolveRs.script_attribute.callback) { + CJX_Object* jsObject = resolveRs.objects.front()->JSObject(); + (jsObject->*(resolveRs.script_attribute.callback))( + pValue, bSetting, resolveRs.script_attribute.attribute); } return true; } @@ -332,13 +329,12 @@ szPropName, pReturnValue, true); if (!bRet) { - const XFA_SCRIPTATTRIBUTEINFO* lpAttributeInfo = - XFA_GetScriptAttributeByName(pObject->GetElementType(), - wsPropName.AsStringView()); - if (lpAttributeInfo) { + Optional<XFA_SCRIPTATTRIBUTEINFO> info = XFA_GetScriptAttributeByName( + pObject->GetElementType(), wsPropName.AsStringView()); + if (info.has_value()) { CJX_Object* jsObject = pObject->JSObject(); - (jsObject->*(lpAttributeInfo->callback))(pReturnValue, false, - lpAttributeInfo->attribute); + (jsObject->*(info.value().callback))(pReturnValue, false, + info.value().attribute); return; } } @@ -372,12 +368,12 @@ CXFA_Object* pObject = lpScriptContext->GetVariablesThis(pOriginalObject, false); WideString wsPropName = WideString::FromUTF8(szPropName); - const XFA_SCRIPTATTRIBUTEINFO* lpAttributeInfo = XFA_GetScriptAttributeByName( + Optional<XFA_SCRIPTATTRIBUTEINFO> info = XFA_GetScriptAttributeByName( pObject->GetElementType(), wsPropName.AsStringView()); - if (lpAttributeInfo) { + if (info.has_value()) { CJX_Object* jsObject = pObject->JSObject(); - (jsObject->*(lpAttributeInfo->callback))(pReturnValue, true, - lpAttributeInfo->attribute); + (jsObject->*(info.value().callback))(pReturnValue, true, + info.value().attribute); return; } @@ -396,9 +392,9 @@ } if (pPropOrChild) { - const XFA_SCRIPTATTRIBUTEINFO* lpAttrInfo = XFA_GetScriptAttributeByName( - pPropOrChild->GetElementType(), L"{default}"); - if (lpAttrInfo) { + info = XFA_GetScriptAttributeByName(pPropOrChild->GetElementType(), + L"{default}"); + if (info.has_value()) { pPropOrChild->JSObject()->ScriptSomDefaultValue(pReturnValue, true, XFA_Attribute::Unknown); return; @@ -658,13 +654,13 @@ continue; if (rndFind.m_dwFlag == XFA_ResolveNode_RSType_Attribute && - rndFind.m_pScriptAttribute && + rndFind.m_ScriptAttribute.callback && nStart < pdfium::base::checked_cast<int32_t>(wsExpression.GetLength())) { auto pValue = pdfium::MakeUnique<CFXJSE_Value>(GetIsolate()); CJX_Object* jsObject = rndFind.m_Objects.front()->JSObject(); - (jsObject->*(rndFind.m_pScriptAttribute->callback))( - pValue.get(), false, rndFind.m_pScriptAttribute->attribute); + (jsObject->*(rndFind.m_ScriptAttribute.callback))( + pValue.get(), false, rndFind.m_ScriptAttribute.attribute); rndFind.m_Objects.front() = ToObject(pValue.get()); } if (!m_upObjectArray.empty()) @@ -712,7 +708,7 @@ findObjects.begin(), findObjects.end()); } if (rndFind.m_dwFlag == XFA_ResolveNode_RSType_Attribute) { - resolveNodeRS->pScriptAttribute = rndFind.m_pScriptAttribute; + resolveNodeRS->script_attribute = rndFind.m_ScriptAttribute; return 1; } }
diff --git a/fxjs/cfxjse_formcalc_context.cpp b/fxjs/cfxjse_formcalc_context.cpp index 464dd0a..a06e65a 100644 --- a/fxjs/cfxjse_formcalc_context.cpp +++ b/fxjs/cfxjse_formcalc_context.cpp
@@ -5774,13 +5774,13 @@ } *bAttribute = true; - if (resolveNodeRS.pScriptAttribute && - resolveNodeRS.pScriptAttribute->eValueType == XFA_ScriptType::Object) { + if (resolveNodeRS.script_attribute.callback && + resolveNodeRS.script_attribute.eValueType == XFA_ScriptType::Object) { for (auto& pObject : resolveNodeRS.objects) { auto pValue = pdfium::MakeUnique<CFXJSE_Value>(pIsolate); CJX_Object* jsObject = pObject->JSObject(); - (jsObject->*(resolveNodeRS.pScriptAttribute->callback))( - pValue.get(), false, resolveNodeRS.pScriptAttribute->attribute); + (jsObject->*(resolveNodeRS.script_attribute.callback))( + pValue.get(), false, resolveNodeRS.script_attribute.attribute); resultValues->push_back(std::move(pValue)); *bAttribute = false;
diff --git a/fxjs/cfxjse_resolveprocessor.cpp b/fxjs/cfxjse_resolveprocessor.cpp index b13f394..a85df15 100644 --- a/fxjs/cfxjse_resolveprocessor.cpp +++ b/fxjs/cfxjse_resolveprocessor.cpp
@@ -203,12 +203,12 @@ CXFA_Object* curNode, CFXJSE_ResolveNodeData& rnd, const WideStringView& strAttr) { - const XFA_SCRIPTATTRIBUTEINFO* lpScriptAttribute = + Optional<XFA_SCRIPTATTRIBUTEINFO> info = XFA_GetScriptAttributeByName(curNode->GetElementType(), strAttr); - if (!lpScriptAttribute) + if (!info.has_value()) return false; - rnd.m_pScriptAttribute = lpScriptAttribute; + rnd.m_ScriptAttribute = info.value(); rnd.m_Objects.push_back(curNode); rnd.m_dwFlag = XFA_ResolveNode_RSType_Attribute; return true; @@ -738,7 +738,6 @@ m_nLevel(0), m_Objects(), m_dwStyles(XFA_RESOLVENODE_Children), - m_pScriptAttribute(nullptr), m_dwFlag(XFA_ResolveNode_RSType_Nodes) {} CFXJSE_ResolveNodeData::~CFXJSE_ResolveNodeData() {}
diff --git a/fxjs/cfxjse_resolveprocessor.h b/fxjs/cfxjse_resolveprocessor.h index 8ad17d5..6a3cced 100644 --- a/fxjs/cfxjse_resolveprocessor.h +++ b/fxjs/cfxjse_resolveprocessor.h
@@ -31,7 +31,7 @@ int32_t m_nLevel; std::vector<CXFA_Object*> m_Objects; // Not owned. uint32_t m_dwStyles; - const XFA_SCRIPTATTRIBUTEINFO* m_pScriptAttribute; + XFA_SCRIPTATTRIBUTEINFO m_ScriptAttribute; XFA_ResolveNode_RSType m_dwFlag; };
diff --git a/fxjs/xfa/cjx_object.h b/fxjs/xfa/cjx_object.h index 8eaab2e..56548b2 100644 --- a/fxjs/xfa/cjx_object.h +++ b/fxjs/xfa/cjx_object.h
@@ -280,10 +280,9 @@ XFA_Attribute eAttribute); struct XFA_SCRIPTATTRIBUTEINFO { - uint32_t uHash; // Hashed as wide string. XFA_Attribute attribute; XFA_ScriptType eValueType; - XFA_ATTRIBUTE_CALLBACK callback; + XFA_ATTRIBUTE_CALLBACK callback = nullptr; }; #endif // FXJS_XFA_CJX_OBJECT_H_
diff --git a/fxjs/xfa/cjx_tree.cpp b/fxjs/xfa/cjx_tree.cpp index 229444f..c1dc937 100644 --- a/fxjs/xfa/cjx_tree.cpp +++ b/fxjs/xfa/cjx_tree.cpp
@@ -65,17 +65,15 @@ value->DirectGetValue().Get(runtime->GetIsolate())); } - const XFA_SCRIPTATTRIBUTEINFO* lpAttributeInfo = - resolveNodeRS.pScriptAttribute.Get(); - if (!lpAttributeInfo || - lpAttributeInfo->eValueType != XFA_ScriptType::Object) { + if (!resolveNodeRS.script_attribute.callback || + resolveNodeRS.script_attribute.eValueType != XFA_ScriptType::Object) { return CJS_Result::Success(runtime->NewNull()); } auto pValue = pdfium::MakeUnique<CFXJSE_Value>(pScriptContext->GetIsolate()); CJX_Object* jsObject = resolveNodeRS.objects.front()->JSObject(); - (jsObject->*(lpAttributeInfo->callback))(pValue.get(), false, - lpAttributeInfo->attribute); + (jsObject->*(resolveNodeRS.script_attribute.callback))( + pValue.get(), false, resolveNodeRS.script_attribute.attribute); return CJS_Result::Success( pValue->DirectGetValue().Get(runtime->GetIsolate())); } @@ -230,14 +228,14 @@ pNodeList->Append(pObject->AsNode()); } } else { - if (resolveNodeRS.pScriptAttribute && - resolveNodeRS.pScriptAttribute->eValueType == XFA_ScriptType::Object) { + if (resolveNodeRS.script_attribute.callback && + resolveNodeRS.script_attribute.eValueType == XFA_ScriptType::Object) { for (auto& pObject : resolveNodeRS.objects) { auto innerValue = pdfium::MakeUnique<CFXJSE_Value>(pScriptContext->GetIsolate()); CJX_Object* jsObject = pObject->JSObject(); - (jsObject->*(resolveNodeRS.pScriptAttribute->callback))( - innerValue.get(), false, resolveNodeRS.pScriptAttribute->attribute); + (jsObject->*(resolveNodeRS.script_attribute.callback))( + innerValue.get(), false, resolveNodeRS.script_attribute.attribute); CXFA_Object* obj = CFXJSE_Engine::ToObject(innerValue.get()); if (obj->IsNode())
diff --git a/xfa/fxfa/parser/xfa_basic_data.cpp b/xfa/fxfa/parser/xfa_basic_data.cpp index 69e5f14..0293e76 100644 --- a/xfa/fxfa/parser/xfa_basic_data.cpp +++ b/xfa/fxfa/parser/xfa_basic_data.cpp
@@ -168,12 +168,12 @@ #undef ELEM_HIDDEN____ }; -struct XFA_SCRIPTINDEX { +struct ScriptIndexRecord { uint16_t wAttributeStart; uint16_t wAttributeCount; }; -const XFA_SCRIPTINDEX g_XFAScriptIndex[] = { +const ScriptIndexRecord g_ScriptIndexTable[] = { {/* ps */ 0, 2}, {/* to */ 2, 2}, {/* ui */ 4, 2}, @@ -506,10 +506,17 @@ static_assert(static_cast<int>(XFA_Element::Model) == 319, "319"); static_assert(static_cast<int>(XFA_Element::Placeholder4) == 320, "320"); +struct ElementAttributeRecord { + uint32_t uHash; // Hashed as wide string. + XFA_Attribute attribute; + XFA_ScriptType eValueType; + XFA_ATTRIBUTE_CALLBACK callback; +}; + #undef ATTR #define ATTR(a, b, c, d, e) a, d, e, reinterpret_cast<XFA_ATTRIBUTE_CALLBACK>(c) -const XFA_SCRIPTATTRIBUTEINFO g_SomAttributeData[] = { +const ElementAttributeRecord g_ElementAttributeTable[] = { /* ps */ {ATTR(0xbe52dfbf, "desc", @@ -6560,24 +6567,29 @@ return {}; } -const XFA_SCRIPTATTRIBUTEINFO* XFA_GetScriptAttributeByName( +Optional<XFA_SCRIPTATTRIBUTEINFO> XFA_GetScriptAttributeByName( XFA_Element eElement, WideStringView wsAttributeName) { if (wsAttributeName.IsEmpty()) - return nullptr; + return {}; uint32_t uHash = FX_HashCode_GetW(wsAttributeName, false); while (eElement != XFA_Element::Unknown) { - const XFA_SCRIPTINDEX* scriptIndex = - &g_XFAScriptIndex[static_cast<size_t>(eElement)]; + const ScriptIndexRecord* scriptIndex = + &g_ScriptIndexTable[static_cast<size_t>(eElement)]; size_t iStart = scriptIndex->wAttributeStart; size_t iEnd = iStart + scriptIndex->wAttributeCount; for (size_t iter = iStart; iter < iEnd; ++iter) { - const XFA_SCRIPTATTRIBUTEINFO* pInfo = &g_SomAttributeData[iter]; - if (uHash == pInfo->uHash) - return pInfo; + const ElementAttributeRecord* pInfo = &g_ElementAttributeTable[iter]; + if (uHash == pInfo->uHash) { + XFA_SCRIPTATTRIBUTEINFO result; + result.attribute = pInfo->attribute; + result.eValueType = pInfo->eValueType; + result.callback = pInfo->callback; + return result; + } } eElement = g_XFAScriptParents[static_cast<size_t>(eElement)]; } - return nullptr; + return {}; }
diff --git a/xfa/fxfa/parser/xfa_basic_data.h b/xfa/fxfa/parser/xfa_basic_data.h index 0a12a75..52768d5 100644 --- a/xfa/fxfa/parser/xfa_basic_data.h +++ b/xfa/fxfa/parser/xfa_basic_data.h
@@ -10,13 +10,15 @@ #include <stddef.h> #include "core/fxcrt/widestring.h" +#include "fxjs/xfa/cjx_object.h" +#include "third_party/base/optional.h" #include "xfa/fxfa/fxfa_basic.h" ByteStringView XFA_AttributeValueToName(XFA_AttributeValue item); Optional<XFA_AttributeValue> XFA_GetAttributeValueByName( const WideStringView& name); -const XFA_SCRIPTATTRIBUTEINFO* XFA_GetScriptAttributeByName( +Optional<XFA_SCRIPTATTRIBUTEINFO> XFA_GetScriptAttributeByName( XFA_Element eElement, WideStringView wsAttributeName);
diff --git a/xfa/fxfa/parser/xfa_basic_data_unittest.cpp b/xfa/fxfa/parser/xfa_basic_data_unittest.cpp index 5c58755..199bda4 100644 --- a/xfa/fxfa/parser/xfa_basic_data_unittest.cpp +++ b/xfa/fxfa/parser/xfa_basic_data_unittest.cpp
@@ -7,11 +7,18 @@ #include "testing/gtest/include/gtest/gtest.h" TEST(XFABasicDataTest, GetAttributeValueByName) { - EXPECT_FALSE(!!XFA_GetAttributeValueByName(L"")); - EXPECT_FALSE(!!XFA_GetAttributeValueByName(L"nonesuch")); - EXPECT_EQ(XFA_AttributeValue::Asterisk, *XFA_GetAttributeValueByName(L"*")); - EXPECT_EQ(XFA_AttributeValue::Visible, - *XFA_GetAttributeValueByName(L"visible")); - EXPECT_EQ(XFA_AttributeValue::Lowered, - *XFA_GetAttributeValueByName(L"lowered")); + EXPECT_FALSE(XFA_GetAttributeValueByName(L"").has_value()); + EXPECT_FALSE(XFA_GetAttributeValueByName(L"nonesuch").has_value()); + + Optional<XFA_AttributeValue> result = XFA_GetAttributeValueByName(L"*"); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(XFA_AttributeValue::Asterisk, result.value()); + + result = XFA_GetAttributeValueByName(L"visible"); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(XFA_AttributeValue::Visible, result.value()); + + result = XFA_GetAttributeValueByName(L"lowered"); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(XFA_AttributeValue::Lowered, result.value()); }
diff --git a/xfa/fxfa/parser/xfa_resolvenode_rs.h b/xfa/fxfa/parser/xfa_resolvenode_rs.h index dffa0cb..ed2acd5 100644 --- a/xfa/fxfa/parser/xfa_resolvenode_rs.h +++ b/xfa/fxfa/parser/xfa_resolvenode_rs.h
@@ -10,9 +10,9 @@ #include <vector> #include "core/fxcrt/unowned_ptr.h" +#include "fxjs/xfa/cjx_object.h" class CXFA_Object; -struct XFA_SCRIPTATTRIBUTEINFO; enum XFA_ResolveNode_RSType { XFA_ResolveNode_RSType_Nodes, @@ -28,8 +28,8 @@ ~XFA_RESOLVENODE_RS(); XFA_ResolveNode_RSType dwFlags = XFA_ResolveNode_RSType_Nodes; + XFA_SCRIPTATTRIBUTEINFO script_attribute; std::vector<UnownedPtr<CXFA_Object>> objects; - UnownedPtr<const XFA_SCRIPTATTRIBUTEINFO> pScriptAttribute; }; inline XFA_RESOLVENODE_RS::XFA_RESOLVENODE_RS() = default;