Return v8::Local<> from CFXJSE_Context::GetGlobalContext()
No reason to cobble together a CFXJSE_Value when a local would do.
-- requires moving some functionality into fxv8 that was formerly
part of CFXJSE_Value.
-- introduce a new scope in one place, which made a method no longer
be const (probably never was logically const to begin with).
Bug: pdfium:1610
Change-Id: Id7064cf802f47b52abc3bbb9a83ff0b9624cf447
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76012
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
diff --git a/fxjs/fxv8.cpp b/fxjs/fxv8.cpp
index 65a0f51..6a952b0 100644
--- a/fxjs/fxv8.cpp
+++ b/fxjs/fxv8.cpp
@@ -213,6 +213,36 @@
return result;
}
+bool ReentrantHasObjectOwnPropertyHelper(v8::Isolate* pIsolate,
+ v8::Local<v8::Object> pObj,
+ ByteStringView bsUTF8PropertyName,
+ bool bUseTypeGetter) {
+ if (pObj.IsEmpty())
+ return false;
+
+ v8::TryCatch squash_exceptions(pIsolate);
+ v8::Local<v8::Context> pContext = pIsolate->GetCurrentContext();
+ v8::Local<v8::String> hKey =
+ fxv8::NewStringHelper(pIsolate, bsUTF8PropertyName);
+ return pObj->HasRealNamedProperty(pContext, hKey).FromJust() ||
+ (bUseTypeGetter &&
+ pObj->HasOwnProperty(pContext, hKey).FromMaybe(false));
+}
+
+bool ReentrantSetObjectOwnPropertyHelper(v8::Isolate* pIsolate,
+ v8::Local<v8::Object> pObj,
+ ByteStringView bsUTF8PropertyName,
+ v8::Local<v8::Value> pValue) {
+ ASSERT(!pValue.IsEmpty());
+ if (pObj.IsEmpty())
+ return false;
+
+ v8::TryCatch squash_exceptions(pIsolate);
+ v8::Local<v8::String> name = NewStringHelper(pIsolate, bsUTF8PropertyName);
+ return pObj->DefineOwnProperty(pIsolate->GetCurrentContext(), name, pValue)
+ .FromMaybe(false);
+}
+
bool ReentrantPutObjectPropertyHelper(v8::Isolate* pIsolate,
v8::Local<v8::Object> pObj,
ByteStringView bsUTF8PropertyName,
diff --git a/fxjs/fxv8.h b/fxjs/fxv8.h
index 0078c62..5e492c2 100644
--- a/fxjs/fxv8.h
+++ b/fxjs/fxv8.h
@@ -66,6 +66,14 @@
std::vector<WideString> ReentrantGetObjectPropertyNamesHelper(
v8::Isolate* pIsolate,
v8::Local<v8::Object> pObj);
+bool ReentrantHasObjectOwnPropertyHelper(v8::Isolate* pIsolate,
+ v8::Local<v8::Object> pObj,
+ ByteStringView bsUTF8PropertyName,
+ bool bUseTypeGetter);
+bool ReentrantSetObjectOwnPropertyHelper(v8::Isolate* pIsolate,
+ v8::Local<v8::Object> pObj,
+ ByteStringView bsUTF8PropertyName,
+ v8::Local<v8::Value> pValue);
bool ReentrantPutObjectPropertyHelper(v8::Isolate* pIsolate,
v8::Local<v8::Object> pObj,
ByteStringView bsUTF8PropertyName,
diff --git a/fxjs/xfa/cfxjse_context.cpp b/fxjs/xfa/cfxjse_context.cpp
index fb706fc..5de17cb 100644
--- a/fxjs/xfa/cfxjse_context.cpp
+++ b/fxjs/xfa/cfxjse_context.cpp
@@ -200,13 +200,14 @@
CFXJSE_Context::~CFXJSE_Context() = default;
-std::unique_ptr<CFXJSE_Value> CFXJSE_Context::GetGlobalObject() {
- CFXJSE_ScopeUtil_IsolateHandleContext scope(this);
+v8::Local<v8::Object> CFXJSE_Context::GetGlobalObject() {
+ v8::Isolate::Scope isolate_scope(GetIsolate());
+ v8::EscapableHandleScope handle_scope(GetIsolate());
v8::Local<v8::Context> hContext =
v8::Local<v8::Context>::New(GetIsolate(), m_hContext);
- v8::Local<v8::Object> hGlobalObject =
+ v8::Local<v8::Object> result =
hContext->Global()->GetPrototype().As<v8::Object>();
- return std::make_unique<CFXJSE_Value>(GetIsolate(), hGlobalObject);
+ return handle_scope.Escape(result);
}
v8::Local<v8::Context> CFXJSE_Context::GetContext() {
diff --git a/fxjs/xfa/cfxjse_context.h b/fxjs/xfa/cfxjse_context.h
index 876caa3..2724809 100644
--- a/fxjs/xfa/cfxjse_context.h
+++ b/fxjs/xfa/cfxjse_context.h
@@ -33,7 +33,8 @@
v8::Isolate* GetIsolate() const { return m_pIsolate.Get(); }
v8::Local<v8::Context> GetContext();
- std::unique_ptr<CFXJSE_Value> GetGlobalObject();
+ v8::Local<v8::Object> GetGlobalObject();
+
void AddClass(std::unique_ptr<CFXJSE_Class> pClass);
CFXJSE_Class* GetClassByName(ByteStringView szName) const;
void EnableCompatibleMode();
diff --git a/fxjs/xfa/cfxjse_engine.cpp b/fxjs/xfa/cfxjse_engine.cpp
index 93595ed..73f5dfd 100644
--- a/fxjs/xfa/cfxjse_engine.cpp
+++ b/fxjs/xfa/cfxjse_engine.cpp
@@ -582,41 +582,47 @@
CXFA_Node* variablesNode = pScriptNode->GetParent();
if (!variablesNode ||
- variablesNode->GetElementType() != XFA_Element::Variables)
+ variablesNode->GetElementType() != XFA_Element::Variables) {
return false;
+ }
auto it = m_mapVariableToContext.find(pScriptNode->JSObject());
if (it == m_mapVariableToContext.end() || !it->second)
return false;
CFXJSE_Context* pVariableContext = it->second.get();
- std::unique_ptr<CFXJSE_Value> pObject = pVariableContext->GetGlobalObject();
- auto hVariableValue = std::make_unique<CFXJSE_Value>();
+ v8::Local<v8::Object> pObject = pVariableContext->GetGlobalObject();
if (!bGetter) {
- pObject->SetObjectOwnProperty(GetIsolate(), szPropName, pValue);
+ fxv8::ReentrantSetObjectOwnPropertyHelper(GetIsolate(), pObject, szPropName,
+ pValue->GetValue(GetIsolate()));
return true;
}
- if (!pObject->HasObjectOwnProperty(GetIsolate(), szPropName, false))
+ if (!fxv8::ReentrantHasObjectOwnPropertyHelper(GetIsolate(), pObject,
+ szPropName, false)) {
return false;
+ }
- pObject->GetObjectProperty(GetIsolate(), szPropName, hVariableValue.get());
- if (hVariableValue->IsFunction(GetIsolate()))
- pValue->SetFunctionBind(GetIsolate(), hVariableValue.get(), pObject.get());
- else if (bGetter)
- pValue->Assign(GetIsolate(), hVariableValue.get());
- else
- hVariableValue.get()->Assign(GetIsolate(), pValue);
+ v8::Local<v8::Value> hVariableValue =
+ fxv8::ReentrantGetObjectPropertyHelper(GetIsolate(), pObject, szPropName);
+ if (fxv8::IsFunction(hVariableValue)) {
+ pValue->SetBoundFunction(GetIsolate(), hVariableValue.As<v8::Function>(),
+ pObject);
+ } else {
+ pValue->ForceSetValue(GetIsolate(), hVariableValue);
+ }
return true;
}
-void CFXJSE_Engine::RemoveBuiltInObjs(CFXJSE_Context* pContext) const {
+void CFXJSE_Engine::RemoveBuiltInObjs(CFXJSE_Context* pContext) {
+ CFXJSE_ScopeUtil_IsolateHandleContext scope(this);
const ByteStringView kObjNames[2] = {"Number", "Date"};
- std::unique_ptr<CFXJSE_Value> pObject = pContext->GetGlobalObject();
- auto hProp = std::make_unique<CFXJSE_Value>();
- for (const auto& obj : kObjNames) {
- if (pObject->GetObjectProperty(GetIsolate(), obj, hProp.get()))
- pObject->DeleteObjectProperty(GetIsolate(), obj);
+ v8::Local<v8::Object> pObject = pContext->GetGlobalObject();
+ for (const auto& name : kObjNames) {
+ if (!fxv8::ReentrantGetObjectPropertyHelper(GetIsolate(), pObject, name)
+ .IsEmpty()) {
+ fxv8::ReentrantDeleteObjectPropertyHelper(GetIsolate(), pObject, name);
+ }
}
}
diff --git a/fxjs/xfa/cfxjse_engine.h b/fxjs/xfa/cfxjse_engine.h
index 24834e5..326aa9b 100644
--- a/fxjs/xfa/cfxjse_engine.h
+++ b/fxjs/xfa/cfxjse_engine.h
@@ -115,7 +115,7 @@
private:
CFXJSE_Context* CreateVariablesContext(CXFA_Node* pScriptNode,
CXFA_Node* pSubform);
- void RemoveBuiltInObjs(CFXJSE_Context* pContext) const;
+ void RemoveBuiltInObjs(CFXJSE_Context* pContext);
bool QueryNodeByFlag(CXFA_Node* refNode,
WideStringView propname,
CFXJSE_Value* pValue,
diff --git a/fxjs/xfa/cfxjse_value.cpp b/fxjs/xfa/cfxjse_value.cpp
index 4ea8209..54ca810 100644
--- a/fxjs/xfa/cfxjse_value.cpp
+++ b/fxjs/xfa/cfxjse_value.cpp
@@ -181,40 +181,27 @@
bool CFXJSE_Value::SetObjectOwnProperty(v8::Isolate* pIsolate,
ByteStringView szPropName,
- CFXJSE_Value* lpPropValue) {
- ASSERT(lpPropValue);
+ CFXJSE_Value* pPropValue) {
CFXJSE_ScopeUtil_IsolateHandleRootContext scope(pIsolate);
v8::Local<v8::Value> hObject = v8::Local<v8::Value>::New(pIsolate, m_hValue);
if (!hObject->IsObject())
return false;
- v8::Local<v8::String> hPropName = fxv8::NewStringHelper(pIsolate, szPropName);
v8::Local<v8::Value> pValue =
- v8::Local<v8::Value>::New(pIsolate, lpPropValue->m_hValue);
- return hObject.As<v8::Object>()
- ->DefineOwnProperty(pIsolate->GetCurrentContext(), hPropName, pValue)
- .FromMaybe(false);
+ v8::Local<v8::Value>::New(pIsolate, pPropValue->m_hValue);
+ return fxv8::ReentrantSetObjectOwnPropertyHelper(
+ pIsolate, hObject.As<v8::Object>(), szPropName, pValue);
}
-bool CFXJSE_Value::SetFunctionBind(v8::Isolate* pIsolate,
- CFXJSE_Value* lpOldFunction,
- CFXJSE_Value* lpNewThis) {
- ASSERT(lpOldFunction);
- ASSERT(lpNewThis);
+bool CFXJSE_Value::SetBoundFunction(v8::Isolate* pIsolate,
+ v8::Local<v8::Function> hOldFunction,
+ v8::Local<v8::Object> hNewThis) {
+ ASSERT(!hOldFunction.IsEmpty());
+ ASSERT(!hNewThis.IsEmpty());
CFXJSE_ScopeUtil_IsolateHandleRootContext scope(pIsolate);
v8::Local<v8::Value> rgArgs[2];
- v8::Local<v8::Value> hOldFunction =
- v8::Local<v8::Value>::New(pIsolate, lpOldFunction->DirectGetValue());
- if (!fxv8::IsFunction(hOldFunction))
- return false;
-
rgArgs[0] = hOldFunction;
- v8::Local<v8::Value> hNewThis =
- v8::Local<v8::Value>::New(pIsolate, lpNewThis->DirectGetValue());
- if (hNewThis.IsEmpty())
- return false;
-
rgArgs[1] = hNewThis;
v8::Local<v8::String> hBinderFuncSource =
fxv8::NewStringHelper(pIsolate,
diff --git a/fxjs/xfa/cfxjse_value.h b/fxjs/xfa/cfxjse_value.h
index 4029a4f..542b384 100644
--- a/fxjs/xfa/cfxjse_value.h
+++ b/fxjs/xfa/cfxjse_value.h
@@ -75,9 +75,9 @@
bool SetObjectOwnProperty(v8::Isolate* pIsolate,
ByteStringView szPropName,
CFXJSE_Value* lpPropValue);
- bool SetFunctionBind(v8::Isolate* pIsolate,
- CFXJSE_Value* lpOldFunction,
- CFXJSE_Value* lpNewThis);
+ bool SetBoundFunction(v8::Isolate* pIsolate,
+ v8::Local<v8::Function> hOldFunction,
+ v8::Local<v8::Object> lpNewThis);
v8::Local<v8::Value> GetValue(v8::Isolate* pIsolate) const;
const v8::Global<v8::Value>& DirectGetValue() const { return m_hValue; }
