From b3f1046cb2c9bb7fc2a2579b76c8e65b24323002 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 8 Feb 2018 20:16:09 +0000 Subject: Make CFXJS_Engine::SetObjectPrivate() static. Avoids call to CFXJS_Engine::EngineFromIsolateCurrentContext() during the Dispose() path, which feels scary because there aren't guarantees about it having an engine at isolate "dispose" time. Fortunately, |this| is not used, so make that fact clear. Replace some c-style callbacks with std::function while we're at it. Change-Id: Ia1a1a1fcc085d8657939e6f8c8d34fc511afddfe Reviewed-on: https://pdfium-review.googlesource.com/25970 Commit-Queue: Tom Sepez Reviewed-by: dsinclair --- fxjs/JS_Define.cpp | 4 ++-- fxjs/JS_Define.h | 2 +- fxjs/cfxjse_context.cpp | 2 +- fxjs/fxjs_v8.cpp | 39 +++++++++++++++++++-------------------- fxjs/fxjs_v8.h | 21 +++++++++++---------- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/fxjs/JS_Define.cpp b/fxjs/JS_Define.cpp index 744cb7dd77..d5b1cefeda 100644 --- a/fxjs/JS_Define.cpp +++ b/fxjs/JS_Define.cpp @@ -167,8 +167,8 @@ int DateFromTime(double t) { } // namespace -void JSDestructor(CFXJS_Engine* pEngine, v8::Local obj) { - pEngine->SetObjectPrivate(obj, nullptr); +void JSDestructor(v8::Local obj) { + CFXJS_Engine::SetObjectPrivate(obj, nullptr); } double JS_GetDateTime() { diff --git a/fxjs/JS_Define.h b/fxjs/JS_Define.h index 61ad9efc1f..58c094fe37 100644 --- a/fxjs/JS_Define.h +++ b/fxjs/JS_Define.h @@ -56,7 +56,7 @@ static void JSConstructor(CFXJS_Engine* pEngine, v8::Local obj) { } // CJS_Object has vitual dtor, template not required. -void JSDestructor(CFXJS_Engine* pEngine, v8::Local obj); +void JSDestructor(v8::Local obj); template void JSPropGetter(const char* prop_name_string, diff --git a/fxjs/cfxjse_context.cpp b/fxjs/cfxjse_context.cpp index e2a0540252..61c330d687 100644 --- a/fxjs/cfxjse_context.cpp +++ b/fxjs/cfxjse_context.cpp @@ -194,7 +194,7 @@ std::unique_ptr CFXJSE_Context::Create( v8::Local hGlobalObject = GetGlobalObjectFromContext(hNewContext); FXJSE_UpdateObjectBinding(hGlobalObject, pGlobalObject); if (pOptionalEngineToSet) - CFXJS_Engine::SetEngineInContext(pOptionalEngineToSet, hNewContext); + pOptionalEngineToSet->SetIntoContext(hNewContext); pContext->m_hContext.Reset(pIsolate, hNewContext); return pContext; diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp index 8a04a0680e..3d82848a6e 100644 --- a/fxjs/fxjs_v8.cpp +++ b/fxjs/fxjs_v8.cpp @@ -181,10 +181,8 @@ void V8TemplateMapTraits::Dispose(v8::Isolate* isolate, CFXJS_ObjDefinition* pObjDef = CFXJS_ObjDefinition::ForID(isolate, id); if (!pObjDef) return; - if (pObjDef->m_pDestructor) { - pObjDef->m_pDestructor( - CFXJS_Engine::EngineFromIsolateCurrentContext(isolate), obj); - } + if (pObjDef->m_pDestructor) + pObjDef->m_pDestructor(obj); CFXJS_Engine::FreeObjectPrivate(obj); } @@ -271,22 +269,28 @@ CFXJS_Engine* CFXJS_Engine::EngineFromIsolateCurrentContext( return EngineFromContext(pIsolate->GetCurrentContext()); } +// static CFXJS_Engine* CFXJS_Engine::EngineFromContext(v8::Local pContext) { return static_cast( pContext->GetAlignedPointerFromEmbedderData(kPerContextDataIndex)); } -void CFXJS_Engine::SetEngineInContext(CFXJS_Engine* pEngine, - v8::Local pContext) { - pContext->SetAlignedPointerInEmbedderData(kPerContextDataIndex, pEngine); -} - // static int CFXJS_Engine::GetObjDefnID(v8::Local pObj) { CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj); return pData ? pData->m_ObjDefID : -1; } +// static +void CFXJS_Engine::SetObjectPrivate(v8::Local pObj, + std::unique_ptr p) { + CFXJS_PerObjectData* pPerObjectData = + CFXJS_PerObjectData::GetFromObject(pObj); + if (!pPerObjectData) + return; + pPerObjectData->m_pPrivate = std::move(p); +} + // static void CFXJS_Engine::FreeObjectPrivate(v8::Local pObj) { CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj); @@ -295,6 +299,10 @@ void CFXJS_Engine::FreeObjectPrivate(v8::Local pObj) { delete pData; } +void CFXJS_Engine::SetIntoContext(v8::Local pContext) { + pContext->SetAlignedPointerInEmbedderData(kPerContextDataIndex, this); +} + int CFXJS_Engine::DefineObj(const char* sObjName, FXJSOBJTYPE eObjType, CFXJS_Engine::Constructor pConstructor, @@ -394,7 +402,7 @@ void CFXJS_Engine::InitializeEngine() { v8::Local v8Context = v8::Context::New( GetIsolate(), nullptr, GetGlobalObjectTemplate(GetIsolate())); v8::Context::Scope context_scope(v8Context); - SetEngineInContext(this, v8Context); + SetIntoContext(v8Context); int maxID = CFXJS_ObjDefinition::MaxID(GetIsolate()); m_StaticObjects.resize(maxID + 1); @@ -449,7 +457,7 @@ void CFXJS_Engine::ReleaseEngine() { if (!pObj.IsEmpty()) { if (pObjDef->m_pDestructor) - pObjDef->m_pDestructor(this, pObj); + pObjDef->m_pDestructor(pObj); FreeObjectPrivate(pObj); } } @@ -527,15 +535,6 @@ void CFXJS_Engine::Error(const WideString& message) { GetIsolate()->ThrowException(NewString(message.AsStringView())); } -void CFXJS_Engine::SetObjectPrivate(v8::Local pObj, - std::unique_ptr p) { - CFXJS_PerObjectData* pPerObjectData = - CFXJS_PerObjectData::GetFromObject(pObj); - if (!pPerObjectData) - return; - pPerObjectData->m_pPrivate = std::move(p); -} - CJS_Object* CFXJS_Engine::GetObjectPrivate(v8::Local pObj) { CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj); if (!pData && !pObj.IsEmpty()) { diff --git a/fxjs/fxjs_v8.h b/fxjs/fxjs_v8.h index 0592847bb2..2c53b65d3a 100644 --- a/fxjs/fxjs_v8.h +++ b/fxjs/fxjs_v8.h @@ -14,6 +14,7 @@ #ifndef FXJS_FXJS_V8_H_ #define FXJS_FXJS_V8_H_ +#include #include #include #include @@ -131,17 +132,21 @@ class CFXJS_Engine : public CJS_V8 { explicit CFXJS_Engine(v8::Isolate* pIsolate); ~CFXJS_Engine() override; - using Constructor = void (*)(CFXJS_Engine* pEngine, - v8::Local obj); - using Destructor = void (*)(CFXJS_Engine* pEngine, v8::Local obj); + using Constructor = + std::function obj)>; + using Destructor = std::function obj)>; static CFXJS_Engine* EngineFromIsolateCurrentContext(v8::Isolate* pIsolate); static CFXJS_Engine* EngineFromContext(v8::Local pContext); - static void SetEngineInContext(CFXJS_Engine* pEngine, - v8::Local pContext); static int GetObjDefnID(v8::Local pObj); + static void SetObjectPrivate(v8::Local pObj, + std::unique_ptr p); + static void FreeObjectPrivate(v8::Local pObj); + + void SetIntoContext(v8::Local pContext); + // Always returns a valid, newly-created objDefnID. int DefineObj(const char* sObjName, FXJSOBJTYPE eObjType, @@ -178,12 +183,8 @@ class CFXJS_Engine : public CJS_V8 { v8::Local GetThisObj(); v8::Local NewFXJSBoundObject(int nObjDefnID, bool bStatic = false); - - // Native object binding. - void SetObjectPrivate(v8::Local pObj, - std::unique_ptr p); + // Retrieve native object binding. CJS_Object* GetObjectPrivate(v8::Local pObj); - static void FreeObjectPrivate(v8::Local pObj); void Error(const WideString& message); -- cgit v1.2.3