Use unique pointer in CFXJS_PerObjectData.
Also use the actual type information, not void* and remove casts.
Template function not required to wrap virtual dtors.
Change-Id: I9397cae136c3c395a368a1ef0ce8162d9b586076
Reviewed-on: https://pdfium-review.googlesource.com/25290
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/JS_Define.cpp b/fxjs/JS_Define.cpp
index 171e05c..744cb7d 100644
--- a/fxjs/JS_Define.cpp
+++ b/fxjs/JS_Define.cpp
@@ -167,6 +167,10 @@
} // namespace
+void JSDestructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
+ pEngine->SetObjectPrivate(obj, nullptr);
+}
+
double JS_GetDateTime() {
if (!FSDK_IsSandBoxPolicyEnabled(FPDF_POLICY_MACHINETIME_ACCESS))
return 0;
diff --git a/fxjs/JS_Define.h b/fxjs/JS_Define.h
index f45738e..1c2410f 100644
--- a/fxjs/JS_Define.h
+++ b/fxjs/JS_Define.h
@@ -7,6 +7,7 @@
#ifndef FXJS_JS_DEFINE_H_
#define FXJS_JS_DEFINE_H_
+#include <utility>
#include <vector>
#include "fxjs/cjs_object.h"
@@ -49,16 +50,14 @@
template <class T, class A>
static void JSConstructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
- CJS_Object* pObj = new T(obj);
- pObj->SetEmbedObject(pdfium::MakeUnique<A>(pObj));
- pEngine->SetObjectPrivate(obj, pObj);
+ auto pObj = pdfium::MakeUnique<T>(obj);
+ pObj->SetEmbedObject(pdfium::MakeUnique<A>(pObj.get()));
pObj->InitInstance(static_cast<CJS_Runtime*>(pEngine));
+ pEngine->SetObjectPrivate(obj, std::move(pObj));
}
-template <class T>
-static void JSDestructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj) {
- delete static_cast<T*>(pEngine->GetObjectPrivate(obj));
-}
+// CJS_Object has vitual dtor, template not required.
+void JSDestructor(CFXJS_Engine* pEngine, v8::Local<v8::Object> obj);
template <class C, CJS_Return (C::*M)(CJS_Runtime*)>
void JSPropGetter(const char* prop_name_string,
@@ -70,8 +69,7 @@
if (!pRuntime)
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(info.Holder()));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder());
if (!pJSObj)
return;
@@ -98,8 +96,7 @@
if (!pRuntime)
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(info.Holder()));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder());
if (!pJSObj)
return;
@@ -122,8 +119,7 @@
if (!pRuntime)
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(info.Holder()));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder());
if (!pJSObj)
return;
diff --git a/fxjs/cjs_annot.cpp b/fxjs/cjs_annot.cpp
index 69eccef..100fa20 100644
--- a/fxjs/cjs_annot.cpp
+++ b/fxjs/cjs_annot.cpp
@@ -34,8 +34,7 @@
// static
void CJS_Annot::DefineJSObjects(CFXJS_Engine* pEngine) {
ObjDefnID = pEngine->DefineObj("Annot", FXJSOBJTYPE_DYNAMIC,
- JSConstructor<CJS_Annot, Annot>,
- JSDestructor<CJS_Annot>);
+ JSConstructor<CJS_Annot, Annot>, JSDestructor);
DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs));
}
diff --git a/fxjs/cjs_app.cpp b/fxjs/cjs_app.cpp
index 32cac22..7d16cba 100644
--- a/fxjs/cjs_app.cpp
+++ b/fxjs/cjs_app.cpp
@@ -77,9 +77,8 @@
// static
void CJS_App::DefineJSObjects(CFXJS_Engine* pEngine) {
- ObjDefnID =
- pEngine->DefineObj("app", FXJSOBJTYPE_STATIC, JSConstructor<CJS_App, app>,
- JSDestructor<CJS_App>);
+ ObjDefnID = pEngine->DefineObj("app", FXJSOBJTYPE_STATIC,
+ JSConstructor<CJS_App, app>, JSDestructor);
DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs));
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
}
@@ -376,8 +375,7 @@
if (CFXJS_Engine::GetObjDefnID(pObj) != CJS_TimerObj::GetObjDefnID())
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(pObj));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(pObj);
if (!pJSObj)
return;
diff --git a/fxjs/cjs_color.cpp b/fxjs/cjs_color.cpp
index 58a98dd..cfef449 100644
--- a/fxjs/cjs_color.cpp
+++ b/fxjs/cjs_color.cpp
@@ -36,8 +36,7 @@
// static
void CJS_Color::DefineJSObjects(CFXJS_Engine* pEngine) {
ObjDefnID = pEngine->DefineObj("color", FXJSOBJTYPE_STATIC,
- JSConstructor<CJS_Color, color>,
- JSDestructor<CJS_Color>);
+ JSConstructor<CJS_Color, color>, JSDestructor);
DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs));
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
}
diff --git a/fxjs/cjs_console.cpp b/fxjs/cjs_console.cpp
index 1921cd2..f4158a9 100644
--- a/fxjs/cjs_console.cpp
+++ b/fxjs/cjs_console.cpp
@@ -22,9 +22,9 @@
// static
void CJS_Console::DefineJSObjects(CFXJS_Engine* pEngine) {
- ObjDefnID = pEngine->DefineObj("console", FXJSOBJTYPE_STATIC,
- JSConstructor<CJS_Console, console>,
- JSDestructor<CJS_Console>);
+ ObjDefnID =
+ pEngine->DefineObj("console", FXJSOBJTYPE_STATIC,
+ JSConstructor<CJS_Console, console>, JSDestructor);
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
}
diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp
index 980edd9..e513ba0 100644
--- a/fxjs/cjs_document.cpp
+++ b/fxjs/cjs_document.cpp
@@ -116,9 +116,9 @@
// static
void CJS_Document::DefineJSObjects(CFXJS_Engine* pEngine) {
- ObjDefnID = pEngine->DefineObj("Document", FXJSOBJTYPE_GLOBAL,
- JSConstructor<CJS_Document, Document>,
- JSDestructor<CJS_Document>);
+ ObjDefnID =
+ pEngine->DefineObj("Document", FXJSOBJTYPE_GLOBAL,
+ JSConstructor<CJS_Document, Document>, JSDestructor);
DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs));
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
}
@@ -362,8 +362,7 @@
if (CFXJS_Engine::GetObjDefnID(pObj) ==
CJS_PrintParamsObj::GetObjDefnID()) {
v8::Local<v8::Object> pObj = pRuntime->ToObject(params[8]);
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(pObj));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(pObj);
if (pJSObj) {
if (PrintParamsObj* pprintparamsObj =
static_cast<PrintParamsObj*>(pJSObj->GetEmbedObject())) {
@@ -1113,7 +1112,7 @@
return CJS_Return(JSGetStringFromID(JSMessage::kTypeError));
v8::Local<v8::Object> pObj = pRuntime->ToObject(params[1]);
- CJS_Object* obj = static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(pObj));
+ CJS_Object* obj = pRuntime->GetObjectPrivate(pObj);
if (!obj->GetEmbedObject())
return CJS_Return(JSGetStringFromID(JSMessage::kTypeError));
diff --git a/fxjs/cjs_event.cpp b/fxjs/cjs_event.cpp
index 4fb988f..09b104a 100644
--- a/fxjs/cjs_event.cpp
+++ b/fxjs/cjs_event.cpp
@@ -39,8 +39,7 @@
// static
void CJS_Event::DefineJSObjects(CFXJS_Engine* pEngine) {
ObjDefnID = pEngine->DefineObj("event", FXJSOBJTYPE_STATIC,
- JSConstructor<CJS_Event, event>,
- JSDestructor<CJS_Event>);
+ JSConstructor<CJS_Event, event>, JSDestructor);
DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs));
}
diff --git a/fxjs/cjs_field.cpp b/fxjs/cjs_field.cpp
index b9b93d4..c8e4897 100644
--- a/fxjs/cjs_field.cpp
+++ b/fxjs/cjs_field.cpp
@@ -165,8 +165,7 @@
// static
void CJS_Field::DefineJSObjects(CFXJS_Engine* pEngine) {
ObjDefnID = pEngine->DefineObj("Field", FXJSOBJTYPE_DYNAMIC,
- JSConstructor<CJS_Field, Field>,
- JSDestructor<CJS_Field>);
+ JSConstructor<CJS_Field, Field>, JSDestructor);
DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs));
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
}
diff --git a/fxjs/cjs_global.cpp b/fxjs/cjs_global.cpp
index 3fc4bf0..0326516 100644
--- a/fxjs/cjs_global.cpp
+++ b/fxjs/cjs_global.cpp
@@ -37,8 +37,7 @@
if (!pRuntime)
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(info.Holder()));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder());
if (!pJSObj)
return;
@@ -57,8 +56,7 @@
if (!pRuntime)
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(info.Holder()));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder());
if (!pJSObj)
return;
@@ -85,8 +83,7 @@
if (!pRuntime)
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(info.Holder()));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder());
if (!pJSObj)
return;
@@ -108,8 +105,7 @@
if (!pRuntime)
return;
- CJS_Object* pJSObj =
- static_cast<CJS_Object*>(pRuntime->GetObjectPrivate(info.Holder()));
+ CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder());
if (!pJSObj)
return;
@@ -227,7 +223,7 @@
void CJS_Global::DefineJSObjects(CFXJS_Engine* pEngine) {
ObjDefnID = pEngine->DefineObj("global", FXJSOBJTYPE_STATIC,
JSConstructor<CJS_Global, JSGlobalAlternate>,
- JSDestructor<CJS_Global>);
+ JSDestructor);
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
DefineAllProperties(pEngine);
}
diff --git a/fxjs/cjs_icon.cpp b/fxjs/cjs_icon.cpp
index 2b56f70..9a4b73b 100644
--- a/fxjs/cjs_icon.cpp
+++ b/fxjs/cjs_icon.cpp
@@ -18,9 +18,8 @@
// static
void CJS_Icon::DefineJSObjects(CFXJS_Engine* pEngine) {
- ObjDefnID =
- pEngine->DefineObj("Icon", FXJSOBJTYPE_DYNAMIC,
- JSConstructor<CJS_Icon, Icon>, JSDestructor<CJS_Icon>);
+ ObjDefnID = pEngine->DefineObj("Icon", FXJSOBJTYPE_DYNAMIC,
+ JSConstructor<CJS_Icon, Icon>, JSDestructor);
DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs));
}
diff --git a/fxjs/cjs_printparamsobj.cpp b/fxjs/cjs_printparamsobj.cpp
index 296c241..7d6a7ea 100644
--- a/fxjs/cjs_printparamsobj.cpp
+++ b/fxjs/cjs_printparamsobj.cpp
@@ -15,10 +15,9 @@
// static
void CJS_PrintParamsObj::DefineJSObjects(CFXJS_Engine* pEngine) {
- ObjDefnID =
- pEngine->DefineObj("PrintParamsObj", FXJSOBJTYPE_DYNAMIC,
- JSConstructor<CJS_PrintParamsObj, PrintParamsObj>,
- JSDestructor<CJS_PrintParamsObj>);
+ ObjDefnID = pEngine->DefineObj(
+ "PrintParamsObj", FXJSOBJTYPE_DYNAMIC,
+ JSConstructor<CJS_PrintParamsObj, PrintParamsObj>, JSDestructor);
}
PrintParamsObj::PrintParamsObj(CJS_Object* pJSObject)
diff --git a/fxjs/cjs_report.cpp b/fxjs/cjs_report.cpp
index 0788a90..6af87a6 100644
--- a/fxjs/cjs_report.cpp
+++ b/fxjs/cjs_report.cpp
@@ -19,9 +19,8 @@
// static
void CJS_Report::DefineJSObjects(CFXJS_Engine* pEngine, FXJSOBJTYPE eObjType) {
- ObjDefnID =
- pEngine->DefineObj("Report", eObjType, JSConstructor<CJS_Report, Report>,
- JSDestructor<CJS_Report>);
+ ObjDefnID = pEngine->DefineObj(
+ "Report", eObjType, JSConstructor<CJS_Report, Report>, JSDestructor);
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
}
diff --git a/fxjs/cjs_timerobj.cpp b/fxjs/cjs_timerobj.cpp
index 410ad08..d892e0b 100644
--- a/fxjs/cjs_timerobj.cpp
+++ b/fxjs/cjs_timerobj.cpp
@@ -17,9 +17,9 @@
// static
void CJS_TimerObj::DefineJSObjects(CFXJS_Engine* pEngine) {
- ObjDefnID = pEngine->DefineObj("TimerObj", FXJSOBJTYPE_DYNAMIC,
- JSConstructor<CJS_TimerObj, TimerObj>,
- JSDestructor<CJS_TimerObj>);
+ ObjDefnID =
+ pEngine->DefineObj("TimerObj", FXJSOBJTYPE_DYNAMIC,
+ JSConstructor<CJS_TimerObj, TimerObj>, JSDestructor);
}
TimerObj::TimerObj(CJS_Object* pJSObject)
diff --git a/fxjs/cjs_util.cpp b/fxjs/cjs_util.cpp
index 57267ad..b161ec7 100644
--- a/fxjs/cjs_util.cpp
+++ b/fxjs/cjs_util.cpp
@@ -68,9 +68,8 @@
// static
void CJS_Util::DefineJSObjects(CFXJS_Engine* pEngine) {
- ObjDefnID =
- pEngine->DefineObj("util", FXJSOBJTYPE_STATIC,
- JSConstructor<CJS_Util, util>, JSDestructor<CJS_Util>);
+ ObjDefnID = pEngine->DefineObj("util", FXJSOBJTYPE_STATIC,
+ JSConstructor<CJS_Util, util>, JSDestructor);
DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs));
}
diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp
index f1555b2..2161076 100644
--- a/fxjs/fxjs_v8.cpp
+++ b/fxjs/fxjs_v8.cpp
@@ -6,9 +6,12 @@
#include "fxjs/fxjs_v8.h"
+#include <memory>
+#include <utility>
#include <vector>
#include "fxjs/cfxjse_runtimedata.h"
+#include "fxjs/cjs_object.h"
#include "third_party/base/allocator/partition_allocator/partition_alloc.h"
// Keep this consistent with the values defined in gin/public/context_holder.h
@@ -25,8 +28,9 @@
class CFXJS_PerObjectData {
public:
- explicit CFXJS_PerObjectData(int nObjDefID)
- : m_ObjDefID(nObjDefID), m_pPrivate(nullptr) {}
+ explicit CFXJS_PerObjectData(int nObjDefID) : m_ObjDefID(nObjDefID) {}
+
+ ~CFXJS_PerObjectData() = default;
static void SetInObject(CFXJS_PerObjectData* pData,
v8::Local<v8::Object> pObj) {
@@ -48,7 +52,7 @@
}
const int m_ObjDefID;
- void* m_pPrivate;
+ std::unique_ptr<CJS_Object> m_pPrivate;
};
class CFXJS_ObjDefinition {
@@ -540,15 +544,16 @@
GetIsolate()->ThrowException(NewString(message.AsStringView()));
}
-void CFXJS_Engine::SetObjectPrivate(v8::Local<v8::Object> pObj, void* p) {
+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 = p;
+ pPerObjectData->m_pPrivate = std::move(p);
}
-void* CFXJS_Engine::GetObjectPrivate(v8::Local<v8::Object> pObj) {
+CJS_Object* CFXJS_Engine::GetObjectPrivate(v8::Local<v8::Object> pObj) {
CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj);
if (!pData && !pObj.IsEmpty()) {
// It could be a global proxy object.
@@ -559,7 +564,7 @@
v->ToObject(context).ToLocalChecked());
}
}
- return pData ? pData->m_pPrivate : nullptr;
+ return pData ? pData->m_pPrivate.get() : nullptr;
}
v8::Local<v8::Array> CFXJS_Engine::GetConstArray(const WideString& name) {
diff --git a/fxjs/fxjs_v8.h b/fxjs/fxjs_v8.h
index 4e6b248..7091b0e 100644
--- a/fxjs/fxjs_v8.h
+++ b/fxjs/fxjs_v8.h
@@ -29,6 +29,7 @@
#endif // PDF_ENABLE_XFA
class CFXJS_ObjDefinition;
+class CJS_Object;
// FXJS_V8 places no restrictions on this class; it merely passes it
// on to caller-provided methods.
@@ -178,9 +179,10 @@
v8::Local<v8::Object> NewFxDynamicObj(int nObjDefnID, bool bStatic = false);
// Native object binding.
- void SetObjectPrivate(v8::Local<v8::Object> pObj, void* p);
- void* GetObjectPrivate(v8::Local<v8::Object> pObj);
- static void FreeObjectPrivate(void* p);
+ void SetObjectPrivate(v8::Local<v8::Object> pObj,
+ std::unique_ptr<CJS_Object> p);
+ CJS_Object* GetObjectPrivate(v8::Local<v8::Object> pObj);
+ static void FreeObjectPrivate(void* pPerObjectData);
static void FreeObjectPrivate(v8::Local<v8::Object> pObj);
void Error(const WideString& message);
