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 <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
diff --git a/fxjs/JS_Define.cpp b/fxjs/JS_Define.cpp
index 744cb7d..d5b1cef 100644
--- a/fxjs/JS_Define.cpp
+++ b/fxjs/JS_Define.cpp
@@ -167,8 +167,8 @@
} // namespace
-void JSDestructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
- pEngine->SetObjectPrivate(obj, nullptr);
+void JSDestructor(v8::Local<v8::Object> obj) {
+ CFXJS_Engine::SetObjectPrivate(obj, nullptr);
}
double JS_GetDateTime() {
diff --git a/fxjs/JS_Define.h b/fxjs/JS_Define.h
index 61ad9ef..58c094f 100644
--- a/fxjs/JS_Define.h
+++ b/fxjs/JS_Define.h
@@ -56,7 +56,7 @@
}
// CJS_Object has vitual dtor, template not required.
-void JSDestructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj);
+void JSDestructor(v8::Local<v8::Object> obj);
template <class C, CJS_Return (C::*M)(CJS_Runtime*)>
void JSPropGetter(const char* prop_name_string,
diff --git a/fxjs/cfxjse_context.cpp b/fxjs/cfxjse_context.cpp
index e2a0540..61c330d 100644
--- a/fxjs/cfxjse_context.cpp
+++ b/fxjs/cfxjse_context.cpp
@@ -194,7 +194,7 @@
v8::Local<v8::Object> 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 8a04a068..3d82848 100644
--- a/fxjs/fxjs_v8.cpp
+++ b/fxjs/fxjs_v8.cpp
@@ -181,10 +181,8 @@
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,16 +269,12 @@
return EngineFromContext(pIsolate->GetCurrentContext());
}
+// static
CFXJS_Engine* CFXJS_Engine::EngineFromContext(v8::Local<v8::Context> pContext) {
return static_cast<CFXJS_Engine*>(
pContext->GetAlignedPointerFromEmbedderData(kPerContextDataIndex));
}
-void CFXJS_Engine::SetEngineInContext(CFXJS_Engine* pEngine,
- v8::Local<v8::Context> pContext) {
- pContext->SetAlignedPointerInEmbedderData(kPerContextDataIndex, pEngine);
-}
-
// static
int CFXJS_Engine::GetObjDefnID(v8::Local<v8::Object> pObj) {
CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj);
@@ -288,6 +282,16 @@
}
// static
+void CFXJS_Engine::SetObjectPrivate(v8::Local<v8::Object> pObj,
+ std::unique_ptr<CJS_Object> p) {
+ CFXJS_PerObjectData* pPerObjectData =
+ CFXJS_PerObjectData::GetFromObject(pObj);
+ if (!pPerObjectData)
+ return;
+ pPerObjectData->m_pPrivate = std::move(p);
+}
+
+// static
void CFXJS_Engine::FreeObjectPrivate(v8::Local<v8::Object> pObj) {
CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj);
pObj->SetAlignedPointerInInternalField(0, nullptr);
@@ -295,6 +299,10 @@
delete pData;
}
+void CFXJS_Engine::SetIntoContext(v8::Local<v8::Context> pContext) {
+ pContext->SetAlignedPointerInEmbedderData(kPerContextDataIndex, this);
+}
+
int CFXJS_Engine::DefineObj(const char* sObjName,
FXJSOBJTYPE eObjType,
CFXJS_Engine::Constructor pConstructor,
@@ -394,7 +402,7 @@
v8::Local<v8::Context> 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 @@
if (!pObj.IsEmpty()) {
if (pObjDef->m_pDestructor)
- pObjDef->m_pDestructor(this, pObj);
+ pObjDef->m_pDestructor(pObj);
FreeObjectPrivate(pObj);
}
}
@@ -527,15 +535,6 @@
GetIsolate()->ThrowException(NewString(message.AsStringView()));
}
-void CFXJS_Engine::SetObjectPrivate(v8::Local<v8::Object> pObj,
- std::unique_ptr<CJS_Object> p) {
- CFXJS_PerObjectData* pPerObjectData =
- CFXJS_PerObjectData::GetFromObject(pObj);
- if (!pPerObjectData)
- return;
- pPerObjectData->m_pPrivate = std::move(p);
-}
-
CJS_Object* CFXJS_Engine::GetObjectPrivate(v8::Local<v8::Object> 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 0592847..2c53b65 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 <functional>
#include <map>
#include <memory>
#include <vector>
@@ -131,17 +132,21 @@
explicit CFXJS_Engine(v8::Isolate* pIsolate);
~CFXJS_Engine() override;
- using Constructor = void (*)(CFXJS_Engine* pEngine,
- v8::Local<v8::Object> obj);
- using Destructor = void (*)(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj);
+ using Constructor =
+ std::function<void(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj)>;
+ using Destructor = std::function<void(v8::Local<v8::Object> obj)>;
static CFXJS_Engine* EngineFromIsolateCurrentContext(v8::Isolate* pIsolate);
static CFXJS_Engine* EngineFromContext(v8::Local<v8::Context> pContext);
- static void SetEngineInContext(CFXJS_Engine* pEngine,
- v8::Local<v8::Context> pContext);
static int GetObjDefnID(v8::Local<v8::Object> pObj);
+ static void SetObjectPrivate(v8::Local<v8::Object> pObj,
+ std::unique_ptr<CJS_Object> p);
+ static void FreeObjectPrivate(v8::Local<v8::Object> pObj);
+
+ void SetIntoContext(v8::Local<v8::Context> pContext);
+
// Always returns a valid, newly-created objDefnID.
int DefineObj(const char* sObjName,
FXJSOBJTYPE eObjType,
@@ -178,12 +183,8 @@
v8::Local<v8::Object> GetThisObj();
v8::Local<v8::Object> NewFXJSBoundObject(int nObjDefnID,
bool bStatic = false);
-
- // Native object binding.
- void SetObjectPrivate(v8::Local<v8::Object> pObj,
- std::unique_ptr<CJS_Object> p);
+ // Retrieve native object binding.
CJS_Object* GetObjectPrivate(v8::Local<v8::Object> pObj);
- static void FreeObjectPrivate(v8::Local<v8::Object> pObj);
void Error(const WideString& message);
