From 54be7be992f1cde40b9d5c0e55a119c6cc4e4e02 Mon Sep 17 00:00:00 2001 From: weili Date: Wed, 21 Sep 2016 10:19:50 -0700 Subject: Fix leaks related to the usage of JSE runtime data Per isolate runtime data should be deleted when the associated isolate's destructed. Also, the internal of per isolate runtime data is obscure to the JS engine. So XFA or this class itself has to be in charge of the memory management. Use smart pointer for it so that the resource could be released properly. BUG=pdfium:242 Review-Url: https://codereview.chromium.org/2354923003 --- fxjs/cfxjse_runtimedata.cpp | 14 +++++++------- fxjs/cfxjse_runtimedata.h | 4 +++- fxjs/fxjs_v8.cpp | 5 ----- fxjs/include/fxjs_v8.h | 14 +++++++------- 4 files changed, 17 insertions(+), 20 deletions(-) (limited to 'fxjs') diff --git a/fxjs/cfxjse_runtimedata.cpp b/fxjs/cfxjse_runtimedata.cpp index 2c9379526e..021fb1dbe4 100644 --- a/fxjs/cfxjse_runtimedata.cpp +++ b/fxjs/cfxjse_runtimedata.cpp @@ -20,10 +20,8 @@ class FXJSE_ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { }; void Runtime_DisposeCallback(v8::Isolate* pIsolate, bool bOwned) { - if (FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate)) { - delete pData->m_pFXJSERuntimeData; - pData->m_pFXJSERuntimeData = nullptr; - } + if (FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate)) + delete pData; if (bOwned) pIsolate->Dispose(); } @@ -77,8 +75,10 @@ CFXJSE_RuntimeData::CFXJSE_RuntimeData(v8::Isolate* pIsolate) CFXJSE_RuntimeData::~CFXJSE_RuntimeData() {} -CFXJSE_RuntimeData* CFXJSE_RuntimeData::Create(v8::Isolate* pIsolate) { - CFXJSE_RuntimeData* pRuntimeData = new CFXJSE_RuntimeData(pIsolate); +std::unique_ptr CFXJSE_RuntimeData::Create( + v8::Isolate* pIsolate) { + std::unique_ptr pRuntimeData( + new CFXJSE_RuntimeData(pIsolate)); CFXJSE_ScopeUtil_IsolateHandle scope(pIsolate); v8::Local hFuncTemplate = v8::FunctionTemplate::New(pIsolate); @@ -101,7 +101,7 @@ CFXJSE_RuntimeData* CFXJSE_RuntimeData::Get(v8::Isolate* pIsolate) { FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate); if (!pData->m_pFXJSERuntimeData) pData->m_pFXJSERuntimeData = CFXJSE_RuntimeData::Create(pIsolate); - return pData->m_pFXJSERuntimeData; + return pData->m_pFXJSERuntimeData.get(); } CFXJSE_IsolateTracker* CFXJSE_IsolateTracker::g_pInstance = nullptr; diff --git a/fxjs/cfxjse_runtimedata.h b/fxjs/cfxjse_runtimedata.h index 34f8f32d92..292fe26ae3 100644 --- a/fxjs/cfxjse_runtimedata.h +++ b/fxjs/cfxjse_runtimedata.h @@ -7,6 +7,8 @@ #ifndef FXJS_CFXJSE_RUNTIMEDATA_H_ #define FXJS_CFXJSE_RUNTIMEDATA_H_ +#include + #include "v8/include/v8.h" class CFXJSE_RuntimeList; @@ -24,7 +26,7 @@ class CFXJSE_RuntimeData { protected: explicit CFXJSE_RuntimeData(v8::Isolate* pIsolate); - static CFXJSE_RuntimeData* Create(v8::Isolate* pIsolate); + static std::unique_ptr Create(v8::Isolate* pIsolate); private: CFXJSE_RuntimeData(const CFXJSE_RuntimeData&) = delete; diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp index 53f2527ad6..5304254242 100644 --- a/fxjs/fxjs_v8.cpp +++ b/fxjs/fxjs_v8.cpp @@ -215,12 +215,7 @@ FXJS_PerIsolateData* FXJS_PerIsolateData::Get(v8::Isolate* pIsolate) { pIsolate->GetData(g_embedderDataSlot)); } -#ifndef PDF_ENABLE_XFA FXJS_PerIsolateData::FXJS_PerIsolateData() : m_pDynamicObjsMap(nullptr) {} -#else // PDF_ENABLE_XFA -FXJS_PerIsolateData::FXJS_PerIsolateData() - : m_pFXJSERuntimeData(nullptr), m_pDynamicObjsMap(nullptr) {} -#endif // PDF_ENABLE_XFA CFXJS_Engine::CFXJS_Engine() : m_isolate(nullptr) {} diff --git a/fxjs/include/fxjs_v8.h b/fxjs/include/fxjs_v8.h index 7c4a9070f8..c08cc8b7ad 100644 --- a/fxjs/include/fxjs_v8.h +++ b/fxjs/include/fxjs_v8.h @@ -18,9 +18,15 @@ #include #include +#include #include #include "core/fxcrt/include/fx_string.h" +#ifdef PDF_ENABLE_XFA +// Header for CFXJSE_RuntimeData. FXJS_V8 doesn't interpret this class, +// it is just passed along to XFA. +#include "fxjs/cfxjse_runtimedata.h" +#endif // PDF_ENABLE_XFA class CFXJS_Engine; class CFXJS_ObjDefinition; @@ -29,12 +35,6 @@ class CFXJS_ObjDefinition; // on to caller-provided methods. class IJS_Context; // A description of the event that caused JS execution. -#ifdef PDF_ENABLE_XFA -// FXJS_V8 places no interpreation on this calass; it merely passes it -// along to XFA. -class CFXJSE_RuntimeData; -#endif // PDF_ENABLE_XFA - enum FXJSOBJTYPE { FXJSOBJTYPE_DYNAMIC = 0, // Created by native method and returned to JS. FXJSOBJTYPE_STATIC, // Created by init and hung off of global object. @@ -111,7 +111,7 @@ class FXJS_PerIsolateData { std::vector m_ObjectDefnArray; #ifdef PDF_ENABLE_XFA - CFXJSE_RuntimeData* m_pFXJSERuntimeData; + std::unique_ptr m_pFXJSERuntimeData; #endif // PDF_ENABLE_XFA V8TemplateMap* m_pDynamicObjsMap; -- cgit v1.2.3