From 76a44dea318041f8229d80e70ca3568a435611eb Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Wed, 11 Jan 2017 08:58:35 -0500 Subject: Cleaning up memory allocation in CXFA_FM2JSContext - IV This CL removes the use of FX_Alloc and any remaining new'd CFXJSE_Value objects from CXFA_FM2JSContext and replaces them with unique_ptrs and vectors. Change-Id: I30ba697d65ee326d2faa895c3217bdc407419298 Reviewed-on: https://pdfium-review.googlesource.com/2157 Commit-Queue: dsinclair Reviewed-by: Tom Sepez --- fxjs/cfxjse_value.cpp | 16 ++-- fxjs/cfxjse_value.h | 5 +- xfa/fxfa/fm2js/xfa_fm2jscontext.cpp | 177 +++++++++++++++--------------------- xfa/fxfa/fm2js/xfa_fm2jscontext.h | 10 +- 4 files changed, 91 insertions(+), 117 deletions(-) diff --git a/fxjs/cfxjse_value.cpp b/fxjs/cfxjse_value.cpp index 471d85cf76..68c82e5deb 100644 --- a/fxjs/cfxjse_value.cpp +++ b/fxjs/cfxjse_value.cpp @@ -102,16 +102,14 @@ void CFXJSE_Value::SetHostObject(CFXJSE_HostObject* lpObject, m_hValue.Reset(m_pIsolate, hObject); } -void CFXJSE_Value::SetArray(uint32_t uValueCount, CFXJSE_Value** rgValues) { +void CFXJSE_Value::SetArray( + const std::vector>& values) { CFXJSE_ScopeUtil_IsolateHandleRootContext scope(m_pIsolate); - v8::Local hArrayObject = v8::Array::New(m_pIsolate, uValueCount); - if (rgValues) { - for (uint32_t i = 0; i < uValueCount; i++) { - if (rgValues[i]) { - hArrayObject->Set(i, v8::Local::New( - m_pIsolate, rgValues[i]->DirectGetValue())); - } - } + v8::Local hArrayObject = v8::Array::New(m_pIsolate, values.size()); + uint32_t count = 0; + for (auto& v : values) { + hArrayObject->Set(count++, v8::Local::New( + m_pIsolate, v.get()->DirectGetValue())); } m_hValue.Reset(m_pIsolate, hArrayObject); } diff --git a/fxjs/cfxjse_value.h b/fxjs/cfxjse_value.h index 487cf0c6e8..f2ebdc1c25 100644 --- a/fxjs/cfxjse_value.h +++ b/fxjs/cfxjse_value.h @@ -7,6 +7,9 @@ #ifndef FXJS_CFXJSE_VALUE_H_ #define FXJS_CFXJSE_VALUE_H_ +#include +#include + #include "core/fxcrt/fx_string.h" #include "core/fxcrt/fx_system.h" #include "fxjs/cfxjse_isolatetracker.h" @@ -52,7 +55,7 @@ class CFXJSE_Value { void SetObject(CFXJSE_HostObject* lpObject, CFXJSE_Class* pClass); void SetHostObject(CFXJSE_HostObject* lpObject, CFXJSE_Class* lpClass); - void SetArray(uint32_t uValueCount, CFXJSE_Value** rgValues); + void SetArray(const std::vector>& values); void SetDate(double dDouble); bool GetObjectProperty(const CFX_ByteStringC& szPropName, diff --git a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp index 3c2b0ee126..b307687f9e 100644 --- a/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp +++ b/xfa/fxfa/fm2js/xfa_fm2jscontext.cpp @@ -2868,18 +2868,14 @@ void CXFA_FM2JSContext::Oneof(CFXJSE_Value* pThis, bool bFlags = false; std::unique_ptr argOne = GetSimpleValue(pThis, args, 0); - CFXJSE_Value** parametersValue = nullptr; - int32_t iCount = 0; - unfoldArgs(pThis, args, parametersValue, iCount, 1); - for (int32_t i = 0; i < iCount; i++) { - if (simpleValueCompare(pThis, argOne.get(), parametersValue[i])) { + std::vector> parameterValues; + unfoldArgs(pThis, args, ¶meterValues, 1); + for (const auto& value : parameterValues) { + if (simpleValueCompare(pThis, argOne.get(), value.get())) { bFlags = true; break; } } - for (int32_t i = 0; i < iCount; i++) - delete parametersValue[i]; - FX_Free(parametersValue); args.GetReturnValue()->SetInteger(bFlags); } @@ -2994,15 +2990,15 @@ void CXFA_FM2JSContext::Ref(CFXJSE_Value* pThis, return; } - CFXJSE_Value* rgValues[3]; + std::vector> values; for (int32_t i = 0; i < 3; i++) - rgValues[i] = new CFXJSE_Value(pIsolate); + values.push_back(pdfium::MakeUnique(pIsolate)); int intVal = 3; if (argOne->IsNull()) { // TODO(dsinclair): Why is this 4 when the others are all 3? intVal = 4; - rgValues[2]->SetNull(); + values[2]->SetNull(); } else if (argOne->IsArray()) { #ifndef NDEBUG auto lengthValue = pdfium::MakeUnique(pIsolate); @@ -3015,24 +3011,18 @@ void CXFA_FM2JSContext::Ref(CFXJSE_Value* pThis, argOne->GetObjectPropertyByIdx(1, propertyValue.get()); argOne->GetObjectPropertyByIdx(2, jsObjectValue.get()); if (!propertyValue->IsNull() || jsObjectValue->IsNull()) { - for (int32_t i = 0; i < 3; i++) - delete rgValues[i]; - pContext->ThrowArgumentMismatchException(); return; } - rgValues[2]->Assign(jsObjectValue.get()); + values[2]->Assign(jsObjectValue.get()); } else if (argOne->IsObject()) { - rgValues[2]->Assign(argOne.get()); + values[2]->Assign(argOne.get()); } - rgValues[0]->SetInteger(intVal); - rgValues[1]->SetNull(); - args.GetReturnValue()->SetArray(3, rgValues); - - for (int32_t i = 0; i < 3; i++) - delete rgValues[i]; + values[0]->SetInteger(intVal); + values[1]->SetNull(); + args.GetReturnValue()->SetArray(values); } // static @@ -4881,9 +4871,9 @@ void CXFA_FM2JSContext::Get(CFXJSE_Value* pThis, return; int32_t size = pFile->GetSize(); - std::unique_ptr pData(FX_Alloc(uint8_t, size)); - pFile->ReadBlock(pData.get(), size); - args.GetReturnValue()->SetString(CFX_ByteStringC(pData.get(), size)); + std::vector pData(size); + pFile->ReadBlock(pData.data(), size); + args.GetReturnValue()->SetString(CFX_ByteStringC(pData.data(), size)); } // static @@ -5481,29 +5471,24 @@ void CXFA_FM2JSContext::dot_accessor(CFXJSE_Value* pThis, return; } - CFXJSE_Value** rgValues = FX_Alloc(CFXJSE_Value*, iCounter + 2); - for (int32_t i = 0; i < (iCounter + 2); i++) - rgValues[i] = new CFXJSE_Value(pIsolate); + std::vector> values; + for (int32_t i = 0; i < iCounter + 2; i++) + values.push_back(pdfium::MakeUnique(pIsolate)); - rgValues[0]->SetInteger(1); + values[0]->SetInteger(1); if (bAttribute) - rgValues[1]->SetString(szName.AsStringC()); + values[1]->SetString(szName.AsStringC()); else - rgValues[1]->SetNull(); + values[1]->SetNull(); int32_t iIndex = 2; for (int32_t i = 0; i < iLength - 2; i++) { for (size_t j = 0; j < resolveValues[i].size(); j++) { - rgValues[iIndex]->Assign(resolveValues[i][j].get()); + values[iIndex]->Assign(resolveValues[i][j].get()); iIndex++; } } - args.GetReturnValue()->SetArray(iCounter + 2, rgValues); - - for (int32_t i = 0; i < (iCounter + 2); i++) - delete rgValues[i]; - - FX_Free(rgValues); + args.GetReturnValue()->SetArray(values); return; } @@ -5531,23 +5516,21 @@ void CXFA_FM2JSContext::dot_accessor(CFXJSE_Value* pThis, bool bAttribute = false; ParseResolveResult(pThis, resoveNodeRS, argAccessor.get(), &resolveValues, &bAttribute); - CFXJSE_Value** rgValues = FX_Alloc(CFXJSE_Value*, resolveValues.size() + 2); + + std::vector> values; for (size_t i = 0; i < resolveValues.size() + 2; i++) - rgValues[i] = new CFXJSE_Value(pIsolate); + values.push_back(pdfium::MakeUnique(pIsolate)); - rgValues[0]->SetInteger(1); + values[0]->SetInteger(1); if (bAttribute) - rgValues[1]->SetString(szName.AsStringC()); + values[1]->SetString(szName.AsStringC()); else - rgValues[1]->SetNull(); + values[1]->SetNull(); for (size_t i = 0; i < resolveValues.size(); i++) - rgValues[i + 2]->Assign(resolveValues[i].get()); + values[i + 2]->Assign(resolveValues[i].get()); - args.GetReturnValue()->SetArray(resolveValues.size() + 2, rgValues); - for (size_t i = 0; i < (resolveValues.size() + 2); i++) - delete rgValues[i]; - FX_Free(rgValues); + args.GetReturnValue()->SetArray(values); } // static @@ -5607,28 +5590,24 @@ void CXFA_FM2JSContext::dotdot_accessor(CFXJSE_Value* pThis, return; } - CFXJSE_Value** rgValues = FX_Alloc(CFXJSE_Value*, iCounter + 2); - for (int32_t i = 0; i < (iCounter + 2); i++) - rgValues[i] = new CFXJSE_Value(pIsolate); + std::vector> values; + for (int32_t i = 0; i < iCounter + 2; i++) + values.push_back(pdfium::MakeUnique(pIsolate)); - rgValues[0]->SetInteger(1); + values[0]->SetInteger(1); if (bAttribute) - rgValues[1]->SetString(szName.AsStringC()); + values[1]->SetString(szName.AsStringC()); else - rgValues[1]->SetNull(); + values[1]->SetNull(); int32_t iIndex = 2; for (int32_t i = 0; i < iLength - 2; i++) { for (size_t j = 0; j < resolveValues[i].size(); j++) { - rgValues[iIndex]->Assign(resolveValues[i][j].get()); + values[iIndex]->Assign(resolveValues[i][j].get()); iIndex++; } } - args.GetReturnValue()->SetArray(iCounter + 2, rgValues); - for (int32_t i = 0; i < (iCounter + 2); i++) - delete rgValues[i]; - - FX_Free(rgValues); + args.GetReturnValue()->SetArray(values); return; } @@ -5656,24 +5635,21 @@ void CXFA_FM2JSContext::dotdot_accessor(CFXJSE_Value* pThis, bool bAttribute = false; ParseResolveResult(pThis, resoveNodeRS, argAccessor.get(), &resolveValues, &bAttribute); - CFXJSE_Value** rgValues = FX_Alloc(CFXJSE_Value*, resolveValues.size() + 2); - for (size_t i = 0; i < (resolveValues.size() + 2); i++) - rgValues[i] = new CFXJSE_Value(pIsolate); - rgValues[0]->SetInteger(1); + std::vector> values; + for (size_t i = 0; i < resolveValues.size() + 2; i++) + values.push_back(pdfium::MakeUnique(pIsolate)); + + values[0]->SetInteger(1); if (bAttribute) - rgValues[1]->SetString(szName.AsStringC()); + values[1]->SetString(szName.AsStringC()); else - rgValues[1]->SetNull(); + values[1]->SetNull(); for (size_t i = 0; i < resolveValues.size(); i++) - rgValues[i + 2]->Assign(resolveValues[i].get()); - - args.GetReturnValue()->SetArray(resolveValues.size() + 2, rgValues); + values[i + 2]->Assign(resolveValues[i].get()); - for (size_t i = 0; i < (resolveValues.size() + 2); i++) - delete rgValues[i]; - FX_Free(rgValues); + args.GetReturnValue()->SetArray(values); } // static @@ -5830,17 +5806,14 @@ void CXFA_FM2JSContext::fm_var_filter(CFXJSE_Value* pThis, } if (iFlags == 4) { - CFXJSE_Value* rgValues[3]; + std::vector> values; for (int32_t i = 0; i < 3; i++) - rgValues[i] = new CFXJSE_Value(pIsolate); - - rgValues[0]->SetInteger(3); - rgValues[1]->SetNull(); - rgValues[2]->SetNull(); - args.GetReturnValue()->SetArray(3, rgValues); - for (int32_t i = 0; i < 3; i++) - delete rgValues[i]; + values.push_back(pdfium::MakeUnique(pIsolate)); + values[0]->SetInteger(3); + values[1]->SetNull(); + values[2]->SetNull(); + args.GetReturnValue()->SetArray(values); return; } @@ -5872,29 +5845,26 @@ void CXFA_FM2JSContext::concat_fm_object(CFXJSE_Value* pThis, iLength += 1; } - CFXJSE_Value** returnValues = FX_Alloc(CFXJSE_Value*, iLength); + std::vector> returnValues; for (int32_t i = 0; i < (int32_t)iLength; i++) - returnValues[i] = new CFXJSE_Value(pIsolate); + returnValues.push_back(pdfium::MakeUnique(pIsolate)); int32_t index = 0; for (int32_t i = 0; i < argc; i++) { if (argValues[i]->IsArray()) { auto lengthValue = pdfium::MakeUnique(pIsolate); argValues[i]->GetObjectProperty("length", lengthValue.get()); + int32_t length = lengthValue->ToInteger(); for (int32_t j = 2; j < length; j++) { - argValues[i]->GetObjectPropertyByIdx(j, returnValues[index]); + argValues[i]->GetObjectPropertyByIdx(j, returnValues[index].get()); index++; } } returnValues[index]->Assign(argValues[i].get()); index++; } - args.GetReturnValue()->SetArray(iLength, returnValues); - for (int32_t i = 0; i < (int32_t)iLength; i++) - delete returnValues[i]; - - FX_Free(returnValues); + args.GetReturnValue()->SetArray(returnValues); } // static @@ -6010,13 +5980,14 @@ bool CXFA_FM2JSContext::simpleValueCompare(CFXJSE_Value* pThis, } // static -void CXFA_FM2JSContext::unfoldArgs(CFXJSE_Value* pThis, - CFXJSE_Arguments& args, - CFXJSE_Value**& resultValues, - int32_t& iCount, - int32_t iStart) { - iCount = 0; +void CXFA_FM2JSContext::unfoldArgs( + CFXJSE_Value* pThis, + CFXJSE_Arguments& args, + std::vector>* resultValues, + int32_t iStart) { + resultValues->clear(); + int32_t iCount = 0; v8::Isolate* pIsolate = ToJSContext(pThis, nullptr)->GetScriptRuntime(); int32_t argc = args.GetLength(); std::vector> argsValue; @@ -6031,9 +6002,9 @@ void CXFA_FM2JSContext::unfoldArgs(CFXJSE_Value* pThis, iCount += 1; } } - resultValues = FX_Alloc(CFXJSE_Value*, iCount); + for (int32_t i = 0; i < iCount; i++) - resultValues[i] = new CFXJSE_Value(pIsolate); + resultValues->push_back(pdfium::MakeUnique(pIsolate)); int32_t index = 0; for (int32_t i = 0; i < argc - iStart; i++) { @@ -6050,22 +6021,24 @@ void CXFA_FM2JSContext::unfoldArgs(CFXJSE_Value* pThis, if (propertyValue->IsNull()) { for (int32_t j = 2; j < iLength; j++) { argsValue[i]->GetObjectPropertyByIdx(j, jsObjectValue.get()); - GetObjectDefaultValue(jsObjectValue.get(), resultValues[index]); + GetObjectDefaultValue(jsObjectValue.get(), + (*resultValues)[index].get()); index++; } } else { for (int32_t j = 2; j < iLength; j++) { argsValue[i]->GetObjectPropertyByIdx(j, jsObjectValue.get()); jsObjectValue->GetObjectProperty( - propertyValue->ToString().AsStringC(), resultValues[index]); + propertyValue->ToString().AsStringC(), + (*resultValues)[index].get()); index++; } } } else if (argsValue[i]->IsObject()) { - GetObjectDefaultValue(argsValue[i].get(), resultValues[index]); + GetObjectDefaultValue(argsValue[i].get(), (*resultValues)[index].get()); index++; } else { - resultValues[index]->Assign(argsValue[i].get()); + (*resultValues)[index]->Assign(argsValue[i].get()); index++; } } @@ -6412,7 +6385,7 @@ CXFA_FM2JSContext::CXFA_FM2JSContext(v8::Isolate* pScriptIsolate, : m_pIsolate(pScriptIsolate), m_pFMClass( CFXJSE_Class::Create(pScriptContext, &formcalc_fm2js_descriptor)), - m_pValue(new CFXJSE_Value(pScriptIsolate)), + m_pValue(pdfium::MakeUnique(pScriptIsolate)), m_pDocument(pDoc) { m_pValue.get()->SetNull(); m_pValue.get()->SetObject(this, m_pFMClass); diff --git a/xfa/fxfa/fm2js/xfa_fm2jscontext.h b/xfa/fxfa/fm2js/xfa_fm2jscontext.h index 66e5dcce8b..0f802fe65e 100644 --- a/xfa/fxfa/fm2js/xfa_fm2jscontext.h +++ b/xfa/fxfa/fm2js/xfa_fm2jscontext.h @@ -401,11 +401,11 @@ class CXFA_FM2JSContext : public CFXJSE_HostObject { static bool simpleValueCompare(CFXJSE_Value* pThis, CFXJSE_Value* firstValue, CFXJSE_Value* secondValue); - static void unfoldArgs(CFXJSE_Value* pThis, - CFXJSE_Arguments& args, - CFXJSE_Value**& resultValues, - int32_t& iCount, - int32_t iStart = 0); + static void unfoldArgs( + CFXJSE_Value* pThis, + CFXJSE_Arguments& args, + std::vector>* resultValues, + int32_t iStart = 0); static void GetObjectDefaultValue(CFXJSE_Value* pObjectValue, CFXJSE_Value* pDefaultValue); static bool SetObjectDefaultValue(CFXJSE_Value* pObjectValue, -- cgit v1.2.3