From e3b782fe20282fd3ccde0ca8a47d8e7fcd9b8dcc Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 11 Jun 2018 17:48:17 +0000 Subject: 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 Reviewed-by: dsinclair --- fxjs/cfxjs_engine.cpp | 81 ++++++++++++++++++++++++--------------------------- fxjs/cfxjs_engine.h | 9 +++++- 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 GetInstanceTemplate() { v8::EscapableHandleScope scope(m_pIsolate); v8::Local function = @@ -182,9 +167,9 @@ class CFXJS_ObjDefinition { static v8::Local 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 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(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 fun = v8::FunctionTemplate::New( GetIsolate(), pMethodCall, v8::Local(), 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 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 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 pObj; if (pObjDef->m_ObjType == FXJSOBJTYPE_GLOBAL) { pObj = @@ -481,7 +478,6 @@ void CFXJS_Engine::ReleaseEngine() { pObj = v8::Local::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 CFXJS_Engine::NewFXJSBoundObject(int nObjDefnID, if (!pData) return v8::Local(); - if (nObjDefnID < 0 || nObjDefnID >= CFXJS_ObjDefinition::MaxID(GetIsolate())) + CFXJS_ObjDefinition* pObjDef = pData->ObjDefinitionForID(nObjDefnID); + if (!pObjDef) return v8::Local(); - CFXJS_ObjDefinition* pObjDef = - CFXJS_ObjDefinition::ForID(GetIsolate(), nObjDefnID); v8::Local obj; if (!pObjDef->GetInstanceTemplate()->NewInstance(context).ToLocal(&obj)) return v8::Local(); 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(m_ObjectDefnArray); + } + CFXJS_ObjDefinition* ObjDefinitionForID(int id) const; + int AssignIDForObjDefinition(std::unique_ptr pDefn); + std::vector> m_ObjectDefnArray; + std::unique_ptr m_pDynamicObjsMap; #ifdef PDF_ENABLE_XFA std::unique_ptr m_pFXJSERuntimeData; #endif // PDF_ENABLE_XFA - std::unique_ptr m_pDynamicObjsMap; protected: explicit FXJS_PerIsolateData(v8::Isolate* pIsolate); -- cgit v1.2.3