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);