From a4941914bb4411dc4e9053cb344e0349db388007 Mon Sep 17 00:00:00 2001 From: tsepez Date: Mon, 15 Aug 2016 11:40:12 -0700 Subject: Move some v8 objects from CJS back into FXJS Create a new class to hold these, CFXJS_Engine (could have been called Runtime, but there are too many "Runtimes" already). In a subsequent patch, all the FXJS_*() functions that take an isolate as the first argument can become methods on the engine. CJS_ must still manage the isolates; this happens outside the engine. The IJS_Runtime abstraction moves up to fpdfsdk/javascript; it remains to allow for either a real JS library or a stubb one to be linked (for non-js builds). Review-Url: https://codereview.chromium.org/2241483004 --- fpdfsdk/javascript/Consts.cpp | 2 +- fpdfsdk/javascript/JS_Define.h | 23 +++++++++++---------- fpdfsdk/javascript/cjs_runtime.cpp | 11 +++++----- fpdfsdk/javascript/cjs_runtime.h | 10 +++------ fxjs/fxjs_v8.cpp | 39 +++++++++++++++++++---------------- fxjs/fxjs_v8_embeddertest.cpp | 12 +++++------ fxjs/include/fxjs_v8.h | 42 +++++++++++++++++++++++++------------- testing/js_embedder_test.cpp | 6 +++--- 8 files changed, 78 insertions(+), 67 deletions(-) diff --git a/fpdfsdk/javascript/Consts.cpp b/fpdfsdk/javascript/Consts.cpp index b71d0a34b3..19988a61ea 100644 --- a/fpdfsdk/javascript/Consts.cpp +++ b/fpdfsdk/javascript/Consts.cpp @@ -148,7 +148,7 @@ void CJS_GlobalConsts::DefineJSObjects(CJS_Runtime* pRuntime) { (rt)->GetIsolate(), (name), \ [](const v8::FunctionCallbackInfo& info) { \ CJS_Runtime* pLocalRuntime = static_cast( \ - FXJS_GetRuntimeFromIsolate(info.GetIsolate())); \ + FXJS_GetCurrentEngineFromIsolate(info.GetIsolate())); \ if (pLocalRuntime) \ info.GetReturnValue().Set(pLocalRuntime->GetConstArray(name)); \ }); \ diff --git a/fpdfsdk/javascript/JS_Define.h b/fpdfsdk/javascript/JS_Define.h index 4c24a4de7b..b9ae1408f6 100644 --- a/fpdfsdk/javascript/JS_Define.h +++ b/fpdfsdk/javascript/JS_Define.h @@ -78,7 +78,7 @@ void JSPropGetter(const char* prop_name_string, const v8::PropertyCallbackInfo& info) { v8::Isolate* isolate = info.GetIsolate(); CJS_Runtime* pRuntime = - static_cast(FXJS_GetRuntimeFromIsolate(isolate)); + static_cast(FXJS_GetCurrentEngineFromIsolate(isolate)); if (!pRuntime) return; IJS_Context* pContext = pRuntime->GetCurrentContext(); @@ -104,7 +104,7 @@ void JSPropSetter(const char* prop_name_string, const v8::PropertyCallbackInfo& info) { v8::Isolate* isolate = info.GetIsolate(); CJS_Runtime* pRuntime = - static_cast(FXJS_GetRuntimeFromIsolate(isolate)); + static_cast(FXJS_GetCurrentEngineFromIsolate(isolate)); if (!pRuntime) return; IJS_Context* pContext = pRuntime->GetCurrentContext(); @@ -143,7 +143,7 @@ void JSMethod(const char* method_name_string, const v8::FunctionCallbackInfo& info) { v8::Isolate* isolate = info.GetIsolate(); CJS_Runtime* pRuntime = - static_cast(FXJS_GetRuntimeFromIsolate(isolate)); + static_cast(FXJS_GetCurrentEngineFromIsolate(isolate)); if (!pRuntime) return; IJS_Context* pContext = pRuntime->GetCurrentContext(); @@ -245,7 +245,7 @@ void JSMethod(const char* method_name_string, } #define DECLARE_JS_CLASS_RICH_PART() \ - static void JSConstructor(IJS_Runtime* pRuntime, v8::Local obj); \ + static void JSConstructor(CFXJS_Engine* pEngine, v8::Local obj); \ static void JSDestructor(v8::Local obj); \ static void DefineProps(v8::Isolate* pIsoalte); \ static void DefineMethods(v8::Isolate* pIsoalte); \ @@ -254,12 +254,12 @@ void JSMethod(const char* method_name_string, #define IMPLEMENT_JS_CLASS_RICH_PART(js_class_name, class_alternate, \ class_name) \ - void js_class_name::JSConstructor(IJS_Runtime* pIRuntime, \ + void js_class_name::JSConstructor(CFXJS_Engine* pEngine, \ v8::Local obj) { \ CJS_Object* pObj = new js_class_name(obj); \ pObj->SetEmbedObject(new class_alternate(pObj)); \ FXJS_SetPrivate(nullptr, obj, (void*)pObj); \ - pObj->InitInstance(pIRuntime); \ + pObj->InitInstance(static_cast(pEngine)); \ } \ void js_class_name::JSDestructor(v8::Local obj) { \ js_class_name* pObj = (js_class_name*)FXJS_GetPrivate(nullptr, obj); \ @@ -366,7 +366,7 @@ void JSSpecialPropGet(const char* class_name, const v8::PropertyCallbackInfo& info) { v8::Isolate* isolate = info.GetIsolate(); CJS_Runtime* pRuntime = - static_cast(FXJS_GetRuntimeFromIsolate(isolate)); + static_cast(FXJS_GetCurrentEngineFromIsolate(isolate)); if (!pRuntime) return; IJS_Context* pContext = pRuntime->GetCurrentContext(); @@ -393,7 +393,7 @@ void JSSpecialPropPut(const char* class_name, const v8::PropertyCallbackInfo& info) { v8::Isolate* isolate = info.GetIsolate(); CJS_Runtime* pRuntime = - static_cast(FXJS_GetRuntimeFromIsolate(isolate)); + static_cast(FXJS_GetCurrentEngineFromIsolate(isolate)); if (!pRuntime) return; IJS_Context* pContext = pRuntime->GetCurrentContext(); @@ -416,7 +416,8 @@ void JSSpecialPropDel(const char* class_name, v8::Local property, const v8::PropertyCallbackInfo& info) { v8::Isolate* isolate = info.GetIsolate(); - IJS_Runtime* pRuntime = FXJS_GetRuntimeFromIsolate(isolate); + CJS_Runtime* pRuntime = + static_cast(FXJS_GetCurrentEngineFromIsolate(isolate)); if (!pRuntime) return; IJS_Context* pContext = pRuntime->GetCurrentContext(); @@ -440,8 +441,8 @@ template void JSGlobalFunc(const char* func_name_string, const v8::FunctionCallbackInfo& info) { - CJS_Runtime* pRuntime = - static_cast(FXJS_GetRuntimeFromIsolate(info.GetIsolate())); + CJS_Runtime* pRuntime = static_cast( + FXJS_GetCurrentEngineFromIsolate(info.GetIsolate())); if (!pRuntime) return; IJS_Context* pContext = pRuntime->GetCurrentContext(); diff --git a/fpdfsdk/javascript/cjs_runtime.cpp b/fpdfsdk/javascript/cjs_runtime.cpp index 7eaf1451ca..36d888a2ea 100644 --- a/fpdfsdk/javascript/cjs_runtime.cpp +++ b/fpdfsdk/javascript/cjs_runtime.cpp @@ -58,8 +58,7 @@ CJS_Runtime* CJS_Runtime::FromContext(const IJS_Context* cc) { CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) : m_pApp(pApp), m_pDocument(nullptr), - m_bBlocking(FALSE), - m_isolate(nullptr), + m_bBlocking(false), m_isolateManaged(false) { #ifndef PDF_ENABLE_XFA IPDF_JSPLATFORM* pPlatform = m_pApp->GetFormFillInfo()->m_pJsPlatform; @@ -96,7 +95,7 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) v8::HandleScope handle_scope(isolate); if (CPDFXFA_App::GetInstance()->IsJavaScriptInitialized()) { CJS_Context* pContext = (CJS_Context*)NewContext(); - FXJS_InitializeRuntime(GetIsolate(), this, &m_context, &m_StaticObjects); + FXJS_InitializeEngine(GetIsolate(), this, &m_context, &m_StaticObjects); ReleaseContext(pContext); return; } @@ -110,7 +109,7 @@ CJS_Runtime::CJS_Runtime(CPDFDoc_Environment* pApp) #endif CJS_Context* pContext = (CJS_Context*)NewContext(); - FXJS_InitializeRuntime(GetIsolate(), this, &m_context, &m_StaticObjects); + FXJS_InitializeEngine(GetIsolate(), this, &m_context, &m_StaticObjects); ReleaseContext(pContext); } @@ -120,7 +119,7 @@ CJS_Runtime::~CJS_Runtime() { m_ContextArray.clear(); m_ConstArrays.clear(); - FXJS_ReleaseRuntime(GetIsolate(), &m_context, &m_StaticObjects); + FXJS_ReleaseEngine(GetIsolate(), &m_context, &m_StaticObjects); m_context.Reset(); if (m_isolateManaged) m_isolate->Dispose(); @@ -284,7 +283,7 @@ FX_BOOL CJS_Runtime::GetValueByName(const CFX_ByteStringC& utf8Name, // Do so now. // TODO(tsepez): redesign PDF-side objects to not rely on v8::Context's // embedder data slots, and/or to always use the right context. - FXJS_SetRuntimeForV8Context(old_context, this); + FXJS_SetEngineForV8Context(old_context, this); v8::Local propvalue = context->Global()->Get(v8::String::NewFromUtf8( diff --git a/fpdfsdk/javascript/cjs_runtime.h b/fpdfsdk/javascript/cjs_runtime.h index c43a34b720..16432d5394 100644 --- a/fpdfsdk/javascript/cjs_runtime.h +++ b/fpdfsdk/javascript/cjs_runtime.h @@ -20,7 +20,7 @@ class CJS_Context; -class CJS_Runtime : public IJS_Runtime { +class CJS_Runtime : public IJS_Runtime, public CFXJS_Engine { public: class Observer { public: @@ -77,13 +77,9 @@ class CJS_Runtime : public IJS_Runtime { std::vector> m_ContextArray; CPDFDoc_Environment* const m_pApp; CPDFSDK_Document* m_pDocument; - FX_BOOL m_bBlocking; - std::set m_FieldEventSet; - v8::Isolate* m_isolate; + bool m_bBlocking; bool m_isolateManaged; - v8::Global m_context; - std::vector*> m_StaticObjects; - std::map> m_ConstArrays; + std::set m_FieldEventSet; std::set m_observers; }; diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp index 92207fdd47..42fc021c83 100644 --- a/fxjs/fxjs_v8.cpp +++ b/fxjs/fxjs_v8.cpp @@ -222,6 +222,9 @@ FXJS_PerIsolateData::FXJS_PerIsolateData() : m_pFXJSERuntimeData(nullptr), m_pDynamicObjsMap(nullptr) {} #endif // PDF_ENABLE_XFA +CFXJS_Engine::CFXJS_Engine() : m_isolate(nullptr) {} +CFXJS_Engine::~CFXJS_Engine() {} + int FXJS_DefineObj(v8::Isolate* pIsolate, const wchar_t* sObjName, FXJSOBJTYPE eObjType, @@ -330,9 +333,9 @@ void FXJS_DefineGlobalConst(v8::Isolate* pIsolate, fun); } -void FXJS_InitializeRuntime( +void FXJS_InitializeEngine( v8::Isolate* pIsolate, - IJS_Runtime* pIRuntime, + CFXJS_Engine* pEngine, v8::Global* pV8PersistentContext, std::vector*>* pStaticObjects) { if (pIsolate == g_isolate) @@ -353,7 +356,7 @@ void FXJS_InitializeRuntime( if (!pData) return; pData->CreateDynamicObjsMap(pIsolate); - v8Context->SetAlignedPointerInEmbedderData(kPerContextDataIndex, pIRuntime); + v8Context->SetAlignedPointerInEmbedderData(kPerContextDataIndex, pEngine); int maxID = CFXJS_ObjDefinition::MaxID(pIsolate); pStaticObjects->resize(maxID + 1); @@ -367,10 +370,10 @@ void FXJS_InitializeRuntime( ->SetAlignedPointerInInternalField(0, new CFXJS_PerObjectData(i)); if (pObjDef->m_pConstructor) - pObjDef->m_pConstructor(pIRuntime, v8Context->Global() - ->GetPrototype() - ->ToObject(v8Context) - .ToLocalChecked()); + pObjDef->m_pConstructor(pEngine, v8Context->Global() + ->GetPrototype() + ->ToObject(v8Context) + .ToLocalChecked()); } else if (pObjDef->m_ObjType == FXJSOBJTYPE_STATIC) { CFX_ByteString bs = CFX_WideString(pObjDef->m_ObjName).UTF8Encode(); v8::Local m_ObjName = @@ -379,7 +382,7 @@ void FXJS_InitializeRuntime( .ToLocalChecked(); v8::Local obj = - FXJS_NewFxDynamicObj(pIsolate, pIRuntime, i, true); + FXJS_NewFxDynamicObj(pIsolate, pEngine, i, true); v8Context->Global()->Set(v8Context, m_ObjName, obj).FromJust(); pStaticObjects->at(i) = new v8::Global(pIsolate, obj); } @@ -387,9 +390,9 @@ void FXJS_InitializeRuntime( pV8PersistentContext->Reset(pIsolate, v8Context); } -void FXJS_ReleaseRuntime(v8::Isolate* pIsolate, - v8::Global* pV8PersistentContext, - std::vector*>* pStaticObjects) { +void FXJS_ReleaseEngine(v8::Isolate* pIsolate, + v8::Global* pV8PersistentContext, + std::vector*>* pStaticObjects) { v8::Isolate::Scope isolate_scope(pIsolate); v8::HandleScope handle_scope(pIsolate); v8::Local context = @@ -431,16 +434,16 @@ void FXJS_ReleaseRuntime(v8::Isolate* pIsolate, delete pData; } -IJS_Runtime* FXJS_GetRuntimeFromIsolate(v8::Isolate* pIsolate) { +CFXJS_Engine* FXJS_GetCurrentEngineFromIsolate(v8::Isolate* pIsolate) { v8::Local context = pIsolate->GetCurrentContext(); - return static_cast( + return static_cast( context->GetAlignedPointerFromEmbedderData(kPerContextDataIndex)); } #ifdef PDF_ENABLE_XFA -void FXJS_SetRuntimeForV8Context(v8::Local v8Context, - IJS_Runtime* pIRuntime) { - v8Context->SetAlignedPointerInEmbedderData(kPerContextDataIndex, pIRuntime); +void FXJS_SetEngineForV8Context(v8::Local v8Context, + CFXJS_Engine* pEngine) { + v8Context->SetAlignedPointerInEmbedderData(kPerContextDataIndex, pEngine); } #endif // PDF_ENABLE_XFA @@ -473,7 +476,7 @@ int FXJS_Execute(v8::Isolate* pIsolate, } v8::Local FXJS_NewFxDynamicObj(v8::Isolate* pIsolate, - IJS_Runtime* pIRuntime, + CFXJS_Engine* pEngine, int nObjDefnID, bool bStatic) { v8::Isolate::Scope isolate_scope(pIsolate); @@ -502,7 +505,7 @@ v8::Local FXJS_NewFxDynamicObj(v8::Isolate* pIsolate, CFXJS_PerObjectData* pPerObjData = new CFXJS_PerObjectData(nObjDefnID); obj->SetAlignedPointerInInternalField(0, pPerObjData); if (pObjDef->m_pConstructor) - pObjDef->m_pConstructor(pIRuntime, obj); + pObjDef->m_pConstructor(pEngine, obj); if (!bStatic && FXJS_PerIsolateData::Get(pIsolate)->m_pDynamicObjsMap) { FXJS_PerIsolateData::Get(pIsolate)->m_pDynamicObjsMap->set(pPerObjData, diff --git a/fxjs/fxjs_v8_embeddertest.cpp b/fxjs/fxjs_v8_embeddertest.cpp index 71b004c144..b573939b14 100644 --- a/fxjs/fxjs_v8_embeddertest.cpp +++ b/fxjs/fxjs_v8_embeddertest.cpp @@ -44,19 +44,17 @@ TEST_F(FXJSV8EmbedderTest, Getters) { CheckAssignmentInCurrentContext(kExpected1); } -TEST_F(FXJSV8EmbedderTest, MultipleRutimes) { +TEST_F(FXJSV8EmbedderTest, MultipleEngines) { v8::Isolate::Scope isolate_scope(isolate()); v8::HandleScope handle_scope(isolate()); v8::Global global_context1; std::vector*> static_objects1; - FXJS_InitializeRuntime(isolate(), nullptr, &global_context1, - &static_objects1); + FXJS_InitializeEngine(isolate(), nullptr, &global_context1, &static_objects1); v8::Global global_context2; std::vector*> static_objects2; - FXJS_InitializeRuntime(isolate(), nullptr, &global_context2, - &static_objects2); + FXJS_InitializeEngine(isolate(), nullptr, &global_context2, &static_objects2); v8::Context::Scope context_scope(GetV8Context()); ExecuteInCurrentContext(CFX_WideString(kScript0)); @@ -69,7 +67,7 @@ TEST_F(FXJSV8EmbedderTest, MultipleRutimes) { ExecuteInCurrentContext(CFX_WideString(kScript1)); CheckAssignmentInCurrentContext(kExpected1); } - FXJS_ReleaseRuntime(isolate(), &global_context1, &static_objects1); + FXJS_ReleaseEngine(isolate(), &global_context1, &static_objects1); { v8::Local context2 = @@ -78,7 +76,7 @@ TEST_F(FXJSV8EmbedderTest, MultipleRutimes) { ExecuteInCurrentContext(CFX_WideString(kScript2)); CheckAssignmentInCurrentContext(kExpected2); } - FXJS_ReleaseRuntime(isolate(), &global_context2, &static_objects2); + FXJS_ReleaseEngine(isolate(), &global_context2, &static_objects2); CheckAssignmentInCurrentContext(kExpected0); } diff --git a/fxjs/include/fxjs_v8.h b/fxjs/include/fxjs_v8.h index 5e12b082be..52afc491b2 100644 --- a/fxjs/include/fxjs_v8.h +++ b/fxjs/include/fxjs_v8.h @@ -17,16 +17,17 @@ #include #include +#include #include #include "core/fxcrt/include/fx_string.h" +class CFXJS_Engine; class CFXJS_ObjDefinition; -// FXJS_V8 places no restrictions on these two classes; it merely passes them +// FXJS_V8 places no restrictions on this class; it merely passes it // on to caller-provided methods. class IJS_Context; // A description of the event that caused JS execution. -class IJS_Runtime; // A native runtime, typically owns the v8::Context. #ifdef PDF_ENABLE_XFA // FXJS_V8 places no interpreation on this calass; it merely passes it @@ -124,12 +125,25 @@ class FXJS_ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { void Free(void* data, size_t length) override; }; -using FXJS_CONSTRUCTOR = void (*)(IJS_Runtime* cc, v8::Local obj); +using FXJS_CONSTRUCTOR = void (*)(CFXJS_Engine* fxjs, + v8::Local obj); using FXJS_DESTRUCTOR = void (*)(v8::Local obj); void FXJS_Initialize(unsigned int embedderDataSlot, v8::Isolate* pIsolate); void FXJS_Release(); +class CFXJS_Engine { + public: + CFXJS_Engine(); + ~CFXJS_Engine(); + + protected: + v8::Isolate* m_isolate; + v8::Global m_context; + std::vector*> m_StaticObjects; + std::map> m_ConstArrays; +}; + // Gets the global isolate set by FXJS_Initialize(), or makes a new one each // time if there is no such isolate. Returns true if a new isolate had to be // created. @@ -172,30 +186,30 @@ void FXJS_DefineGlobalConst(v8::Isolate* pIsolate, v8::FunctionCallback pConstGetter); // Called after FXJS_Define* calls made. -void FXJS_InitializeRuntime( +void FXJS_InitializeEngine( v8::Isolate* pIsolate, - IJS_Runtime* pIRuntime, + CFXJS_Engine* pEngine, v8::Global* pV8PersistentContext, std::vector*>* pStaticObjects); -void FXJS_ReleaseRuntime(v8::Isolate* pIsolate, - v8::Global* pV8PersistentContext, - std::vector*>* pStaticObjects); -IJS_Runtime* FXJS_GetRuntimeFromIsolate(v8::Isolate* pIsolate); +void FXJS_ReleaseEngine(v8::Isolate* pIsolate, + v8::Global* pV8PersistentContext, + std::vector*>* pStaticObjects); +CFXJS_Engine* FXJS_GetCurrentEngineFromIsolate(v8::Isolate* pIsolate); #ifdef PDF_ENABLE_XFA -// Called as part of FXJS_InitializeRuntime, exposed so PDF can make its +// Called as part of FXJS_InitializeEngine, exposed so PDF can make its // own contexts compatible with XFA or vice versa. -void FXJS_SetRuntimeForV8Context(v8::Local v8Context, - IJS_Runtime* pIRuntime); +void FXJS_SetEngineForV8Context(v8::Local v8Context, + CFXJS_Engine* pEngine); #endif // PDF_ENABLE_XFA -// Called after FXJS_InitializeRuntime call made. +// Called after FXJS_InitializeEngine call made. int FXJS_Execute(v8::Isolate* pIsolate, const CFX_WideString& script, FXJSErr* perror); v8::Local FXJS_NewFxDynamicObj(v8::Isolate* pIsolate, - IJS_Runtime* pJSContext, + CFXJS_Engine* pEngine, int nObjDefnID, bool bStatic = false); v8::Local FXJS_GetThisObj(v8::Isolate* pIsolate); diff --git a/testing/js_embedder_test.cpp b/testing/js_embedder_test.cpp index 5927d89af5..1b21fe04db 100644 --- a/testing/js_embedder_test.cpp +++ b/testing/js_embedder_test.cpp @@ -21,12 +21,12 @@ void JSEmbedderTest::SetUp() { v8::Isolate::Scope isolate_scope(m_pIsolate); v8::HandleScope handle_scope(m_pIsolate); FXJS_PerIsolateData::SetUp(m_pIsolate); - FXJS_InitializeRuntime(m_pIsolate, nullptr, &m_pPersistentContext, - &m_StaticObjects); + FXJS_InitializeEngine(m_pIsolate, nullptr, &m_pPersistentContext, + &m_StaticObjects); } void JSEmbedderTest::TearDown() { - FXJS_ReleaseRuntime(m_pIsolate, &m_pPersistentContext, &m_StaticObjects); + FXJS_ReleaseEngine(m_pIsolate, &m_pPersistentContext, &m_StaticObjects); m_pPersistentContext.Reset(); EmbedderTest::TearDown(); m_pIsolate->Dispose(); -- cgit v1.2.3