Get CJS_RuntimeFactory out of the CJS_GlobalData management business.
authorTom Sepez <tsepez@chromium.org>
Mon, 14 Sep 2015 21:32:33 +0000 (14:32 -0700)
committerTom Sepez <tsepez@chromium.org>
Mon, 14 Sep 2015 21:32:33 +0000 (14:32 -0700)
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
fpdfsdk/include/javascript/JS_GlobalData.h
fpdfsdk/src/javascript/JS_GlobalData.cpp
fpdfsdk/src/javascript/JS_Runtime.cpp
fpdfsdk/src/javascript/global.cpp

index 18ab34b..3524a5b 100644 (file)
 #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_
index 607eec7..98a963d 100644 (file)
@@ -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<CJS_GlobalData_Element*> m_arrayGlobalData;
   CFX_WideString m_sFilePath;
 };
index 0a21a92..8074918 100644 (file)
@@ -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(
index 4b18d80..8ba7e81 100644 (file)
@@ -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);
 }
index bde4e7d..e75dbd5 100644 (file)
@@ -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();
 }