Make CJS_Global V8 callbacks conform to V8 API.
Several of these were not in strict conformance with the API
in v8/include/v8-template.h (see bug for specifics).
-- sort expected results file as these are now coming out in
C++ map order, not JS object order.
Bug: pdfium:1835, v8:12806
Change-Id: I9f38674520361b90df0f3a80b2cfc87730d7ebad
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/93670
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fxjs/cfxjs_engine.cpp b/fxjs/cfxjs_engine.cpp
index 6335c2a..ce15bf8 100644
--- a/fxjs/cfxjs_engine.cpp
+++ b/fxjs/cfxjs_engine.cpp
@@ -205,12 +205,14 @@
GetInstanceTemplate()->Set(sMethodName, fun, v8::ReadOnly);
}
- void DefineAllProperties(v8::GenericNamedPropertyQueryCallback pPropQurey,
- v8::GenericNamedPropertyGetterCallback pPropGet,
- v8::GenericNamedPropertySetterCallback pPropPut,
- v8::GenericNamedPropertyDeleterCallback pPropDel) {
+ void DefineAllProperties(
+ v8::GenericNamedPropertyQueryCallback pPropQurey,
+ v8::GenericNamedPropertyGetterCallback pPropGet,
+ v8::GenericNamedPropertySetterCallback pPropPut,
+ v8::GenericNamedPropertyDeleterCallback pPropDel,
+ v8::GenericNamedPropertyEnumeratorCallback pPropEnum) {
GetInstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
- pPropGet, pPropPut, pPropQurey, pPropDel, nullptr,
+ pPropGet, pPropPut, pPropQurey, pPropDel, pPropEnum,
v8::Local<v8::Value>(),
v8::PropertyHandlerFlags::kOnlyInterceptStrings));
}
@@ -429,12 +431,14 @@
v8::GenericNamedPropertyQueryCallback pPropQurey,
v8::GenericNamedPropertyGetterCallback pPropGet,
v8::GenericNamedPropertySetterCallback pPropPut,
- v8::GenericNamedPropertyDeleterCallback pPropDel) {
+ v8::GenericNamedPropertyDeleterCallback pPropDel,
+ v8::GenericNamedPropertyEnumeratorCallback pPropEnum) {
v8::Isolate::Scope isolate_scope(GetIsolate());
v8::HandleScope handle_scope(GetIsolate());
FXJS_PerIsolateData* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(nObjDefnID);
- pObjDef->DefineAllProperties(pPropQurey, pPropGet, pPropPut, pPropDel);
+ pObjDef->DefineAllProperties(pPropQurey, pPropGet, pPropPut, pPropDel,
+ pPropEnum);
}
void CFXJS_Engine::DefineObjConst(uint32_t nObjDefnID,
diff --git a/fxjs/cfxjs_engine.h b/fxjs/cfxjs_engine.h
index 304b4d3..56c9ad6 100644
--- a/fxjs/cfxjs_engine.h
+++ b/fxjs/cfxjs_engine.h
@@ -103,11 +103,13 @@
const char* sPropName,
v8::AccessorGetterCallback pPropGet,
v8::AccessorSetterCallback pPropPut);
- void DefineObjAllProperties(uint32_t nObjDefnID,
- v8::GenericNamedPropertyQueryCallback pPropQurey,
- v8::GenericNamedPropertyGetterCallback pPropGet,
- v8::GenericNamedPropertySetterCallback pPropPut,
- v8::GenericNamedPropertyDeleterCallback pPropDel);
+ void DefineObjAllProperties(
+ uint32_t nObjDefnID,
+ v8::GenericNamedPropertyQueryCallback pPropQurey,
+ v8::GenericNamedPropertyGetterCallback pPropGet,
+ v8::GenericNamedPropertySetterCallback pPropPut,
+ v8::GenericNamedPropertyDeleterCallback pPropDel,
+ v8::GenericNamedPropertyEnumeratorCallback pPropEnum);
void DefineObjConst(uint32_t nObjDefnID,
const char* sConstName,
v8::Local<v8::Value> pDefault);
diff --git a/fxjs/cjs_global.cpp b/fxjs/cjs_global.cpp
index 21814e3..216b548 100644
--- a/fxjs/cjs_global.cpp
+++ b/fxjs/cjs_global.cpp
@@ -19,6 +19,7 @@
#include "fxjs/js_define.h"
#include "fxjs/js_resources.h"
#include "third_party/base/check.h"
+#include "third_party/base/containers/contains.h"
#include "v8/include/v8-isolate.h"
namespace {
@@ -34,12 +35,8 @@
return;
WideString wsProp = fxv8::ToWideString(info.GetIsolate(), property);
- CJS_Result result = pObj->QueryProperty(wsProp);
- v8::PropertyAttribute attr = !result.HasError()
- ? v8::PropertyAttribute::DontDelete
- : v8::PropertyAttribute::None;
-
- info.GetReturnValue().Set(static_cast<int>(attr));
+ if (pObj->HasProperty(wsProp))
+ info.GetReturnValue().Set(static_cast<int>(v8::PropertyAttribute::None));
}
void JSSpecialPropGet(v8::Local<v8::String> property,
@@ -79,7 +76,9 @@
if (result.HasError()) {
pRuntime->Error(
JSFormatErrorString("global", "PutProperty", result.Error()));
+ return;
}
+ info.GetReturnValue().Set(value);
}
void JSSpecialPropDel(v8::Local<v8::String> property,
@@ -93,7 +92,20 @@
return;
WideString wsProp = fxv8::ToWideString(info.GetIsolate(), property);
- pObj->DelProperty(pRuntime, wsProp); // Silently ignore error.
+ if (pObj->DelProperty(pRuntime, wsProp))
+ info.GetReturnValue().Set(true);
+}
+
+void JSSpecialPropEnum(const v8::PropertyCallbackInfo<v8::Array>& info) {
+ auto pObj = JSGetObject<CJS_Global>(info.GetIsolate(), info.Holder());
+ if (!pObj)
+ return;
+
+ CJS_Runtime* pRuntime = pObj->GetRuntime();
+ if (!pRuntime)
+ return;
+
+ pObj->EnumProperties(pRuntime, info);
}
v8::Local<v8::String> GetV8StringFromName(v8::Isolate* pIsolate,
@@ -153,11 +165,17 @@
JSSpecialPropDel(GetV8StringFromName(info.GetIsolate(), property), info);
}
+void CJS_Global::enumprop_static(
+ const v8::PropertyCallbackInfo<v8::Array>& info) {
+ JSSpecialPropEnum(info);
+}
+
// static
void CJS_Global::DefineAllProperties(CFXJS_Engine* pEngine) {
pEngine->DefineObjAllProperties(
ObjDefnID, CJS_Global::queryprop_static, CJS_Global::getprop_static,
- CJS_Global::putprop_static, CJS_Global::delprop_static);
+ CJS_Global::putprop_static, CJS_Global::delprop_static,
+ CJS_Global::enumprop_static);
}
// static
@@ -184,21 +202,18 @@
m_pGlobalData.Release()->Release();
}
-CJS_Result CJS_Global::QueryProperty(const WideString& propname) {
- if (propname.EqualsASCII("setPersistent"))
- return CJS_Result::Success();
-
- return CJS_Result::Failure(JSMessage::kUnknownProperty);
+bool CJS_Global::HasProperty(const WideString& propname) {
+ return pdfium::Contains(m_MapGlobal, propname.ToDefANSI());
}
-CJS_Result CJS_Global::DelProperty(CJS_Runtime* pRuntime,
- const WideString& propname) {
+bool CJS_Global::DelProperty(CJS_Runtime* pRuntime,
+ const WideString& propname) {
auto it = m_MapGlobal.find(propname.ToDefANSI());
if (it == m_MapGlobal.end())
- return CJS_Result::Failure(JSMessage::kUnknownProperty);
+ return false;
it->second->bDeleted = true;
- return CJS_Result::Success();
+ return true;
}
CJS_Result CJS_Global::GetProperty(CJS_Runtime* pRuntime,
@@ -264,6 +279,21 @@
return CJS_Result::Failure(JSMessage::kObjectTypeError);
}
+void CJS_Global::EnumProperties(
+ CJS_Runtime* pRuntime,
+ const v8::PropertyCallbackInfo<v8::Array>& info) {
+ v8::Local<v8::Array> result = pRuntime->NewArray();
+ int idx = 0;
+ for (const auto& it : m_MapGlobal) {
+ if (it.second->bDeleted)
+ continue;
+ v8::Local<v8::Name> name = pRuntime->NewString(it.first.AsStringView());
+ pRuntime->PutArrayElement(result, idx, name);
+ ++idx;
+ }
+ info.GetReturnValue().Set(result);
+}
+
CJS_Result CJS_Global::setPersistent(
CJS_Runtime* pRuntime,
const std::vector<v8::Local<v8::Value>>& params) {
diff --git a/fxjs/cjs_global.h b/fxjs/cjs_global.h
index 994589f..47a43ce 100644
--- a/fxjs/cjs_global.h
+++ b/fxjs/cjs_global.h
@@ -44,6 +44,7 @@
const v8::PropertyCallbackInfo<v8::Value>& info);
static void delprop_static(v8::Local<v8::Name> property,
const v8::PropertyCallbackInfo<v8::Boolean>& info);
+ static void enumprop_static(const v8::PropertyCallbackInfo<v8::Array>& info);
static void setPersistent_static(
const v8::FunctionCallbackInfo<v8::Value>& info);
@@ -53,12 +54,14 @@
CJS_Result setPersistent(CJS_Runtime* pRuntime,
const std::vector<v8::Local<v8::Value>>& params);
- CJS_Result QueryProperty(const WideString& propname);
- CJS_Result DelProperty(CJS_Runtime* pRuntime, const WideString& propname);
+ bool HasProperty(const WideString& propname);
+ bool DelProperty(CJS_Runtime* pRuntime, const WideString& propname);
CJS_Result GetProperty(CJS_Runtime* pRuntime, const WideString& propname);
CJS_Result SetProperty(CJS_Runtime* pRuntime,
const WideString& propname,
v8::Local<v8::Value> vp);
+ void EnumProperties(CJS_Runtime* pRuntime,
+ const v8::PropertyCallbackInfo<v8::Array>& info);
private:
struct JSGlobalData : public CFX_Value {
diff --git a/testing/SUPPRESSIONS b/testing/SUPPRESSIONS
index 6151eaa..bac9e2d 100644
--- a/testing/SUPPRESSIONS
+++ b/testing/SUPPRESSIONS
@@ -314,9 +314,6 @@
bug_679643.in * * noxfa *
bug_735912.in * * noxfa *
-# TODO(v8:12806) Fix regression and remove this suppression.
-globals.in * * * *
-
# JS tests in nov8 mode expect empty results. This one will
# not be empty as the callback is not js-based.
named_action.in * nov8 * *
diff --git a/testing/resources/javascript/globals_expected.txt b/testing/resources/javascript/globals_expected.txt
index fcebd70..26c878b 100644
--- a/testing/resources/javascript/globals_expected.txt
+++ b/testing/resources/javascript/globals_expected.txt
@@ -13,14 +13,13 @@
Alert: ************ After Setup ************
Alert: Enumerable Globals:
Alert: setPersistent = function setPersistent() { [native code] }, own property = true
-Alert: true_var = true, own property = true
Alert: false_var = false, own property = true
-Alert: zero_var = 0, own property = true
-Alert: number_var = -3.918, own property = true
-Alert: string_var = This is a string, own property = true
-Alert: object_var = [object Object], own property = true
Alert: null_var = null, own property = true
-Alert: undefined_var = undefined, own property = true
+Alert: number_var = -3.918, own property = true
+Alert: object_var = [object Object], own property = true
+Alert: string_var = This is a string, own property = true
+Alert: true_var = true, own property = true
+Alert: zero_var = 0, own property = true
Alert: Expected Globals:
Alert: true_var = true
Alert: false_var = false
@@ -49,14 +48,13 @@
Alert: ************ After Setup and Persist false ************
Alert: Enumerable Globals:
Alert: setPersistent = function setPersistent() { [native code] }, own property = true
-Alert: true_var = true, own property = true
Alert: false_var = false, own property = true
-Alert: zero_var = 0, own property = true
-Alert: number_var = -3.918, own property = true
-Alert: string_var = This is a string, own property = true
-Alert: object_var = [object Object], own property = true
Alert: null_var = null, own property = true
-Alert: undefined_var = undefined, own property = true
+Alert: number_var = -3.918, own property = true
+Alert: object_var = [object Object], own property = true
+Alert: string_var = This is a string, own property = true
+Alert: true_var = true, own property = true
+Alert: zero_var = 0, own property = true
Alert: Expected Globals:
Alert: true_var = true
Alert: false_var = false
@@ -93,14 +91,13 @@
Alert: ************ After Setup and Persist true ************
Alert: Enumerable Globals:
Alert: setPersistent = function setPersistent() { [native code] }, own property = true
-Alert: true_var = true, own property = true
Alert: false_var = false, own property = true
-Alert: zero_var = 0, own property = true
-Alert: number_var = -3.918, own property = true
-Alert: string_var = This is a string, own property = true
-Alert: object_var = [object Object], own property = true
Alert: null_var = null, own property = true
-Alert: undefined_var = undefined, own property = true
+Alert: number_var = -3.918, own property = true
+Alert: object_var = [object Object], own property = true
+Alert: string_var = This is a string, own property = true
+Alert: true_var = true, own property = true
+Alert: zero_var = 0, own property = true
Alert: Expected Globals:
Alert: true_var = true
Alert: false_var = false
