From 68c77592f25f9173d2166fa01fb34b4155f4cfcb Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Mon, 20 Nov 2017 20:27:43 +0000 Subject: Remove {Set|Get|Try}Object methods The CJX_Node::{Set|Get|Try}Object methods are used to set CXFA_WidgetData and CXFA_Node objects into the CJX_Node. This is stored in the node as void* data and custom delete methods are passed when needed. Instead, this CL just stores the two types of things we need on the CJX_Node and returns the correct types. This uses a bit more space on the CJX_Node, but it makes the code a lot clearer and simpler. Change-Id: I8546ea6456a1c1f5f8b602a6a420a0e85f3fac29 Reviewed-on: https://pdfium-review.googlesource.com/18570 Reviewed-by: Tom Sepez Commit-Queue: dsinclair --- fxjs/cjx_node.cpp | 47 ++++++++++++++++++++--------------------------- fxjs/cjx_node.h | 31 ++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 32 deletions(-) (limited to 'fxjs') diff --git a/fxjs/cjx_node.cpp b/fxjs/cjx_node.cpp index 07b96b895f..b86c2fa6fb 100644 --- a/fxjs/cjx_node.cpp +++ b/fxjs/cjx_node.cpp @@ -1439,14 +1439,14 @@ void CJX_Node::Script_Som_DefaultValue(CFXJSE_Value* pValue, } if (bSetting) { WideString wsNewValue; - if (!(pValue && (pValue->IsNull() || pValue->IsUndefined()))) + if (pValue && !(pValue->IsNull() || pValue->IsUndefined())) wsNewValue = pValue->ToWideString(); WideString wsFormatValue(wsNewValue); CXFA_WidgetData* pContainerWidgetData = nullptr; if (GetXFANode()->GetPacketID() == XFA_XDPPACKET_Datasets) { WideString wsPicture; - for (CXFA_Node* pFormNode : GetXFANode()->GetBindItems()) { + for (const auto& pFormNode : *(GetXFANode()->GetBindItems())) { if (!pFormNode || pFormNode->HasRemovedChildren()) continue; @@ -1511,7 +1511,7 @@ void CJX_Node::Script_Boolean_Value(CFXJSE_Value* pValue, } ByteString newValue; - if (!(pValue && (pValue->IsNull() || pValue->IsUndefined()))) + if (pValue && !(pValue->IsNull() || pValue->IsUndefined())) newValue = pValue->ToString(); int32_t iValue = FXSYS_atoi(newValue.c_str()); @@ -1670,7 +1670,7 @@ void CJX_Node::Script_Field_DefaultValue(CFXJSE_Value* pValue, } WideString wsNewText; - if (!(pValue && (pValue->IsNull() || pValue->IsUndefined()))) + if (pValue && !(pValue->IsNull() || pValue->IsUndefined())) wsNewText = pValue->ToWideString(); CXFA_Node* pUIChild = pWidgetData->GetUIChild(); @@ -3130,7 +3130,7 @@ bool CJX_Node::SetCData(XFA_Attribute eAttr, GetXFANode()->GetNodeItem(XFA_NODEITEM_FirstChild); pChildDataNode; pChildDataNode = pChildDataNode->GetNodeItem( XFA_NODEITEM_NextSibling)) { - if (!pChildDataNode->GetBindItems().empty()) { + if (!pChildDataNode->GetBindItems()->empty()) { bDeleteChildren = false; break; } @@ -3192,7 +3192,7 @@ bool CJX_Node::SetAttributeValue(const WideString& wsValue, GetXFANode()->GetNodeItem(XFA_NODEITEM_FirstChild); pChildDataNode; pChildDataNode = pChildDataNode->GetNodeItem( XFA_NODEITEM_NextSibling)) { - if (!pChildDataNode->GetBindItems().empty()) { + if (!pChildDataNode->GetBindItems()->empty()) { bDeleteChildren = false; break; } @@ -3232,24 +3232,6 @@ pdfium::Optional CJX_Node::TryCData(XFA_Attribute eAttr, return GetXFANode()->GetDefaultCData(eAttr); } -bool CJX_Node::SetObject(XFA_Attribute eAttr, - void* pData, - XFA_MAPDATABLOCKCALLBACKINFO* pCallbackInfo) { - void* pKey = GetMapKey_Element(GetXFANode()->GetElementType(), eAttr); - return SetUserData(pKey, pData, pCallbackInfo); -} - -void* CJX_Node::GetObject(XFA_Attribute eAttr) { - void* pData; - return TryObject(eAttr, pData) ? pData : nullptr; -} - -bool CJX_Node::TryObject(XFA_Attribute eAttr, void*& pData) { - void* pKey = GetMapKey_Element(GetXFANode()->GetElementType(), eAttr); - pData = GetUserData(pKey, false); - return !!pData; -} - bool CJX_Node::SetValue(XFA_Attribute eAttr, XFA_AttributeType eType, void* pValue, @@ -3289,6 +3271,13 @@ bool CJX_Node::SetValue(XFA_Attribute eAttr, return true; } +// TODO(dsinclair): This should not be needed. Nodes should get un-bound when +// they're deleted instead of us pointing to bad objects. +void CJX_Node::ReleaseBindingNodes() { + for (auto& node : binding_nodes_) + node.Release(); +} + void* CJX_Node::GetUserData(void* pKey, bool bProtoAlso) { void* pData; return TryUserData(pKey, pData, bProtoAlso) ? pData : nullptr; @@ -3391,7 +3380,7 @@ bool CJX_Node::SetContent(const WideString& wsContent, i++; } } - for (CXFA_Node* pArrayNode : pBind->GetBindItems()) { + for (const auto& pArrayNode : *(pBind->GetBindItems())) { if (pArrayNode != GetXFANode()) { pArrayNode->JSNode()->SetContent(wsContent, wsContent, bNotify, bScriptModify, false); @@ -3416,7 +3405,7 @@ bool CJX_Node::SetContent(const WideString& wsContent, if (pBindNode && bSyncData) { pBindNode->JSNode()->SetContent(wsContent, wsXMLValue, bNotify, bScriptModify, false); - for (CXFA_Node* pArrayNode : pBindNode->GetBindItems()) { + for (const auto& pArrayNode : *(pBindNode->GetBindItems())) { if (pArrayNode != GetXFANode()) { pArrayNode->JSNode()->SetContent(wsContent, wsContent, bNotify, true, false); @@ -3486,7 +3475,7 @@ bool CJX_Node::SetContent(const WideString& wsContent, SetAttributeValue(wsContent, wsXMLValue, bNotify, bScriptModify); if (pBindNode && bSyncData) { - for (CXFA_Node* pArrayNode : pBindNode->GetBindItems()) { + for (const auto& pArrayNode : *(pBindNode->GetBindItems())) { pArrayNode->JSNode()->SetContent(wsContent, wsContent, bNotify, bScriptModify, false); } @@ -3561,6 +3550,10 @@ pdfium::Optional CJX_Node::TryContent(bool bScriptModify, return {}; } +void CJX_Node::SetWidgetData(std::unique_ptr data) { + widget_data_ = std::move(data); +} + pdfium::Optional CJX_Node::TryNamespace() { if (GetXFANode()->IsModelNode() || GetXFANode()->GetElementType() == XFA_Element::Packet) { diff --git a/fxjs/cjx_node.h b/fxjs/cjx_node.h index 686c54c164..3393a52406 100644 --- a/fxjs/cjx_node.h +++ b/fxjs/cjx_node.h @@ -8,6 +8,8 @@ #define FXJS_CJX_NODE_H_ #include +#include +#include #include "core/fxcrt/unowned_ptr.h" #include "fxjs/cjx_object.h" @@ -29,6 +31,7 @@ enum XFA_SOM_MESSAGETYPE { class CFXJSE_Arguments; class CXFA_Node; +class CXFA_WidgetData; struct XFA_MAPMODULEDATA; @@ -102,11 +105,27 @@ class CJX_Node : public CJX_Object { XFA_MAPDATABLOCKCALLBACKINFO* pCallbackInfo); void* GetUserData(void* pKey, bool bProtoAlso); - bool TryObject(XFA_Attribute eAttr, void*& pData); - bool SetObject(XFA_Attribute eAttr, - void* pData, - XFA_MAPDATABLOCKCALLBACKINFO* pCallbackInfo); - void* GetObject(XFA_Attribute eAttr); + void SetBindingNodes(std::vector> nodes) { + binding_nodes_ = std::move(nodes); + } + std::vector>* GetBindingNodes() { + return &binding_nodes_; + } + void ReleaseBindingNodes(); + + void SetBindingNode(CXFA_Node* node) { + binding_nodes_.clear(); + if (node) + binding_nodes_.emplace_back(node); + } + CXFA_Node* GetBindingNode() const { + if (binding_nodes_.empty()) + return nullptr; + return binding_nodes_[0].Get(); + } + + void SetWidgetData(std::unique_ptr data); + CXFA_WidgetData* GetWidgetData() const { return widget_data_.get(); } pdfium::Optional TryNamespace(); @@ -437,6 +456,8 @@ class CJX_Node : public CJX_Object { XFA_Element eType); std::unique_ptr map_module_data_; + std::unique_ptr widget_data_; + std::vector> binding_nodes_; }; #endif // FXJS_CJX_NODE_H_ -- cgit v1.2.3