From 9a817f092e6800e9338b41acf4ea0416b6ad4467 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 14 Sep 2015 14:32:33 -0700 Subject: Get CJS_RuntimeFactory out of the CJS_GlobalData management business. First part of getting rid of CJS_RuntimeFactory. The factory design pattern isn't appropriate here since we only ever make one kind of object. CJS_GlobalData is now perfectly capable of managing itself through internal ref counts. I'm philosophically opposed to keeping ref-counts outside the object (do you hear me std::shared_ptr, you're bad!) R=thestig@chromium.org Review URL: https://codereview.chromium.org/1338993005 . --- fpdfsdk/include/javascript/IJavaScript.h | 17 ++++--------- fpdfsdk/include/javascript/JS_GlobalData.h | 11 ++++++--- fpdfsdk/src/javascript/JS_GlobalData.cpp | 39 +++++++++++++++++------------- fpdfsdk/src/javascript/JS_Runtime.cpp | 20 --------------- fpdfsdk/src/javascript/global.cpp | 4 +-- 5 files changed, 37 insertions(+), 54 deletions(-) diff --git a/fpdfsdk/include/javascript/IJavaScript.h b/fpdfsdk/include/javascript/IJavaScript.h index 18ab34b8cb..3524a5b8ff 100644 --- a/fpdfsdk/include/javascript/IJavaScript.h +++ b/fpdfsdk/include/javascript/IJavaScript.h @@ -10,10 +10,11 @@ #include "../../../core/include/fxcrt/fx_string.h" #include "../../../core/include/fxcrt/fx_system.h" -class CPDF_Bookmark; -class CPDF_FormField; +class CPDFDoc_Environment; class CPDFSDK_Annot; class CPDFSDK_Document; +class CPDF_Bookmark; +class CPDF_FormField; class IFXJS_Context { public: @@ -143,27 +144,19 @@ class IFXJS_Runtime { virtual ~IFXJS_Runtime() {} }; -class CPDFDoc_Environment; -class CJS_GlobalData; - class CJS_RuntimeFactory { public: - CJS_RuntimeFactory() - : m_bInit(FALSE), m_nRef(0), m_pGlobalData(NULL), m_nGlobalDataCount(0) {} + CJS_RuntimeFactory() : m_bInit(FALSE), m_nRef(0) {} ~CJS_RuntimeFactory(); + IFXJS_Runtime* NewJSRuntime(CPDFDoc_Environment* pApp); void DeleteJSRuntime(IFXJS_Runtime* pRuntime); void AddRef(); void Release(); - CJS_GlobalData* NewGlobalData(CPDFDoc_Environment* pApp); - void ReleaseGlobalData(); - private: FX_BOOL m_bInit; int m_nRef; - CJS_GlobalData* m_pGlobalData; - int32_t m_nGlobalDataCount; }; #endif // FPDFSDK_INCLUDE_JAVASCRIPT_IJAVASCRIPT_H_ diff --git a/fpdfsdk/include/javascript/JS_GlobalData.h b/fpdfsdk/include/javascript/JS_GlobalData.h index 607eec79bd..98a963d87b 100644 --- a/fpdfsdk/include/javascript/JS_GlobalData.h +++ b/fpdfsdk/include/javascript/JS_GlobalData.h @@ -58,10 +58,9 @@ class CJS_GlobalData_Element { class CJS_GlobalData { public: - CJS_GlobalData(CPDFDoc_Environment* pApp); - virtual ~CJS_GlobalData(); + static CJS_GlobalData* GetRetainedInstance(CPDFDoc_Environment* pApp); + void Release(); - public: void SetGlobalVariableNumber(const FX_CHAR* propname, double dData); void SetGlobalVariableBoolean(const FX_CHAR* propname, bool bData); void SetGlobalVariableString(const FX_CHAR* propname, @@ -78,6 +77,11 @@ class CJS_GlobalData { CJS_GlobalData_Element* GetAt(int index) const; private: + static CJS_GlobalData* g_Instance; + + CJS_GlobalData(CPDFDoc_Environment* pApp); + ~CJS_GlobalData(); + void LoadGlobalPersistentVariables(); void SaveGlobalPersisitentVariables(); @@ -95,6 +99,7 @@ class CJS_GlobalData { CFX_BinaryBuf& sData); private: + size_t m_RefCount; CFX_ArrayTemplate m_arrayGlobalData; CFX_WideString m_sFilePath; }; diff --git a/fpdfsdk/src/javascript/JS_GlobalData.cpp b/fpdfsdk/src/javascript/JS_GlobalData.cpp index 0a21a9253c..807491854a 100644 --- a/fpdfsdk/src/javascript/JS_GlobalData.cpp +++ b/fpdfsdk/src/javascript/JS_GlobalData.cpp @@ -99,19 +99,31 @@ static const uint8_t JS_RC4KEY[] = { 0x0e, 0xd0, 0x6b, 0xbb, 0xd5, 0x75, 0x55, 0x8b, 0x6e, 0x6b, 0x19, 0xa0, 0xf8, 0x77, 0xd5, 0xa3}; -CJS_GlobalData::CJS_GlobalData(CPDFDoc_Environment* pApp) { - // IBaseAnnot* pBaseAnnot = IBaseAnnot::GetBaseAnnot(m_pApp); - // ASSERT(pBaseAnnot != NULL); - // - // m_sFilePath = pBaseAnnot->GetUserPath(); - m_sFilePath += SDK_JS_GLOBALDATA_FILENAME; +CJS_GlobalData* CJS_GlobalData::g_Instance = nullptr; + +// static +CJS_GlobalData* CJS_GlobalData::GetRetainedInstance(CPDFDoc_Environment* pApp) { + if (!g_Instance) { + g_Instance = new CJS_GlobalData(pApp); + } + ++g_Instance->m_RefCount; + return g_Instance; +} +void CJS_GlobalData::Release() { + if (!--m_RefCount) { + delete g_Instance; + g_Instance = nullptr; + } +} + +CJS_GlobalData::CJS_GlobalData(CPDFDoc_Environment* pApp) : m_RefCount(0) { + m_sFilePath += SDK_JS_GLOBALDATA_FILENAME; LoadGlobalPersistentVariables(); } CJS_GlobalData::~CJS_GlobalData() { SaveGlobalPersisitentVariables(); - for (int i = 0, sz = m_arrayGlobalData.GetSize(); i < sz; i++) delete m_arrayGlobalData.GetAt(i); @@ -119,19 +131,12 @@ CJS_GlobalData::~CJS_GlobalData() { } int CJS_GlobalData::FindGlobalVariable(const FX_CHAR* propname) { - ASSERT(propname != NULL); - - int nRet = -1; - for (int i = 0, sz = m_arrayGlobalData.GetSize(); i < sz; i++) { CJS_GlobalData_Element* pTemp = m_arrayGlobalData.GetAt(i); - if (pTemp->data.sKey[0] == *propname && pTemp->data.sKey == propname) { - nRet = i; - break; - } + if (pTemp->data.sKey[0] == *propname && pTemp->data.sKey == propname) + return i; } - - return nRet; + return -1; } CJS_GlobalData_Element* CJS_GlobalData::GetGlobalVariable( diff --git a/fpdfsdk/src/javascript/JS_Runtime.cpp b/fpdfsdk/src/javascript/JS_Runtime.cpp index 4b18d80f6c..8ba7e81654 100644 --- a/fpdfsdk/src/javascript/JS_Runtime.cpp +++ b/fpdfsdk/src/javascript/JS_Runtime.cpp @@ -50,7 +50,6 @@ void CJS_RuntimeFactory::Release() { // to do.Should be implemented as atom manipulation. if (--m_nRef == 0) { JS_Release(); - ReleaseGlobalData(); m_bInit = FALSE; } } @@ -60,25 +59,6 @@ void CJS_RuntimeFactory::DeleteJSRuntime(IFXJS_Runtime* pRuntime) { delete (CJS_Runtime*)pRuntime; } -CJS_GlobalData* CJS_RuntimeFactory::NewGlobalData(CPDFDoc_Environment* pApp) { - if (m_pGlobalData) { - m_nGlobalDataCount++; - return m_pGlobalData; - } - m_nGlobalDataCount = 1; - m_pGlobalData = new CJS_GlobalData(pApp); - return m_pGlobalData; -} - -void CJS_RuntimeFactory::ReleaseGlobalData() { - m_nGlobalDataCount--; - - if (m_nGlobalDataCount <= 0) { - delete m_pGlobalData; - m_pGlobalData = NULL; - } -} - void* CJS_ArrayBufferAllocator::Allocate(size_t length) { return calloc(1, length); } diff --git a/fpdfsdk/src/javascript/global.cpp b/fpdfsdk/src/javascript/global.cpp index bde4e7d5ae..e75dbd5585 100644 --- a/fpdfsdk/src/javascript/global.cpp +++ b/fpdfsdk/src/javascript/global.cpp @@ -108,12 +108,12 @@ JSGlobalAlternate::JSGlobalAlternate(CJS_Object* pJSObject) JSGlobalAlternate::~JSGlobalAlternate() { DestroyGlobalPersisitentVariables(); - m_pApp->GetRuntimeFactory()->ReleaseGlobalData(); + m_pGlobalData->Release(); } void JSGlobalAlternate::Initial(CPDFDoc_Environment* pApp) { m_pApp = pApp; - m_pGlobalData = pApp->GetRuntimeFactory()->NewGlobalData(pApp); + m_pGlobalData = CJS_GlobalData::GetRetainedInstance(pApp); UpdateGlobalPersistentVariables(); } -- cgit v1.2.3