From 54be7be992f1cde40b9d5c0e55a119c6cc4e4e02 Mon Sep 17 00:00:00 2001
From: weili <weili@chromium.org>
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(-)

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> CFXJSE_RuntimeData::Create(
+    v8::Isolate* pIsolate) {
+  std::unique_ptr<CFXJSE_RuntimeData> pRuntimeData(
+      new CFXJSE_RuntimeData(pIsolate));
   CFXJSE_ScopeUtil_IsolateHandle scope(pIsolate);
   v8::Local<v8::FunctionTemplate> 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 <memory>
+
 #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<CFXJSE_RuntimeData> 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 <v8.h>
 
 #include <map>
+#include <memory>
 #include <vector>
 
 #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<CFXJS_ObjDefinition*> m_ObjectDefnArray;
 #ifdef PDF_ENABLE_XFA
-  CFXJSE_RuntimeData* m_pFXJSERuntimeData;
+  std::unique_ptr<CFXJSE_RuntimeData> m_pFXJSERuntimeData;
 #endif  // PDF_ENABLE_XFA
   V8TemplateMap* m_pDynamicObjsMap;
 
-- 
cgit v1.2.3