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>

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<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 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 <utility>
 #include <vector>
 
 #include "fxjs/cjs_object.h"
@@ -49,16 +50,14 @@ std::vector<v8::Local<v8::Value>> ExpandKeywordParams(
 
 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 @@ void JSPropGetter(const char* prop_name_string,
   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 @@ void JSPropSetter(const char* prop_name_string,
   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 @@ void JSMethod(const char* method_name_string,
   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 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<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 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<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 @@ void app::ClearTimerCommon(CJS_Runtime* pRuntime, v8::Local<v8::Value> param) {
   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 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<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 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<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 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<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 @@ CJS_Return Document::print(CJS_Runtime* pRuntime,
       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 @@ CJS_Return Document::addIcon(CJS_Runtime* pRuntime,
     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 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<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 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<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 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<CJS_Object*>(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<CJS_Object*>(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<CJS_Object*>(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<CJS_Object*>(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<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 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<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 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<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 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<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 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<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 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<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 f1555b2e96..2161076e36 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 @@ 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<v8::Object> pObj) {
@@ -48,7 +52,7 @@ class CFXJS_PerObjectData {
   }
 
   const int m_ObjDefID;
-  void* m_pPrivate;
+  std::unique_ptr<CJS_Object> 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<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 @@ void* CFXJS_Engine::GetObjectPrivate(v8::Local<v8::Object> pObj) {
           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 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<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);