Move some more binding logic out of CFXJSE_Value.
Pass locals where possible rather than references to globals,
which means plumbing an isolate into a few place so we can
make a local from the global.
-- de-duplicates some code in CFXJSE_Engine in the process.
-- rename NewXFAObject() and remove arg that is always the same.
-- replace friendship with name testing method.
Bug: pdfium:1610
Change-Id: Iddf429bb7ee388090100ba60871c56503c292f07
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75931
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/xfa/cfxjse_class.cpp b/fxjs/xfa/cfxjse_class.cpp
index 411542a..86512c5 100644
--- a/fxjs/xfa/cfxjse_class.cpp
+++ b/fxjs/xfa/cfxjse_class.cpp
@@ -322,3 +322,8 @@
CFXJSE_Class::CFXJSE_Class(CFXJSE_Context* lpContext) : m_pContext(lpContext) {}
CFXJSE_Class::~CFXJSE_Class() = default;
+
+v8::Local<v8::FunctionTemplate> CFXJSE_Class::GetTemplate(
+ v8::Isolate* pIsolate) {
+ return v8::Local<v8::FunctionTemplate>::New(pIsolate, m_hTemplate);
+}
diff --git a/fxjs/xfa/cfxjse_class.h b/fxjs/xfa/cfxjse_class.h
index f5bc232..f23e2d9 100644
--- a/fxjs/xfa/cfxjse_class.h
+++ b/fxjs/xfa/cfxjse_class.h
@@ -12,7 +12,6 @@
#include "v8/include/v8.h"
class CFXJSE_Context;
-class CFXJSE_Value;
struct FXJSE_CLASS_DESCRIPTOR;
class CFXJSE_Class {
@@ -24,13 +23,11 @@
explicit CFXJSE_Class(CFXJSE_Context* lpContext);
~CFXJSE_Class();
+ bool IsName(ByteStringView name) const { return name == m_szClassName; }
CFXJSE_Context* GetContext() const { return m_pContext.Get(); }
- v8::Global<v8::FunctionTemplate>& GetTemplate() { return m_hTemplate; }
+ v8::Local<v8::FunctionTemplate> GetTemplate(v8::Isolate* pIsolate);
protected:
- friend class CFXJSE_Context;
- friend class CFXJSE_Value;
-
ByteString m_szClassName;
UnownedPtr<const FXJSE_CLASS_DESCRIPTOR> m_lpClassDefinition;
UnownedPtr<CFXJSE_Context> const m_pContext;
diff --git a/fxjs/xfa/cfxjse_context.cpp b/fxjs/xfa/cfxjse_context.cpp
index ffca7d5..27196b4 100644
--- a/fxjs/xfa/cfxjse_context.cpp
+++ b/fxjs/xfa/cfxjse_context.cpp
@@ -188,11 +188,8 @@
if (pGlobalClass) {
CFXJSE_Class* pGlobalClassObj =
CFXJSE_Class::Create(pContext.get(), pGlobalClass, true);
- ASSERT(pGlobalClassObj);
- v8::Local<v8::FunctionTemplate> hFunctionTemplate =
- v8::Local<v8::FunctionTemplate>::New(pIsolate,
- pGlobalClassObj->m_hTemplate);
- hObjectTemplate = hFunctionTemplate->InstanceTemplate();
+ hObjectTemplate =
+ pGlobalClassObj->GetTemplate(pIsolate)->InstanceTemplate();
} else {
hObjectTemplate = v8::ObjectTemplate::New(pIsolate);
hObjectTemplate->SetInternalFieldCount(2);
@@ -243,7 +240,7 @@
auto pClass =
std::find_if(m_rgClasses.begin(), m_rgClasses.end(),
[szName](const std::unique_ptr<CFXJSE_Class>& item) {
- return szName == item->m_szClassName;
+ return item->IsName(szName);
});
return pClass != m_rgClasses.end() ? pClass->get() : nullptr;
}
diff --git a/fxjs/xfa/cfxjse_engine.cpp b/fxjs/xfa/cfxjse_engine.cpp
index 5833fb7..46ed7bf 100644
--- a/fxjs/xfa/cfxjse_engine.cpp
+++ b/fxjs/xfa/cfxjse_engine.cpp
@@ -807,15 +807,9 @@
return pJSObject ? pJSObject->GetXFAObject() : nullptr;
}
-v8::Local<v8::Value> CFXJSE_Engine::NewXFAObject(
- CXFA_Object* obj,
- v8::Global<v8::FunctionTemplate>& tmpl) {
+v8::Local<v8::Object> CFXJSE_Engine::NewNormalXFAObject(CXFA_Object* obj) {
v8::EscapableHandleScope scope(GetIsolate());
- v8::Local<v8::FunctionTemplate> klass =
- v8::Local<v8::FunctionTemplate>::New(GetIsolate(), tmpl);
- v8::Local<v8::Object> object = klass->InstanceTemplate()
- ->NewInstance(m_JsContext->GetContext())
- .ToLocalChecked();
- FXJSE_UpdateObjectBinding(object, obj->JSObject());
+ v8::Local<v8::Object> object = obj->JSObject()->NewBoundV8Object(
+ GetIsolate(), GetJseNormalClass()->GetTemplate(GetIsolate()));
return scope.Escape(object);
}
diff --git a/fxjs/xfa/cfxjse_engine.h b/fxjs/xfa/cfxjse_engine.h
index 502f072..ba3c2a8 100644
--- a/fxjs/xfa/cfxjse_engine.h
+++ b/fxjs/xfa/cfxjse_engine.h
@@ -102,8 +102,7 @@
CXFA_Document* GetDocument() const { return m_pDocument.Get(); }
CXFA_Object* ToXFAObject(v8::Local<v8::Value> obj);
- v8::Local<v8::Value> NewXFAObject(CXFA_Object* obj,
- v8::Global<v8::FunctionTemplate>& tmpl);
+ v8::Local<v8::Object> NewNormalXFAObject(CXFA_Object* obj);
private:
CFXJSE_Context* CreateVariablesContext(CXFA_Node* pScriptNode,
diff --git a/fxjs/xfa/cfxjse_value.cpp b/fxjs/xfa/cfxjse_value.cpp
index 74a3a53..c034808 100644
--- a/fxjs/xfa/cfxjse_value.cpp
+++ b/fxjs/xfa/cfxjse_value.cpp
@@ -80,17 +80,12 @@
v8::Local<v8::Value>::New(GetIsolate(), m_hValue));
}
-void CFXJSE_Value::SetHostObject(CFXJSE_HostObject* lpObject,
+void CFXJSE_Value::SetHostObject(CFXJSE_HostObject* pObject,
CFXJSE_Class* pClass) {
CFXJSE_ScopeUtil_IsolateHandleRootContext scope(GetIsolate());
- v8::Local<v8::FunctionTemplate> hClass =
- v8::Local<v8::FunctionTemplate>::New(GetIsolate(), pClass->m_hTemplate);
- v8::Local<v8::Object> hObject =
- hClass->InstanceTemplate()
- ->NewInstance(GetIsolate()->GetCurrentContext())
- .ToLocalChecked();
- FXJSE_UpdateObjectBinding(hObject, lpObject);
- m_hValue.Reset(GetIsolate(), hObject);
+ m_hValue.Reset(GetIsolate(),
+ pObject->NewBoundV8Object(GetIsolate(),
+ pClass->GetTemplate(GetIsolate())));
}
void CFXJSE_Value::ClearHostObject() {
diff --git a/fxjs/xfa/cfxjse_value.h b/fxjs/xfa/cfxjse_value.h
index 2bf0bf4..a588633 100644
--- a/fxjs/xfa/cfxjse_value.h
+++ b/fxjs/xfa/cfxjse_value.h
@@ -52,7 +52,7 @@
void SetString(ByteStringView szString);
void SetFloat(float fFloat);
- void SetHostObject(CFXJSE_HostObject* lpObject, CFXJSE_Class* pClass);
+ void SetHostObject(CFXJSE_HostObject* pObject, CFXJSE_Class* pClass);
void ClearHostObject();
void SetArray(const std::vector<std::unique_ptr<CFXJSE_Value>>& values);
diff --git a/fxjs/xfa/cjx_container.cpp b/fxjs/xfa/cjx_container.cpp
index 3352f18..1ae7c09 100644
--- a/fxjs/xfa/cjx_container.cpp
+++ b/fxjs/xfa/cjx_container.cpp
@@ -45,6 +45,5 @@
pDoc->GetNodeOwner()->PersistList(pList);
auto* pEngine = static_cast<CFXJSE_Engine*>(runtime);
- return CJS_Result::Success(pEngine->NewXFAObject(
- pList, pEngine->GetJseNormalClass()->GetTemplate()));
+ return CJS_Result::Success(pEngine->NewNormalXFAObject(pList));
}
diff --git a/fxjs/xfa/cjx_layoutpseudomodel.cpp b/fxjs/xfa/cjx_layoutpseudomodel.cpp
index da8181f..e4a6f03 100644
--- a/fxjs/xfa/cjx_layoutpseudomodel.cpp
+++ b/fxjs/xfa/cjx_layoutpseudomodel.cpp
@@ -378,8 +378,7 @@
GetObjArray(pDocLayout, iIndex, wsType, bOnPageArea));
CFXJSE_Engine* pEngine = static_cast<CFXJSE_Engine*>(runtime);
- return CJS_Result::Success(pEngine->NewXFAObject(
- pArrayNodeList, pEngine->GetJseNormalClass()->GetTemplate()));
+ return CJS_Result::Success(pEngine->NewNormalXFAObject(pArrayNodeList));
}
CJS_Result CJX_LayoutPseudoModel::absPageCount(
diff --git a/fxjs/xfa/cjx_list.cpp b/fxjs/xfa/cjx_list.cpp
index 23d7e0d..75d63b3 100644
--- a/fxjs/xfa/cjx_list.cpp
+++ b/fxjs/xfa/cjx_list.cpp
@@ -92,9 +92,9 @@
if (index < 0 || cast_index >= GetXFAList()->GetLength())
return CJS_Result::Failure(JSMessage::kInvalidInputError);
- return CJS_Result::Success(static_cast<CFXJSE_Engine*>(runtime)->NewXFAObject(
- GetXFAList()->Item(cast_index),
- GetDocument()->GetScriptContext()->GetJseNormalClass()->GetTemplate()));
+ auto* pEngine = static_cast<CFXJSE_Engine*>(runtime);
+ return CJS_Result::Success(
+ pEngine->NewNormalXFAObject(GetXFAList()->Item(cast_index)));
}
void CJX_List::length(CFXJSE_Value* pValue,
diff --git a/fxjs/xfa/fxjse.cpp b/fxjs/xfa/fxjse.cpp
index 45e893f..b4f8d61 100644
--- a/fxjs/xfa/fxjse.cpp
+++ b/fxjs/xfa/fxjse.cpp
@@ -37,3 +37,14 @@
CJX_Object* CFXJSE_HostObject::AsCJXObject() {
return nullptr;
}
+
+v8::Local<v8::Object> CFXJSE_HostObject::NewBoundV8Object(
+ v8::Isolate* pIsolate,
+ v8::Local<v8::FunctionTemplate> tmpl) {
+ v8::Local<v8::Object> hObject =
+ tmpl->InstanceTemplate()
+ ->NewInstance(pIsolate->GetCurrentContext())
+ .ToLocalChecked();
+ FXJSE_UpdateObjectBinding(hObject, this);
+ return hObject;
+}
diff --git a/fxjs/xfa/fxjse.h b/fxjs/xfa/fxjse.h
index 2d70576..10750ef 100644
--- a/fxjs/xfa/fxjse.h
+++ b/fxjs/xfa/fxjse.h
@@ -37,6 +37,9 @@
virtual CFXJSE_FormCalcContext* AsFormCalcContext();
virtual CJX_Object* AsCJXObject();
+ v8::Local<v8::Object> NewBoundV8Object(v8::Isolate* pIsolate,
+ v8::Local<v8::FunctionTemplate> tmpl);
+
protected:
CFXJSE_HostObject();
};
