Move some FXJS methods onto the per-isolate object.

This more clearly shows how information is flowing out of V8
and into our C++ callbacks.

Change-Id: I5c37d2c28c166443eb9983076fbb0e944bebbf47
Reviewed-on: https://pdfium-review.googlesource.com/34790
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

2 files changed, 46 insertions, 44 deletions

diff --git a/fxjs/cfxjs_engine.cpp b/fxjs/cfxjs_engine.cpp
index 763a3d9f14..f964c80e02 100644
--- a/fxjs/cfxjs_engine.cpp
+++ b/fxjs/cfxjs_engine.cpp
@@ -110,15 +110,6 @@ class CFXJS_PerObjectData {
 
 class CFXJS_ObjDefinition {
  public:
-  static int MaxID(v8::Isolate* pIsolate) {
-    return FXJS_PerIsolateData::Get(pIsolate)->m_ObjectDefnArray.size();
-  }
-
-  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].get();
-  }
-
   CFXJS_ObjDefinition(v8::Isolate* isolate,
                       const char* sObjName,
                       FXJSOBJTYPE eObjType,
@@ -152,12 +143,6 @@ class CFXJS_ObjDefinition {
     m_Signature.Reset(isolate, sig);
   }
 
-  int AssignID() {
-    FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(m_pIsolate);
-    pData->m_ObjectDefnArray.emplace_back(this);
-    return pData->m_ObjectDefnArray.size() - 1;
-  }
-
   v8::Local<v8::ObjectTemplate> GetInstanceTemplate() {
     v8::EscapableHandleScope scope(m_pIsolate);
     v8::Local<v8::FunctionTemplate> function =
@@ -182,9 +167,9 @@ class CFXJS_ObjDefinition {
 
 static v8::Local<v8::ObjectTemplate> GetGlobalObjectTemplate(
     v8::Isolate* pIsolate) {
-  int maxID = CFXJS_ObjDefinition::MaxID(pIsolate);
-  for (int i = 0; i < maxID; ++i) {
-    CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(pIsolate, i);
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(pIsolate);
+  for (int i = 0; i < pIsolateData->MaxObjDefinitionID(); ++i) {
+    CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(i);
     if (pObjDef->m_ObjType == FXJSOBJTYPE_GLOBAL)
       return pObjDef->GetInstanceTemplate();
   }
@@ -210,7 +195,8 @@ void V8TemplateMapTraits::Dispose(v8::Isolate* isolate,
   int id = CFXJS_Engine::GetObjDefnID(obj);
   if (id == -1)
     return;
-  CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(isolate, id);
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(isolate);
+  CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(id);
   if (!pObjDef)
     return;
   if (pObjDef->m_pDestructor)
@@ -285,6 +271,17 @@ FXJS_PerIsolateData* FXJS_PerIsolateData::Get(v8::Isolate* pIsolate) {
 FXJS_PerIsolateData::FXJS_PerIsolateData(v8::Isolate* pIsolate)
     : m_pDynamicObjsMap(new V8TemplateMap(pIsolate)) {}
 
+CFXJS_ObjDefinition* FXJS_PerIsolateData::ObjDefinitionForID(int id) const {
+  return (id >= 0 && id < MaxObjDefinitionID()) ? m_ObjectDefnArray[id].get()
+                                                : nullptr;
+}
+
+int FXJS_PerIsolateData::AssignIDForObjDefinition(
+    std::unique_ptr<CFXJS_ObjDefinition> pDefn) {
+  m_ObjectDefnArray.push_back(std::move(pDefn));
+  return m_ObjectDefnArray.size() - 1;
+}
+
 CFXJS_Engine::CFXJS_Engine() : CFX_V8(nullptr) {}
 
 CFXJS_Engine::CFXJS_Engine(v8::Isolate* pIsolate) : CFX_V8(pIsolate) {}
@@ -322,9 +319,10 @@ int CFXJS_Engine::DefineObj(const char* sObjName,
   v8::Isolate::Scope isolate_scope(GetIsolate());
   v8::HandleScope handle_scope(GetIsolate());
   FXJS_PerIsolateData::SetUp(GetIsolate());
-  CFXJS_ObjDefinition* pObjDef = new CFXJS_ObjDefinition(
-      GetIsolate(), sObjName, eObjType, pConstructor, pDestructor);
-  return pObjDef->AssignID();
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
+  return pIsolateData->AssignIDForObjDefinition(
+      pdfium::MakeUnique<CFXJS_ObjDefinition>(GetIsolate(), sObjName, eObjType,
+                                              pConstructor, pDestructor));
 }
 
 void CFXJS_Engine::DefineObjMethod(int nObjDefnID,
@@ -332,8 +330,8 @@ void CFXJS_Engine::DefineObjMethod(int nObjDefnID,
                                    v8::FunctionCallback pMethodCall) {
   v8::Isolate::Scope isolate_scope(GetIsolate());
   v8::HandleScope handle_scope(GetIsolate());
-  CFXJS_ObjDefinition* pObjDef =
-      CFXJS_ObjDefinition::ForID(GetIsolate(), nObjDefnID);
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
+  CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(nObjDefnID);
   v8::Local<v8::FunctionTemplate> fun = v8::FunctionTemplate::New(
       GetIsolate(), pMethodCall, v8::Local<v8::Value>(),
       pObjDef->GetSignature());
@@ -348,8 +346,8 @@ void CFXJS_Engine::DefineObjProperty(int nObjDefnID,
                                      v8::AccessorSetterCallback pPropPut) {
   v8::Isolate::Scope isolate_scope(GetIsolate());
   v8::HandleScope handle_scope(GetIsolate());
-  CFXJS_ObjDefinition* pObjDef =
-      CFXJS_ObjDefinition::ForID(GetIsolate(), nObjDefnID);
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
+  CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(nObjDefnID);
   pObjDef->GetInstanceTemplate()->SetAccessor(NewString(sPropName), pPropGet,
                                               pPropPut);
 }
@@ -362,8 +360,8 @@ void CFXJS_Engine::DefineObjAllProperties(
     v8::GenericNamedPropertyDeleterCallback pPropDel) {
   v8::Isolate::Scope isolate_scope(GetIsolate());
   v8::HandleScope handle_scope(GetIsolate());
-  CFXJS_ObjDefinition* pObjDef =
-      CFXJS_ObjDefinition::ForID(GetIsolate(), nObjDefnID);
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
+  CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(nObjDefnID);
   pObjDef->GetInstanceTemplate()->SetHandler(
       v8::NamedPropertyHandlerConfiguration(
           pPropGet, pPropPut, pPropQurey, pPropDel, nullptr,
@@ -376,8 +374,8 @@ void CFXJS_Engine::DefineObjConst(int nObjDefnID,
                                   v8::Local<v8::Value> pDefault) {
   v8::Isolate::Scope isolate_scope(GetIsolate());
   v8::HandleScope handle_scope(GetIsolate());
-  CFXJS_ObjDefinition* pObjDef =
-      CFXJS_ObjDefinition::ForID(GetIsolate(), nObjDefnID);
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
+  CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(nObjDefnID);
   pObjDef->GetInstanceTemplate()->Set(GetIsolate(), sConstName, pDefault);
 }
 
@@ -430,11 +428,11 @@ void CFXJS_Engine::InitializeEngine() {
   }
 
   v8::Context::Scope context_scope(v8Context);
-
-  int maxID = CFXJS_ObjDefinition::MaxID(GetIsolate());
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
+  int maxID = pIsolateData->MaxObjDefinitionID();
   m_StaticObjects.resize(maxID + 1);
   for (int i = 0; i < maxID; ++i) {
-    CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(GetIsolate(), i);
+    CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(i);
     if (pObjDef->m_ObjType == FXJSOBJTYPE_GLOBAL) {
       CFXJS_PerObjectData::SetInObject(new CFXJS_PerObjectData(i),
                                        v8Context->Global()
@@ -464,15 +462,14 @@ void CFXJS_Engine::ReleaseEngine() {
   v8::HandleScope handle_scope(GetIsolate());
   v8::Local<v8::Context> context = GetV8Context();
   v8::Context::Scope context_scope(context);
-  FXJS_PerIsolateData* pData = FXJS_PerIsolateData::Get(GetIsolate());
-  if (!pData)
+  FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
+  if (!pIsolateData)
     return;
 
   m_ConstArrays.clear();
 
-  int maxID = CFXJS_ObjDefinition::MaxID(GetIsolate());
-  for (int i = 0; i < maxID; ++i) {
-    CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(GetIsolate(), i);
+  for (int i = 0; i < pIsolateData->MaxObjDefinitionID(); ++i) {
+    CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(i);
     v8::Local<v8::Object> pObj;
     if (pObjDef->m_ObjType == FXJSOBJTYPE_GLOBAL) {
       pObj =
@@ -481,7 +478,6 @@ void CFXJS_Engine::ReleaseEngine() {
       pObj = v8::Local<v8::Object>::New(GetIsolate(), m_StaticObjects[i]);
       m_StaticObjects[i].Reset();
     }
-
     if (!pObj.IsEmpty()) {
       if (pObjDef->m_pDestructor)
         pObjDef->m_pDestructor(pObj);
@@ -494,7 +490,7 @@ void CFXJS_Engine::ReleaseEngine() {
   if (GetIsolate() == g_isolate && --g_isolate_ref_count > 0)
     return;
 
-  delete pData;
+  delete pIsolateData;
   GetIsolate()->SetData(g_embedderDataSlot, nullptr);
 }
 
@@ -533,11 +529,10 @@ v8::Local<v8::Object> CFXJS_Engine::NewFXJSBoundObject(int nObjDefnID,
   if (!pData)
     return v8::Local<v8::Object>();
 
-  if (nObjDefnID < 0 || nObjDefnID >= CFXJS_ObjDefinition::MaxID(GetIsolate()))
+  CFXJS_ObjDefinition* pObjDef = pData->ObjDefinitionForID(nObjDefnID);
+  if (!pObjDef)
     return v8::Local<v8::Object>();
 
-  CFXJS_ObjDefinition* pObjDef =
-      CFXJS_ObjDefinition::ForID(GetIsolate(), nObjDefnID);
   v8::Local<v8::Object> obj;
   if (!pObjDef->GetInstanceTemplate()->NewInstance(context).ToLocal(&obj))
     return v8::Local<v8::Object>();
diff --git a/fxjs/cfxjs_engine.h b/fxjs/cfxjs_engine.h
index a5f94767c5..a940f3fc46 100644
--- a/fxjs/cfxjs_engine.h
+++ b/fxjs/cfxjs_engine.h
@@ -22,6 +22,7 @@
 #include "core/fxcrt/fx_string.h"
 #include "fxjs/cfx_v8.h"
 #include "fxjs/ijs_runtime.h"
+#include "third_party/base/stl_util.h"
 #include "v8/include/v8-util.h"
 #include "v8/include/v8.h"
 
@@ -51,11 +52,17 @@ class FXJS_PerIsolateData {
   static void SetUp(v8::Isolate* pIsolate);
   static FXJS_PerIsolateData* Get(v8::Isolate* pIsolate);
 
+  int MaxObjDefinitionID() const {
+    return pdfium::CollectionSize<int>(m_ObjectDefnArray);
+  }
+  CFXJS_ObjDefinition* ObjDefinitionForID(int id) const;
+  int AssignIDForObjDefinition(std::unique_ptr<CFXJS_ObjDefinition> pDefn);
+
   std::vector<std::unique_ptr<CFXJS_ObjDefinition>> m_ObjectDefnArray;
+  std::unique_ptr<V8TemplateMap> m_pDynamicObjsMap;
 #ifdef PDF_ENABLE_XFA
   std::unique_ptr<CFXJSE_RuntimeData> m_pFXJSERuntimeData;
 #endif  // PDF_ENABLE_XFA
-  std::unique_ptr<V8TemplateMap> m_pDynamicObjsMap;
 
  protected:
   explicit FXJS_PerIsolateData(v8::Isolate* pIsolate);