Track shared isolates better in FXJSE.

Fix the asymmetry between creating only some isolates and
releasing all of them, even the shared ones, by being more
careful not to trash those we didn't create.

Review-Url: https://codereview.chromium.org/2025193002

3 files changed, 47 insertions, 62 deletions

diff --git a/xfa/fxjse/include/fxjse.h b/xfa/fxjse/include/fxjse.h
index 4a85251c03..ab95a6d675 100644
--- a/xfa/fxjse/include/fxjse.h
+++ b/xfa/fxjse/include/fxjse.h
@@ -69,8 +69,8 @@ struct FXJSE_CLASS_DESCRIPTOR {
 void FXJSE_Initialize();
 void FXJSE_Finalize();
 
-v8::Isolate* FXJSE_Runtime_Create();
-void FXJSE_Runtime_Release(v8::Isolate* pIsolate, bool bOwnedRuntime);
+v8::Isolate* FXJSE_Runtime_Create_Own();
+void FXJSE_Runtime_Release(v8::Isolate* pIsolate);
 
 CFXJSE_Context* FXJSE_Context_Create(
     v8::Isolate* pIsolate,
diff --git a/xfa/fxjse/runtime.cpp b/xfa/fxjse/runtime.cpp
index 70d90c44fa..c3d12c3745 100644
--- a/xfa/fxjse/runtime.cpp
+++ b/xfa/fxjse/runtime.cpp
@@ -24,56 +24,49 @@ static void FXJSE_KillV8() {
 }
 
 void FXJSE_Initialize() {
-  if (!CFXJSE_RuntimeData::g_RuntimeList) {
-    CFXJSE_RuntimeData::g_RuntimeList = new CFXJSE_RuntimeList;
-  }
+  if (!CFXJSE_IsolateTracker::g_pInstance)
+    CFXJSE_IsolateTracker::g_pInstance = new CFXJSE_IsolateTracker;
+
   static FX_BOOL bV8Initialized = FALSE;
-  if (bV8Initialized) {
+  if (bV8Initialized)
     return;
-  }
+
   bV8Initialized = TRUE;
   atexit(FXJSE_KillV8);
 }
 
-static void FXJSE_Runtime_DisposeCallback(v8::Isolate* pIsolate) {
+static void FXJSE_Runtime_DisposeCallback(v8::Isolate* pIsolate, bool bOwned) {
   if (FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate)) {
     delete pData->m_pFXJSERuntimeData;
     pData->m_pFXJSERuntimeData = nullptr;
   }
-  pIsolate->Dispose();
+  if (bOwned)
+    pIsolate->Dispose();
 }
 
 void FXJSE_Finalize() {
-  if (CFXJSE_RuntimeData::g_RuntimeList) {
-    CFXJSE_RuntimeData::g_RuntimeList->RemoveAllRuntimes(
-        FXJSE_Runtime_DisposeCallback);
-    delete CFXJSE_RuntimeData::g_RuntimeList;
-    CFXJSE_RuntimeData::g_RuntimeList = NULL;
-  }
+  if (!CFXJSE_IsolateTracker::g_pInstance)
+    return;
+
+  CFXJSE_IsolateTracker::g_pInstance->RemoveAll(FXJSE_Runtime_DisposeCallback);
+  delete CFXJSE_IsolateTracker::g_pInstance;
+  CFXJSE_IsolateTracker::g_pInstance = nullptr;
 }
 
-v8::Isolate* FXJSE_Runtime_Create() {
+v8::Isolate* FXJSE_Runtime_Create_Own() {
   v8::Isolate::CreateParams params;
   params.array_buffer_allocator = new FXJSE_ArrayBufferAllocator();
   v8::Isolate* pIsolate = v8::Isolate::New(params);
-  ASSERT(pIsolate && CFXJSE_RuntimeData::g_RuntimeList);
-  CFXJSE_RuntimeData::g_RuntimeList->AppendRuntime(pIsolate);
+  ASSERT(pIsolate && CFXJSE_IsolateTracker::g_pInstance);
+  CFXJSE_IsolateTracker::g_pInstance->Append(pIsolate);
   return pIsolate;
 }
 
-void FXJSE_Runtime_Release(v8::Isolate* pIsolate, bool bOwnedRuntime) {
+void FXJSE_Runtime_Release(v8::Isolate* pIsolate) {
   if (!pIsolate)
     return;
-  if (bOwnedRuntime) {
-    ASSERT(CFXJSE_RuntimeData::g_RuntimeList);
-    CFXJSE_RuntimeData::g_RuntimeList->RemoveRuntime(
-        pIsolate, FXJSE_Runtime_DisposeCallback);
-  } else {
-    if (FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(pIsolate)) {
-      delete pData->m_pFXJSERuntimeData;
-      pData->m_pFXJSERuntimeData = nullptr;
-    }
-  }
+  CFXJSE_IsolateTracker::g_pInstance->Remove(pIsolate,
+                                             FXJSE_Runtime_DisposeCallback);
 }
 
 CFXJSE_RuntimeData* CFXJSE_RuntimeData::Create(v8::Isolate* pIsolate) {
@@ -97,27 +90,26 @@ CFXJSE_RuntimeData* CFXJSE_RuntimeData::Get(v8::Isolate* pIsolate) {
   return pData->m_pFXJSERuntimeData;
 }
 
-CFXJSE_RuntimeList* CFXJSE_RuntimeData::g_RuntimeList = nullptr;
+CFXJSE_IsolateTracker* CFXJSE_IsolateTracker::g_pInstance = nullptr;
 
-void CFXJSE_RuntimeList::AppendRuntime(v8::Isolate* pIsolate) {
-  m_RuntimeList.push_back(pIsolate);
+void CFXJSE_IsolateTracker::Append(v8::Isolate* pIsolate) {
+  m_OwnedIsolates.push_back(pIsolate);
 }
 
-void CFXJSE_RuntimeList::RemoveRuntime(
+void CFXJSE_IsolateTracker::Remove(
     v8::Isolate* pIsolate,
-    CFXJSE_RuntimeList::RuntimeDisposeCallback lpfnDisposeCallback) {
-  auto it = std::find(m_RuntimeList.begin(), m_RuntimeList.end(), pIsolate);
-  if (it != m_RuntimeList.end())
-    m_RuntimeList.erase(it);
-  if (lpfnDisposeCallback)
-    lpfnDisposeCallback(pIsolate);
+    CFXJSE_IsolateTracker::DisposeCallback lpfnDisposeCallback) {
+  auto it = std::find(m_OwnedIsolates.begin(), m_OwnedIsolates.end(), pIsolate);
+  bool bFound = it != m_OwnedIsolates.end();
+  if (bFound)
+    m_OwnedIsolates.erase(it);
+  lpfnDisposeCallback(pIsolate, bFound);
 }
 
-void CFXJSE_RuntimeList::RemoveAllRuntimes(
-    CFXJSE_RuntimeList::RuntimeDisposeCallback lpfnDisposeCallback) {
-  if (lpfnDisposeCallback) {
-    for (v8::Isolate* pIsolate : m_RuntimeList)
-      lpfnDisposeCallback(pIsolate);
-  }
-  m_RuntimeList.clear();
+void CFXJSE_IsolateTracker::RemoveAll(
+    CFXJSE_IsolateTracker::DisposeCallback lpfnDisposeCallback) {
+  for (v8::Isolate* pIsolate : m_OwnedIsolates)
+    lpfnDisposeCallback(pIsolate, true);
+
+  m_OwnedIsolates.clear();
 }
diff --git a/xfa/fxjse/runtime.h b/xfa/fxjse/runtime.h
index 1240089be4..b57efe1df7 100644
--- a/xfa/fxjse/runtime.h
+++ b/xfa/fxjse/runtime.h
@@ -15,39 +15,32 @@
 class CFXJSE_RuntimeList;
 
 class CFXJSE_RuntimeData {
- protected:
-  CFXJSE_RuntimeData(v8::Isolate* pIsolate) : m_pIsolate(pIsolate) {}
-
  public:
-  static CFXJSE_RuntimeData* Create(v8::Isolate* pIsolate);
   static CFXJSE_RuntimeData* Get(v8::Isolate* pIsolate);
 
- public:
   v8::Isolate* m_pIsolate;
   v8::Global<v8::FunctionTemplate> m_hRootContextGlobalTemplate;
   v8::Global<v8::Context> m_hRootContext;
 
- public:
-  static CFXJSE_RuntimeList* g_RuntimeList;
-
  protected:
+  static CFXJSE_RuntimeData* Create(v8::Isolate* pIsolate);
+  CFXJSE_RuntimeData(v8::Isolate* pIsolate) : m_pIsolate(pIsolate) {}
   CFXJSE_RuntimeData();
   CFXJSE_RuntimeData(const CFXJSE_RuntimeData&);
   CFXJSE_RuntimeData& operator=(const CFXJSE_RuntimeData&);
 };
 
-class CFXJSE_RuntimeList {
+class CFXJSE_IsolateTracker {
  public:
-  typedef void (*RuntimeDisposeCallback)(v8::Isolate*);
+  typedef void (*DisposeCallback)(v8::Isolate*, bool bOwnedIsolate);
+  static CFXJSE_IsolateTracker* g_pInstance;
 
- public:
-  void AppendRuntime(v8::Isolate* pIsolate);
-  void RemoveRuntime(v8::Isolate* pIsolate,
-                     RuntimeDisposeCallback lpfnDisposeCallback);
-  void RemoveAllRuntimes(RuntimeDisposeCallback lpfnDisposeCallback);
+  void Append(v8::Isolate* pIsolate);
+  void Remove(v8::Isolate* pIsolate, DisposeCallback lpfnDisposeCallback);
+  void RemoveAll(DisposeCallback lpfnDisposeCallback);
 
  protected:
-  std::vector<v8::Isolate*> m_RuntimeList;
+  std::vector<v8::Isolate*> m_OwnedIsolates;
 };
 
 #endif  // XFA_FXJSE_RUNTIME_H_