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>

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<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 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<v8::Object> obj) {
 }
 
 // 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 e2a0540252..61c330d687 100644
--- a/fxjs/cfxjse_context.cpp
+++ b/fxjs/cfxjse_context.cpp
@@ -194,7 +194,7 @@ std::unique_ptr<CFXJSE_Context> CFXJSE_Context::Create(
   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 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,16 +269,12 @@ CFXJS_Engine* CFXJS_Engine::EngineFromIsolateCurrentContext(
   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 @@ int CFXJS_Engine::GetObjDefnID(v8::Local<v8::Object> pObj) {
 }
 
 // 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 @@ void CFXJS_Engine::FreeObjectPrivate(v8::Local<v8::Object> pObj) {
   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 @@ void CFXJS_Engine::InitializeEngine() {
   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 @@ 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<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 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 <functional>
 #include <map>
 #include <memory>
 #include <vector>
@@ -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<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 @@ class CFXJS_Engine : public CJS_V8 {
   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);