From a4ad5957af799374d4710f3847b85f57bea13f47 Mon Sep 17 00:00:00 2001
From: weili <weili@chromium.org>
Date: Thu, 22 Sep 2016 10:38:53 -0700
Subject: Simplify memory management for per isolate data

Use smart pointers for FXJS_PerIsolateData owned member variables. Also
move creation and deletion of dynamic object map into
FXJS_PerIsolateData's constructor and destructor. Overall, the
interfaces and memory management should be simpler.

BUG=pdfium:518

Review-Url: https://codereview.chromium.org/2358343002
---
 fxjs/fxjs_v8.cpp       | 21 +++++++--------------
 fxjs/include/fxjs_v8.h | 15 +++------------
 2 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp
index ebe38fdb7f..19197b9ffb 100644
--- a/fxjs/fxjs_v8.cpp
+++ b/fxjs/fxjs_v8.cpp
@@ -38,7 +38,7 @@ class CFXJS_ObjDefinition {
 
   static CFXJS_ObjDefinition* ForID(v8::Isolate* pIsolate, int id) {
     // Note: GetAt() halts if out-of-range even in release builds.
-    return FXJS_PerIsolateData::Get(pIsolate)->m_ObjectDefnArray[id];
+    return FXJS_PerIsolateData::Get(pIsolate)->m_ObjectDefnArray[id].get();
   }
 
   CFXJS_ObjDefinition(v8::Isolate* isolate,
@@ -70,7 +70,7 @@ class CFXJS_ObjDefinition {
 
   int AssignID() {
     FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(m_pIsolate);
-    pData->m_ObjectDefnArray.push_back(this);
+    pData->m_ObjectDefnArray.emplace_back(this);
     return pData->m_ObjectDefnArray.size() - 1;
   }
 
@@ -150,7 +150,7 @@ void V8TemplateMapTraits::Dispose(v8::Isolate* isolate,
 V8TemplateMapTraits::MapType* V8TemplateMapTraits::MapFromWeakCallbackInfo(
     const v8::WeakCallbackInfo<WeakCallbackDataType>& data) {
   V8TemplateMap* pMap =
-      (FXJS_PerIsolateData::Get(data.GetIsolate()))->m_pDynamicObjsMap;
+      (FXJS_PerIsolateData::Get(data.GetIsolate()))->m_pDynamicObjsMap.get();
   return pMap ? &pMap->m_map : nullptr;
 }
 
@@ -206,7 +206,7 @@ FXJS_PerIsolateData::~FXJS_PerIsolateData() {}
 // static
 void FXJS_PerIsolateData::SetUp(v8::Isolate* pIsolate) {
   if (!pIsolate->GetData(g_embedderDataSlot))
-    pIsolate->SetData(g_embedderDataSlot, new FXJS_PerIsolateData());
+    pIsolate->SetData(g_embedderDataSlot, new FXJS_PerIsolateData(pIsolate));
 }
 
 // static
@@ -215,7 +215,8 @@ FXJS_PerIsolateData* FXJS_PerIsolateData::Get(v8::Isolate* pIsolate) {
       pIsolate->GetData(g_embedderDataSlot));
 }
 
-FXJS_PerIsolateData::FXJS_PerIsolateData() : m_pDynamicObjsMap(nullptr) {}
+FXJS_PerIsolateData::FXJS_PerIsolateData(v8::Isolate* pIsolate)
+    : m_pDynamicObjsMap(new V8TemplateMap(pIsolate)) {}
 
 CFXJS_Engine::CFXJS_Engine() : m_isolate(nullptr) {}
 
@@ -380,10 +381,6 @@ void CFXJS_Engine::InitializeEngine() {
       v8::Context::New(m_isolate, nullptr, GetGlobalObjectTemplate(m_isolate));
   v8::Context::Scope context_scope(v8Context);
 
-  FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(m_isolate);
-  if (!pData)
-    return;
-  pData->CreateDynamicObjsMap(m_isolate);
   v8Context->SetAlignedPointerInEmbedderData(kPerContextDataIndex, this);
 
   int maxID = CFXJS_ObjDefinition::MaxID(m_isolate);
@@ -455,12 +452,8 @@ void CFXJS_Engine::ReleaseEngine() {
   if (m_isolate == g_isolate && --g_isolate_ref_count > 0)
     return;
 
-  pData->ReleaseDynamicObjsMap();
-  for (int i = 0; i < maxID; ++i)
-    delete CFXJS_ObjDefinition::ForID(m_isolate, i);
-
-  m_isolate->SetData(g_embedderDataSlot, nullptr);
   delete pData;
+  m_isolate->SetData(g_embedderDataSlot, nullptr);
 }
 
 int CFXJS_Engine::Execute(const CFX_WideString& script, FXJSErr* pError) {
diff --git a/fxjs/include/fxjs_v8.h b/fxjs/include/fxjs_v8.h
index 8b5fc8399a..c87d8a5873 100644
--- a/fxjs/include/fxjs_v8.h
+++ b/fxjs/include/fxjs_v8.h
@@ -100,23 +100,14 @@ class FXJS_PerIsolateData {
   static void SetUp(v8::Isolate* pIsolate);
   static FXJS_PerIsolateData* Get(v8::Isolate* pIsolate);
 
-  void CreateDynamicObjsMap(v8::Isolate* pIsolate) {
-    if (!m_pDynamicObjsMap)
-      m_pDynamicObjsMap = new V8TemplateMap(pIsolate);
-  }
-  void ReleaseDynamicObjsMap() {
-    delete m_pDynamicObjsMap;
-    m_pDynamicObjsMap = nullptr;
-  }
-
-  std::vector<CFXJS_ObjDefinition*> m_ObjectDefnArray;
+  std::vector<std::unique_ptr<CFXJS_ObjDefinition>> m_ObjectDefnArray;
 #ifdef PDF_ENABLE_XFA
   std::unique_ptr<CFXJSE_RuntimeData> m_pFXJSERuntimeData;
 #endif  // PDF_ENABLE_XFA
-  V8TemplateMap* m_pDynamicObjsMap;
+  std::unique_ptr<V8TemplateMap> m_pDynamicObjsMap;
 
  protected:
-  FXJS_PerIsolateData();
+  explicit FXJS_PerIsolateData(v8::Isolate* pIsolate);
 };
 
 class FXJS_ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
-- 
cgit v1.2.3