From 61dfd72ab3a913b143c03bdcf4ec5816dcb09cd1 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Thu, 11 Jan 2018 14:51:11 +0000 Subject: Rename GetItem to GetItemIfExists Rename GetItem to make it clearer it can return nullptr. Change-Id: I0e09a79c3e2244b08212d3b51d3ad0e6a86edfd9 Reviewed-on: https://pdfium-review.googlesource.com/22713 Reviewed-by: Ryan Harrison Commit-Queue: dsinclair --- fxjs/xfa/cjx_instancemanager.cpp | 29 ++++++++++++++++++++++------- fxjs/xfa/cjx_object.cpp | 4 ++-- xfa/fxfa/parser/cxfa_node.cpp | 15 ++++++++++++--- xfa/fxfa/parser/cxfa_node.h | 2 +- 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/fxjs/xfa/cjx_instancemanager.cpp b/fxjs/xfa/cjx_instancemanager.cpp index d6ac66d9df..830541ed7c 100644 --- a/fxjs/xfa/cjx_instancemanager.cpp +++ b/fxjs/xfa/cjx_instancemanager.cpp @@ -58,8 +58,15 @@ int32_t CJX_InstanceManager::SetInstances(int32_t iDesired) { : wsInstManagerName.Right(wsInstManagerName.GetLength() - 1)); uint32_t dInstanceNameHash = FX_HashCode_GetW(wsInstanceName.AsStringView(), false); - CXFA_Node* pPrevSibling = - iDesired == 0 ? GetXFANode() : GetXFANode()->GetItem(iDesired - 1); + CXFA_Node* pPrevSibling = iDesired == 0 + ? GetXFANode() + : GetXFANode()->GetItemIfExists(iDesired - 1); + if (!pPrevSibling) { + // TODO(dsinclair): Better error? + ThrowIndexOutOfBoundsException(); + return 0; + } + while (iCount > iDesired) { CXFA_Node* pRemoveInstance = pPrevSibling->GetNextSibling(); if (pRemoveInstance->GetElementType() != XFA_Element::Subform && @@ -109,7 +116,12 @@ int32_t CJX_InstanceManager::MoveInstance(int32_t iTo, int32_t iFrom) { if (iFrom < 0 || iTo < 0 || iFrom == iTo) return 0; - CXFA_Node* pMoveInstance = GetXFANode()->GetItem(iFrom); + CXFA_Node* pMoveInstance = GetXFANode()->GetItemIfExists(iFrom); + if (!pMoveInstance) { + ThrowIndexOutOfBoundsException(); + return 1; + } + GetXFANode()->RemoveItem(pMoveInstance, false); GetXFANode()->InsertItem(pMoveInstance, iTo, iCount - 1, true); CXFA_LayoutProcessor* pLayoutPro = GetDocument()->GetLayoutProcessor(); @@ -134,11 +146,11 @@ CJS_Return CJX_InstanceManager::moveInstance( if (!pNotify) return CJS_Return(true); - CXFA_Node* pToInstance = GetXFANode()->GetItem(iTo); + CXFA_Node* pToInstance = GetXFANode()->GetItemIfExists(iTo); if (pToInstance && pToInstance->GetElementType() == XFA_Element::Subform) pNotify->RunSubformIndexChange(pToInstance); - CXFA_Node* pFromInstance = GetXFANode()->GetItem(iFrom); + CXFA_Node* pFromInstance = GetXFANode()->GetItemIfExists(iFrom); if (pFromInstance && pFromInstance->GetElementType() == XFA_Element::Subform) { pNotify->RunSubformIndexChange(pFromInstance); @@ -163,13 +175,16 @@ CJS_Return CJX_InstanceManager::removeInstance( if (iCount - 1 < iMin) return CJS_Return(JSGetStringFromID(JSMessage::kTooManyOccurances)); - CXFA_Node* pRemoveInstance = GetXFANode()->GetItem(iIndex); + CXFA_Node* pRemoveInstance = GetXFANode()->GetItemIfExists(iIndex); + if (!pRemoveInstance) + return CJS_Return(JSGetStringFromID(JSMessage::kParamError)); + GetXFANode()->RemoveItem(pRemoveInstance, true); CXFA_FFNotify* pNotify = GetDocument()->GetNotify(); if (pNotify) { for (int32_t i = iIndex; i < iCount - 1; i++) { - CXFA_Node* pSubformInstance = GetXFANode()->GetItem(i); + CXFA_Node* pSubformInstance = GetXFANode()->GetItemIfExists(i); if (pSubformInstance && pSubformInstance->GetElementType() == XFA_Element::Subform) { pNotify->RunSubformIndexChange(pSubformInstance); diff --git a/fxjs/xfa/cjx_object.cpp b/fxjs/xfa/cjx_object.cpp index cb76497618..8608cc0e8d 100644 --- a/fxjs/xfa/cjx_object.cpp +++ b/fxjs/xfa/cjx_object.cpp @@ -1629,12 +1629,12 @@ void CJX_Object::Script_Som_InstanceIndex(CFXJSE_Value* pValue, if (!pNotify) return; - CXFA_Node* pToInstance = pManagerNode->GetItem(iTo); + CXFA_Node* pToInstance = pManagerNode->GetItemIfExists(iTo); if (pToInstance && pToInstance->GetElementType() == XFA_Element::Subform) { pNotify->RunSubformIndexChange(pToInstance); } - CXFA_Node* pFromInstance = pManagerNode->GetItem(iFrom); + CXFA_Node* pFromInstance = pManagerNode->GetItemIfExists(iFrom); if (pFromInstance && pFromInstance->GetElementType() == XFA_Element::Subform) { pNotify->RunSubformIndexChange(pFromInstance); diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp index c29e30347f..075ee46836 100644 --- a/xfa/fxfa/parser/cxfa_node.cpp +++ b/xfa/fxfa/parser/cxfa_node.cpp @@ -1129,7 +1129,7 @@ bool CXFA_Node::IsNeedSavingXMLNode() { GetElementType() == XFA_Element::Xfa); } -CXFA_Node* CXFA_Node::GetItem(int32_t iIndex) { +CXFA_Node* CXFA_Node::GetItemIfExists(int32_t iIndex) { int32_t iCount = 0; uint32_t dwNameHash = 0; for (CXFA_Node* pNode = GetNextSibling(); pNode; @@ -1198,8 +1198,12 @@ void CXFA_Node::InsertItem(CXFA_Node* pNewInstance, if (iPos < 0) iPos = iCount; if (iPos == iCount) { + CXFA_Node* item = GetItemIfExists(iCount - 1); + if (!item) + return; + CXFA_Node* pNextSibling = - iCount > 0 ? GetItem(iCount - 1)->GetNextSibling() : GetNextSibling(); + iCount > 0 ? item->GetNextSibling() : GetNextSibling(); GetParent()->InsertChild(pNewInstance, pNextSibling); if (bMoveDataBindingNodes) { std::set sNew; @@ -1229,7 +1233,12 @@ void CXFA_Node::InsertItem(CXFA_Node* pNewInstance, ReorderDataNodes(sNew, sAfter, false); } } else { - CXFA_Node* pBeforeInstance = GetItem(iPos); + CXFA_Node* pBeforeInstance = GetItemIfExists(iPos); + if (!pBeforeInstance) { + // TODO(dsinclair): What should happen here? + return; + } + GetParent()->InsertChild(pNewInstance, pBeforeInstance); if (bMoveDataBindingNodes) { std::set sNew; diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h index 1e20ace09c..8108eeccc4 100644 --- a/xfa/fxfa/parser/cxfa_node.h +++ b/xfa/fxfa/parser/cxfa_node.h @@ -104,7 +104,7 @@ class CXFA_Node : public CXFA_Object { CXFA_Node* CreateInstanceIfPossible(bool bDataMerge); int32_t GetCount(); - CXFA_Node* GetItem(int32_t iIndex); + CXFA_Node* GetItemIfExists(int32_t iIndex); void RemoveItem(CXFA_Node* pRemoveInstance, bool bRemoveDataBinding); void InsertItem(CXFA_Node* pNewInstance, int32_t iPos, -- cgit v1.2.3