From 57e097750846bf3ffc3e4e2ef9e78be8a82c69de Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 5 Feb 2018 18:52:09 +0000 Subject: 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 Commit-Queue: Tom Sepez --- fxjs/JS_Define.cpp | 4 ++++ fxjs/JS_Define.h | 22 +++++++++------------- fxjs/cjs_annot.cpp | 3 +-- fxjs/cjs_app.cpp | 8 +++----- fxjs/cjs_color.cpp | 3 +-- fxjs/cjs_console.cpp | 6 +++--- fxjs/cjs_document.cpp | 11 +++++------ fxjs/cjs_event.cpp | 3 +-- fxjs/cjs_field.cpp | 3 +-- fxjs/cjs_global.cpp | 14 +++++--------- fxjs/cjs_icon.cpp | 5 ++--- fxjs/cjs_printparamsobj.cpp | 7 +++---- fxjs/cjs_report.cpp | 5 ++--- fxjs/cjs_timerobj.cpp | 6 +++--- fxjs/cjs_util.cpp | 5 ++--- fxjs/fxjs_v8.cpp | 19 ++++++++++++------- fxjs/fxjs_v8.h | 8 +++++--- 17 files changed, 62 insertions(+), 70 deletions(-) diff --git a/fxjs/JS_Define.cpp b/fxjs/JS_Define.cpp index 171e05c66e..744cb7dd77 100644 --- a/fxjs/JS_Define.cpp +++ b/fxjs/JS_Define.cpp @@ -167,6 +167,10 @@ int DateFromTime(double t) { } // namespace +void JSDestructor(CFXJS_Engine* pEngine, v8::Local 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 f45738e847..1c2410f18f 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 #include #include "fxjs/cjs_object.h" @@ -49,16 +50,14 @@ std::vector> ExpandKeywordParams( template static void JSConstructor(CFXJS_Engine* pEngine, v8::Local obj) { - CJS_Object* pObj = new T(obj); - pObj->SetEmbedObject(pdfium::MakeUnique(pObj)); - pEngine->SetObjectPrivate(obj, pObj); + auto pObj = pdfium::MakeUnique(obj); + pObj->SetEmbedObject(pdfium::MakeUnique(pObj.get())); pObj->InitInstance(static_cast(pEngine)); + pEngine->SetObjectPrivate(obj, std::move(pObj)); } -template -static void JSDestructor(CFXJS_Engine* pEngine, v8::Local obj) { - delete static_cast(pEngine->GetObjectPrivate(obj)); -} +// CJS_Object has vitual dtor, template not required. +void JSDestructor(CFXJS_Engine* pEngine, v8::Local obj); template void JSPropGetter(const char* prop_name_string, @@ -70,8 +69,7 @@ void JSPropGetter(const char* prop_name_string, if (!pRuntime) return; - CJS_Object* pJSObj = - static_cast(pRuntime->GetObjectPrivate(info.Holder())); + CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder()); if (!pJSObj) return; @@ -98,8 +96,7 @@ void JSPropSetter(const char* prop_name_string, if (!pRuntime) return; - CJS_Object* pJSObj = - static_cast(pRuntime->GetObjectPrivate(info.Holder())); + CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder()); if (!pJSObj) return; @@ -122,8 +119,7 @@ void JSMethod(const char* method_name_string, if (!pRuntime) return; - CJS_Object* pJSObj = - static_cast(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 69eccefa27..100fa20cd4 100644 --- a/fxjs/cjs_annot.cpp +++ b/fxjs/cjs_annot.cpp @@ -34,8 +34,7 @@ int CJS_Annot::GetObjDefnID() { // static void CJS_Annot::DefineJSObjects(CFXJS_Engine* pEngine) { ObjDefnID = pEngine->DefineObj("Annot", FXJSOBJTYPE_DYNAMIC, - JSConstructor, - JSDestructor); + JSConstructor, JSDestructor); DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs)); } diff --git a/fxjs/cjs_app.cpp b/fxjs/cjs_app.cpp index 32cac22153..7d16cbaaa2 100644 --- a/fxjs/cjs_app.cpp +++ b/fxjs/cjs_app.cpp @@ -77,9 +77,8 @@ int CJS_App::ObjDefnID = -1; // static void CJS_App::DefineJSObjects(CFXJS_Engine* pEngine) { - ObjDefnID = - pEngine->DefineObj("app", FXJSOBJTYPE_STATIC, JSConstructor, - JSDestructor); + ObjDefnID = pEngine->DefineObj("app", FXJSOBJTYPE_STATIC, + JSConstructor, JSDestructor); DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs)); DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs)); } @@ -376,8 +375,7 @@ void app::ClearTimerCommon(CJS_Runtime* pRuntime, v8::Local param) { if (CFXJS_Engine::GetObjDefnID(pObj) != CJS_TimerObj::GetObjDefnID()) return; - CJS_Object* pJSObj = - static_cast(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 58a98dd8ff..cfef4493b3 100644 --- a/fxjs/cjs_color.cpp +++ b/fxjs/cjs_color.cpp @@ -36,8 +36,7 @@ int CJS_Color::ObjDefnID = -1; // static void CJS_Color::DefineJSObjects(CFXJS_Engine* pEngine) { ObjDefnID = pEngine->DefineObj("color", FXJSOBJTYPE_STATIC, - JSConstructor, - JSDestructor); + JSConstructor, 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 1921cd2069..f4158a9c0a 100644 --- a/fxjs/cjs_console.cpp +++ b/fxjs/cjs_console.cpp @@ -22,9 +22,9 @@ int CJS_Console::ObjDefnID = -1; // static void CJS_Console::DefineJSObjects(CFXJS_Engine* pEngine) { - ObjDefnID = pEngine->DefineObj("console", FXJSOBJTYPE_STATIC, - JSConstructor, - JSDestructor); + ObjDefnID = + pEngine->DefineObj("console", FXJSOBJTYPE_STATIC, + JSConstructor, JSDestructor); DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs)); } diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp index 980edd990a..e513ba0159 100644 --- a/fxjs/cjs_document.cpp +++ b/fxjs/cjs_document.cpp @@ -116,9 +116,9 @@ int CJS_Document::GetObjDefnID() { // static void CJS_Document::DefineJSObjects(CFXJS_Engine* pEngine) { - ObjDefnID = pEngine->DefineObj("Document", FXJSOBJTYPE_GLOBAL, - JSConstructor, - JSDestructor); + ObjDefnID = + pEngine->DefineObj("Document", FXJSOBJTYPE_GLOBAL, + JSConstructor, JSDestructor); DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs)); DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs)); } @@ -362,8 +362,7 @@ CJS_Return Document::print(CJS_Runtime* pRuntime, if (CFXJS_Engine::GetObjDefnID(pObj) == CJS_PrintParamsObj::GetObjDefnID()) { v8::Local pObj = pRuntime->ToObject(params[8]); - CJS_Object* pJSObj = - static_cast(pRuntime->GetObjectPrivate(pObj)); + CJS_Object* pJSObj = pRuntime->GetObjectPrivate(pObj); if (pJSObj) { if (PrintParamsObj* pprintparamsObj = static_cast(pJSObj->GetEmbedObject())) { @@ -1113,7 +1112,7 @@ CJS_Return Document::addIcon(CJS_Runtime* pRuntime, return CJS_Return(JSGetStringFromID(JSMessage::kTypeError)); v8::Local pObj = pRuntime->ToObject(params[1]); - CJS_Object* obj = static_cast(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 4fb988f4a6..09b104a8d5 100644 --- a/fxjs/cjs_event.cpp +++ b/fxjs/cjs_event.cpp @@ -39,8 +39,7 @@ int CJS_Event::ObjDefnID = -1; // static void CJS_Event::DefineJSObjects(CFXJS_Engine* pEngine) { ObjDefnID = pEngine->DefineObj("event", FXJSOBJTYPE_STATIC, - JSConstructor, - JSDestructor); + JSConstructor, JSDestructor); DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs)); } diff --git a/fxjs/cjs_field.cpp b/fxjs/cjs_field.cpp index b9b93d40a5..c8e4897917 100644 --- a/fxjs/cjs_field.cpp +++ b/fxjs/cjs_field.cpp @@ -165,8 +165,7 @@ int CJS_Field::GetObjDefnID() { // static void CJS_Field::DefineJSObjects(CFXJS_Engine* pEngine) { ObjDefnID = pEngine->DefineObj("Field", FXJSOBJTYPE_DYNAMIC, - JSConstructor, - JSDestructor); + JSConstructor, 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 3fc4bf0f0f..0326516f1b 100644 --- a/fxjs/cjs_global.cpp +++ b/fxjs/cjs_global.cpp @@ -37,8 +37,7 @@ void JSSpecialPropQuery(const char*, if (!pRuntime) return; - CJS_Object* pJSObj = - static_cast(pRuntime->GetObjectPrivate(info.Holder())); + CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder()); if (!pJSObj) return; @@ -57,8 +56,7 @@ void JSSpecialPropGet(const char* class_name, if (!pRuntime) return; - CJS_Object* pJSObj = - static_cast(pRuntime->GetObjectPrivate(info.Holder())); + CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder()); if (!pJSObj) return; @@ -85,8 +83,7 @@ void JSSpecialPropPut(const char* class_name, if (!pRuntime) return; - CJS_Object* pJSObj = - static_cast(pRuntime->GetObjectPrivate(info.Holder())); + CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder()); if (!pJSObj) return; @@ -108,8 +105,7 @@ void JSSpecialPropDel(const char* class_name, if (!pRuntime) return; - CJS_Object* pJSObj = - static_cast(pRuntime->GetObjectPrivate(info.Holder())); + CJS_Object* pJSObj = pRuntime->GetObjectPrivate(info.Holder()); if (!pJSObj) return; @@ -227,7 +223,7 @@ void CJS_Global::DefineAllProperties(CFXJS_Engine* pEngine) { void CJS_Global::DefineJSObjects(CFXJS_Engine* pEngine) { ObjDefnID = pEngine->DefineObj("global", FXJSOBJTYPE_STATIC, JSConstructor, - JSDestructor); + JSDestructor); DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs)); DefineAllProperties(pEngine); } diff --git a/fxjs/cjs_icon.cpp b/fxjs/cjs_icon.cpp index 2b56f70cdc..9a4b73b2d6 100644 --- a/fxjs/cjs_icon.cpp +++ b/fxjs/cjs_icon.cpp @@ -18,9 +18,8 @@ int CJS_Icon::GetObjDefnID() { // static void CJS_Icon::DefineJSObjects(CFXJS_Engine* pEngine) { - ObjDefnID = - pEngine->DefineObj("Icon", FXJSOBJTYPE_DYNAMIC, - JSConstructor, JSDestructor); + ObjDefnID = pEngine->DefineObj("Icon", FXJSOBJTYPE_DYNAMIC, + JSConstructor, JSDestructor); DefineProps(pEngine, ObjDefnID, PropertySpecs, FX_ArraySize(PropertySpecs)); } diff --git a/fxjs/cjs_printparamsobj.cpp b/fxjs/cjs_printparamsobj.cpp index 296c241736..7d6a7ea453 100644 --- a/fxjs/cjs_printparamsobj.cpp +++ b/fxjs/cjs_printparamsobj.cpp @@ -15,10 +15,9 @@ int CJS_PrintParamsObj::GetObjDefnID() { // static void CJS_PrintParamsObj::DefineJSObjects(CFXJS_Engine* pEngine) { - ObjDefnID = - pEngine->DefineObj("PrintParamsObj", FXJSOBJTYPE_DYNAMIC, - JSConstructor, - JSDestructor); + ObjDefnID = pEngine->DefineObj( + "PrintParamsObj", FXJSOBJTYPE_DYNAMIC, + JSConstructor, JSDestructor); } PrintParamsObj::PrintParamsObj(CJS_Object* pJSObject) diff --git a/fxjs/cjs_report.cpp b/fxjs/cjs_report.cpp index 0788a90a4c..6af87a6fb9 100644 --- a/fxjs/cjs_report.cpp +++ b/fxjs/cjs_report.cpp @@ -19,9 +19,8 @@ int CJS_Report::ObjDefnID = -1; // static void CJS_Report::DefineJSObjects(CFXJS_Engine* pEngine, FXJSOBJTYPE eObjType) { - ObjDefnID = - pEngine->DefineObj("Report", eObjType, JSConstructor, - JSDestructor); + ObjDefnID = pEngine->DefineObj( + "Report", eObjType, JSConstructor, JSDestructor); DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs)); } diff --git a/fxjs/cjs_timerobj.cpp b/fxjs/cjs_timerobj.cpp index 410ad0846e..d892e0b3ee 100644 --- a/fxjs/cjs_timerobj.cpp +++ b/fxjs/cjs_timerobj.cpp @@ -17,9 +17,9 @@ int CJS_TimerObj::GetObjDefnID() { // static void CJS_TimerObj::DefineJSObjects(CFXJS_Engine* pEngine) { - ObjDefnID = pEngine->DefineObj("TimerObj", FXJSOBJTYPE_DYNAMIC, - JSConstructor, - JSDestructor); + ObjDefnID = + pEngine->DefineObj("TimerObj", FXJSOBJTYPE_DYNAMIC, + JSConstructor, JSDestructor); } TimerObj::TimerObj(CJS_Object* pJSObject) diff --git a/fxjs/cjs_util.cpp b/fxjs/cjs_util.cpp index 57267ad418..b161ec73fd 100644 --- a/fxjs/cjs_util.cpp +++ b/fxjs/cjs_util.cpp @@ -68,9 +68,8 @@ int CJS_Util::ObjDefnID = -1; // static void CJS_Util::DefineJSObjects(CFXJS_Engine* pEngine) { - ObjDefnID = - pEngine->DefineObj("util", FXJSOBJTYPE_STATIC, - JSConstructor, JSDestructor); + ObjDefnID = pEngine->DefineObj("util", FXJSOBJTYPE_STATIC, + JSConstructor, JSDestructor); DefineMethods(pEngine, ObjDefnID, MethodSpecs, FX_ArraySize(MethodSpecs)); } diff --git a/fxjs/fxjs_v8.cpp b/fxjs/fxjs_v8.cpp index f1555b2e96..2161076e36 100644 --- a/fxjs/fxjs_v8.cpp +++ b/fxjs/fxjs_v8.cpp @@ -6,9 +6,12 @@ #include "fxjs/fxjs_v8.h" +#include +#include #include #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 @@ static wchar_t kPerObjectDataTag[] = L"CFXJS_PerObjectData"; 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 pObj) { @@ -48,7 +52,7 @@ class CFXJS_PerObjectData { } const int m_ObjDefID; - void* m_pPrivate; + std::unique_ptr m_pPrivate; }; class CFXJS_ObjDefinition { @@ -540,15 +544,16 @@ void CFXJS_Engine::Error(const WideString& message) { GetIsolate()->ThrowException(NewString(message.AsStringView())); } -void CFXJS_Engine::SetObjectPrivate(v8::Local pObj, void* p) { +void CFXJS_Engine::SetObjectPrivate(v8::Local pObj, + std::unique_ptr 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 pObj) { +CJS_Object* CFXJS_Engine::GetObjectPrivate(v8::Local pObj) { CFXJS_PerObjectData* pData = CFXJS_PerObjectData::GetFromObject(pObj); if (!pData && !pObj.IsEmpty()) { // It could be a global proxy object. @@ -559,7 +564,7 @@ void* CFXJS_Engine::GetObjectPrivate(v8::Local pObj) { v->ToObject(context).ToLocalChecked()); } } - return pData ? pData->m_pPrivate : nullptr; + return pData ? pData->m_pPrivate.get() : nullptr; } v8::Local CFXJS_Engine::GetConstArray(const WideString& name) { diff --git a/fxjs/fxjs_v8.h b/fxjs/fxjs_v8.h index 4e6b248b5e..7091b0e43e 100644 --- a/fxjs/fxjs_v8.h +++ b/fxjs/fxjs_v8.h @@ -29,6 +29,7 @@ class CFXJSE_RuntimeData; #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 @@ class CFXJS_Engine : public CJS_V8 { v8::Local NewFxDynamicObj(int nObjDefnID, bool bStatic = false); // Native object binding. - void SetObjectPrivate(v8::Local pObj, void* p); - void* GetObjectPrivate(v8::Local pObj); - static void FreeObjectPrivate(void* p); + void SetObjectPrivate(v8::Local pObj, + std::unique_ptr p); + CJS_Object* GetObjectPrivate(v8::Local pObj); + static void FreeObjectPrivate(void* pPerObjectData); static void FreeObjectPrivate(v8::Local pObj); void Error(const WideString& message); -- cgit v1.2.3