Avoid allocating CFXJSE_Values for |this| in poperty callbacks
Let it flow further down the code as a v8::Local<>. The only real
consequence is that we need to expose a delete wrapper for locals
that was formerly part of the CFXJSE_Value code.
Bug: pdfium:1610
Change-Id: I95dd709c4ff3716e3e37e6824b7ee4d0a00d5375
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76011
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/fxv8.cpp b/fxjs/fxv8.cpp
index 0f2c656..65a0f51 100644
--- a/fxjs/fxv8.cpp
+++ b/fxjs/fxv8.cpp
@@ -227,6 +227,15 @@
return result.IsJust() && result.FromJust();
}
+void ReentrantDeleteObjectPropertyHelper(v8::Isolate* pIsolate,
+ v8::Local<v8::Object> pObj,
+ ByteStringView bsUTF8PropertyName) {
+ v8::TryCatch squash_exceptions(pIsolate);
+ pObj->Delete(pIsolate->GetCurrentContext(),
+ fxv8::NewStringHelper(pIsolate, bsUTF8PropertyName))
+ .FromJust();
+}
+
bool ReentrantPutArrayElementHelper(v8::Isolate* pIsolate,
v8::Local<v8::Array> pArray,
unsigned index,
diff --git a/fxjs/fxv8.h b/fxjs/fxv8.h
index d5c19fa..0078c62 100644
--- a/fxjs/fxv8.h
+++ b/fxjs/fxv8.h
@@ -74,6 +74,9 @@
v8::Local<v8::Array> pArray,
unsigned index,
v8::Local<v8::Value> pValue);
+void ReentrantDeleteObjectPropertyHelper(v8::Isolate* pIsolate,
+ v8::Local<v8::Object> pObj,
+ ByteStringView bsUTF8PropertyName);
v8::Local<v8::Value> ReentrantGetArrayElementHelper(v8::Isolate* pIsolate,
v8::Local<v8::Array> pArray,
unsigned index);
diff --git a/fxjs/xfa/cfxjse_class.cpp b/fxjs/xfa/cfxjse_class.cpp
index b34c172..d2c1749 100644
--- a/fxjs/xfa/cfxjse_class.cpp
+++ b/fxjs/xfa/cfxjse_class.cpp
@@ -114,7 +114,7 @@
void DynPropGetterAdapter(v8::Isolate* pIsolate,
const FXJSE_CLASS_DESCRIPTOR* lpClass,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue) {
ASSERT(lpClass);
@@ -151,7 +151,7 @@
void DynPropSetterAdapter(v8::Isolate* pIsolate,
const FXJSE_CLASS_DESCRIPTOR* lpClass,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue) {
ASSERT(lpClass);
@@ -167,7 +167,7 @@
bool DynPropQueryAdapter(v8::Isolate* pIsolate,
const FXJSE_CLASS_DESCRIPTOR* lpClass,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName) {
ASSERT(lpClass);
int32_t nPropType =
@@ -180,7 +180,6 @@
void NamedPropertyQueryCallback(
v8::Local<v8::Name> property,
const v8::PropertyCallbackInfo<v8::Integer>& info) {
- v8::Local<v8::Object> thisObject = info.Holder();
const FXJSE_CLASS_DESCRIPTOR* pClass =
AsClassDescriptor(info.Data().As<v8::External>()->Value());
if (!pClass)
@@ -189,9 +188,7 @@
v8::HandleScope scope(info.GetIsolate());
v8::String::Utf8Value szPropName(info.GetIsolate(), property);
ByteStringView szFxPropName(*szPropName, szPropName.length());
- auto pThisValue =
- std::make_unique<CFXJSE_Value>(info.GetIsolate(), thisObject);
- if (DynPropQueryAdapter(info.GetIsolate(), pClass, pThisValue.get(),
+ if (DynPropQueryAdapter(info.GetIsolate(), pClass, info.Holder(),
szFxPropName)) {
info.GetReturnValue().Set(v8::DontDelete);
return;
@@ -203,7 +200,6 @@
void NamedPropertyGetterCallback(
v8::Local<v8::Name> property,
const v8::PropertyCallbackInfo<v8::Value>& info) {
- v8::Local<v8::Object> thisObject = info.Holder();
const FXJSE_CLASS_DESCRIPTOR* pClass =
AsClassDescriptor(info.Data().As<v8::External>()->Value());
if (!pClass)
@@ -211,11 +207,9 @@
v8::String::Utf8Value szPropName(info.GetIsolate(), property);
ByteStringView szFxPropName(*szPropName, szPropName.length());
- auto pThisValue =
- std::make_unique<CFXJSE_Value>(info.GetIsolate(), thisObject);
auto pNewValue = std::make_unique<CFXJSE_Value>();
- DynPropGetterAdapter(info.GetIsolate(), pClass, pThisValue.get(),
- szFxPropName, pNewValue.get());
+ DynPropGetterAdapter(info.GetIsolate(), pClass, info.Holder(), szFxPropName,
+ pNewValue.get());
info.GetReturnValue().Set(pNewValue->DirectGetValue());
}
@@ -223,7 +217,6 @@
v8::Local<v8::Name> property,
v8::Local<v8::Value> value,
const v8::PropertyCallbackInfo<v8::Value>& info) {
- v8::Local<v8::Object> thisObject = info.Holder();
const FXJSE_CLASS_DESCRIPTOR* pClass =
AsClassDescriptor(info.Data().As<v8::External>()->Value());
if (!pClass)
@@ -231,11 +224,9 @@
v8::String::Utf8Value szPropName(info.GetIsolate(), property);
ByteStringView szFxPropName(*szPropName, szPropName.length());
- auto pThisValue =
- std::make_unique<CFXJSE_Value>(info.GetIsolate(), thisObject);
auto pNewValue = std::make_unique<CFXJSE_Value>(info.GetIsolate(), value);
- DynPropSetterAdapter(info.GetIsolate(), pClass, pThisValue.get(),
- szFxPropName, pNewValue.get());
+ DynPropSetterAdapter(info.GetIsolate(), pClass, info.Holder(), szFxPropName,
+ pNewValue.get());
info.GetReturnValue().Set(value);
}
diff --git a/fxjs/xfa/cfxjse_engine.cpp b/fxjs/xfa/cfxjse_engine.cpp
index 9689403..93595ed 100644
--- a/fxjs/xfa/cfxjse_engine.cpp
+++ b/fxjs/xfa/cfxjse_engine.cpp
@@ -197,7 +197,7 @@
// static
void CFXJSE_Engine::GlobalPropertySetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue) {
CXFA_Object* lpOrginalNode = ToObject(pIsolate, pObject);
@@ -217,7 +217,8 @@
return;
}
if (lpOrginalNode->IsThisProxy() && pValue && pValue->IsUndefined(pIsolate)) {
- pObject->DeleteObjectProperty(lpScriptContext->GetIsolate(), szPropName);
+ fxv8::ReentrantDeleteObjectPropertyHelper(lpScriptContext->GetIsolate(),
+ pObject, szPropName);
return;
}
CXFA_FFNotify* pNotify = pDoc->GetNotify();
@@ -238,7 +239,7 @@
// static
void CFXJSE_Engine::GlobalPropertyGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue) {
CXFA_Object* pOriginalObject = ToObject(pIsolate, pObject);
@@ -312,10 +313,10 @@
// static
int32_t CFXJSE_Engine::GlobalPropTypeGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pOriginalValue,
+ v8::Local<v8::Object> pHolder,
ByteStringView szPropName,
bool bQueryIn) {
- CXFA_Object* pObject = ToObject(pIsolate, pOriginalValue);
+ CXFA_Object* pObject = ToObject(pIsolate, pHolder);
if (!pObject)
return FXJSE_ClassPropType_None;
@@ -330,11 +331,11 @@
// static
void CFXJSE_Engine::NormalPropertyGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pOriginalValue,
+ v8::Local<v8::Object> pHolder,
ByteStringView szPropName,
CFXJSE_Value* pReturnValue) {
pReturnValue->SetUndefined(pIsolate); // Assume failure.
- CXFA_Object* pOriginalObject = ToObject(pIsolate, pOriginalValue);
+ CXFA_Object* pOriginalObject = ToObject(pIsolate, pHolder);
if (!pOriginalObject)
return;
@@ -409,10 +410,10 @@
// static
void CFXJSE_Engine::NormalPropertySetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pOriginalValue,
+ v8::Local<v8::Object> pHolder,
ByteStringView szPropName,
CFXJSE_Value* pReturnValue) {
- CXFA_Object* pOriginalObject = ToObject(pIsolate, pOriginalValue);
+ CXFA_Object* pOriginalObject = ToObject(pIsolate, pHolder);
if (!pOriginalObject)
return;
@@ -464,10 +465,10 @@
}
int32_t CFXJSE_Engine::NormalPropTypeGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pOriginalValue,
+ v8::Local<v8::Object> pHolder,
ByteStringView szPropName,
bool bQueryIn) {
- CXFA_Object* pObject = ToObject(pIsolate, pOriginalValue);
+ CXFA_Object* pObject = ToObject(pIsolate, pHolder);
if (!pObject)
return FXJSE_ClassPropType_None;
diff --git a/fxjs/xfa/cfxjse_engine.h b/fxjs/xfa/cfxjse_engine.h
index fbb5bb8..24834e5 100644
--- a/fxjs/xfa/cfxjse_engine.h
+++ b/fxjs/xfa/cfxjse_engine.h
@@ -48,30 +48,30 @@
static CXFA_Object* ToObject(v8::Isolate* pIsolate, CFXJSE_Value* pValue);
static CXFA_Object* ToObject(CFXJSE_HostObject* pHostObj);
static void GlobalPropertyGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue);
static void GlobalPropertySetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue);
static void NormalPropertyGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue);
static void NormalPropertySetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue);
static CJS_Result NormalMethodCall(
const v8::FunctionCallbackInfo<v8::Value>& info,
const WideString& functionName);
static int32_t NormalPropTypeGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
bool bQueryIn);
static int32_t GlobalPropTypeGetter(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
bool bQueryIn);
diff --git a/fxjs/xfa/cfxjse_value.cpp b/fxjs/xfa/cfxjse_value.cpp
index 478e97e..4ea8209 100644
--- a/fxjs/xfa/cfxjse_value.cpp
+++ b/fxjs/xfa/cfxjse_value.cpp
@@ -151,15 +151,14 @@
return true;
}
-bool CFXJSE_Value::DeleteObjectProperty(v8::Isolate* pIsolate,
+void CFXJSE_Value::DeleteObjectProperty(v8::Isolate* pIsolate,
ByteStringView szPropName) {
CFXJSE_ScopeUtil_IsolateHandleRootContext scope(pIsolate);
v8::Local<v8::Value> hObject = v8::Local<v8::Value>::New(pIsolate, m_hValue);
- return hObject->IsObject() &&
- hObject.As<v8::Object>()
- ->Delete(pIsolate->GetCurrentContext(),
- fxv8::NewStringHelper(pIsolate, szPropName))
- .FromJust();
+ if (hObject->IsObject()) {
+ fxv8::ReentrantDeleteObjectPropertyHelper(
+ pIsolate, hObject.As<v8::Object>(), szPropName);
+ }
}
bool CFXJSE_Value::HasObjectOwnProperty(v8::Isolate* pIsolate,
diff --git a/fxjs/xfa/cfxjse_value.h b/fxjs/xfa/cfxjse_value.h
index d02f00a..4029a4f 100644
--- a/fxjs/xfa/cfxjse_value.h
+++ b/fxjs/xfa/cfxjse_value.h
@@ -68,7 +68,7 @@
bool GetObjectPropertyByIdx(v8::Isolate* pIsolate,
uint32_t uPropIdx,
CFXJSE_Value* lpPropValue);
- bool DeleteObjectProperty(v8::Isolate* pIsolate, ByteStringView szPropName);
+ void DeleteObjectProperty(v8::Isolate* pIsolate, ByteStringView szPropName);
bool HasObjectOwnProperty(v8::Isolate* pIsolate,
ByteStringView szPropName,
bool bUseTypeGetter);
diff --git a/fxjs/xfa/fxjse.h b/fxjs/xfa/fxjse.h
index 5d70351..a1cb183 100644
--- a/fxjs/xfa/fxjse.h
+++ b/fxjs/xfa/fxjse.h
@@ -51,11 +51,11 @@
CFXJSE_HostObject* pThis,
const v8::FunctionCallbackInfo<v8::Value>& info);
typedef void (*FXJSE_PropAccessor)(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
CFXJSE_Value* pValue);
typedef int32_t (*FXJSE_PropTypeGetter)(v8::Isolate* pIsolate,
- CFXJSE_Value* pObject,
+ v8::Local<v8::Object> pObject,
ByteStringView szPropName,
bool bQueryIn);