Skip to content

Commit e1a7699

Browse files
camillobruniCommit Bot
camillobruni
authored and
Commit Bot
committed
[api][runtime] Support all-in ctors of {Named,Indexed}PropertyHandlerConfiguration
- Explicitly allows construction of {Named,Indexed}PropertyHandlerConfiguration with all the members filled. Bug: v8:7612 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I426ea33846b5dbf2b3482c722c963a6e4b0abded Reviewed-on: https://chromium-review.googlesource.com/1163882 Reviewed-by: Toon Verwaest <[email protected]> Reviewed-by: Adam Klein <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/master@{#55142}
1 parent 660e7ae commit e1a7699

4 files changed

Lines changed: 247 additions & 35 deletions

File tree

‎include/v8.h‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5893,6 +5893,26 @@ enum class PropertyHandlerFlags {
58935893
};
58945894

58955895
struct NamedPropertyHandlerConfiguration {
5896+
NamedPropertyHandlerConfiguration(
5897+
GenericNamedPropertyGetterCallback getter,
5898+
GenericNamedPropertySetterCallback setter,
5899+
GenericNamedPropertyQueryCallback query,
5900+
GenericNamedPropertyDeleterCallback deleter,
5901+
GenericNamedPropertyEnumeratorCallback enumerator,
5902+
GenericNamedPropertyDefinerCallback definer,
5903+
GenericNamedPropertyDescriptorCallback descriptor,
5904+
Local<Value> data = Local<Value>(),
5905+
PropertyHandlerFlags flags = PropertyHandlerFlags::kNone)
5906+
: getter(getter),
5907+
setter(setter),
5908+
query(query),
5909+
deleter(deleter),
5910+
enumerator(enumerator),
5911+
definer(definer),
5912+
descriptor(descriptor),
5913+
data(data),
5914+
flags(flags) {}
5915+
58965916
NamedPropertyHandlerConfiguration(
58975917
/** Note: getter is required */
58985918
GenericNamedPropertyGetterCallback getter = 0,
@@ -5944,6 +5964,25 @@ struct NamedPropertyHandlerConfiguration {
59445964

59455965

59465966
struct IndexedPropertyHandlerConfiguration {
5967+
IndexedPropertyHandlerConfiguration(
5968+
IndexedPropertyGetterCallback getter,
5969+
IndexedPropertySetterCallback setter, IndexedPropertyQueryCallback query,
5970+
IndexedPropertyDeleterCallback deleter,
5971+
IndexedPropertyEnumeratorCallback enumerator,
5972+
IndexedPropertyDefinerCallback definer,
5973+
IndexedPropertyDescriptorCallback descriptor,
5974+
Local<Value> data = Local<Value>(),
5975+
PropertyHandlerFlags flags = PropertyHandlerFlags::kNone)
5976+
: getter(getter),
5977+
setter(setter),
5978+
query(query),
5979+
deleter(deleter),
5980+
enumerator(enumerator),
5981+
definer(definer),
5982+
descriptor(descriptor),
5983+
data(data),
5984+
flags(flags) {}
5985+
59475986
IndexedPropertyHandlerConfiguration(
59485987
/** Note: getter is required */
59495988
IndexedPropertyGetterCallback getter = 0,

‎src/api.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,10 +1760,6 @@ static i::Handle<i::InterceptorInfo> CreateInterceptorInfo(
17601760
i::Isolate* isolate, Getter getter, Setter setter, Query query,
17611761
Descriptor descriptor, Deleter remover, Enumerator enumerator,
17621762
Definer definer, Local<Value> data, PropertyHandlerFlags flags) {
1763-
// Either intercept attributes or descriptor.
1764-
DCHECK(query == nullptr || descriptor == nullptr);
1765-
// Only use descriptor callback with definer callback.
1766-
DCHECK(query == nullptr || definer == nullptr);
17671763
auto obj = i::Handle<i::InterceptorInfo>::cast(
17681764
isolate->factory()->NewStruct(i::INTERCEPTOR_INFO_TYPE, i::TENURED));
17691765
obj->set_flags(0);

‎src/objects.cc‎

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7833,41 +7833,42 @@ Maybe<bool> GetPropertyDescriptorWithInterceptor(LookupIterator* it,
78337833
}
78347834
}
78357835

7836-
if (it->state() == LookupIterator::INTERCEPTOR) {
7837-
Isolate* isolate = it->isolate();
7838-
Handle<InterceptorInfo> interceptor = it->GetInterceptor();
7839-
if (!interceptor->descriptor()->IsUndefined(isolate)) {
7840-
Handle<Object> result;
7841-
Handle<JSObject> holder = it->GetHolder<JSObject>();
7836+
if (it->state() != LookupIterator::INTERCEPTOR) return Just(false);
78427837

7843-
Handle<Object> receiver = it->GetReceiver();
7844-
if (!receiver->IsJSReceiver()) {
7845-
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
7846-
isolate, receiver, Object::ConvertReceiver(isolate, receiver),
7847-
Nothing<bool>());
7848-
}
7838+
Isolate* isolate = it->isolate();
7839+
Handle<InterceptorInfo> interceptor = it->GetInterceptor();
7840+
if (interceptor->descriptor()->IsUndefined(isolate)) return Just(false);
78497841

7850-
PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
7851-
*holder, kDontThrow);
7852-
if (it->IsElement()) {
7853-
result = args.CallIndexedDescriptor(interceptor, it->index());
7854-
} else {
7855-
result = args.CallNamedDescriptor(interceptor, it->name());
7856-
}
7857-
if (!result.is_null()) {
7858-
// Request successfully intercepted, try to set the property
7859-
// descriptor.
7860-
Utils::ApiCheck(
7861-
PropertyDescriptor::ToPropertyDescriptor(isolate, result, desc),
7862-
it->IsElement() ? "v8::IndexedPropertyDescriptorCallback"
7863-
: "v8::NamedPropertyDescriptorCallback",
7864-
"Invalid property descriptor.");
7842+
Handle<Object> result;
7843+
Handle<JSObject> holder = it->GetHolder<JSObject>();
78657844

7866-
return Just(true);
7867-
}
7868-
it->Next();
7869-
}
7845+
Handle<Object> receiver = it->GetReceiver();
7846+
if (!receiver->IsJSReceiver()) {
7847+
ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, receiver,
7848+
Object::ConvertReceiver(isolate, receiver),
7849+
Nothing<bool>());
7850+
}
7851+
7852+
PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
7853+
*holder, kDontThrow);
7854+
if (it->IsElement()) {
7855+
result = args.CallIndexedDescriptor(interceptor, it->index());
7856+
} else {
7857+
result = args.CallNamedDescriptor(interceptor, it->name());
78707858
}
7859+
if (!result.is_null()) {
7860+
// Request successfully intercepted, try to set the property
7861+
// descriptor.
7862+
Utils::ApiCheck(
7863+
PropertyDescriptor::ToPropertyDescriptor(isolate, result, desc),
7864+
it->IsElement() ? "v8::IndexedPropertyDescriptorCallback"
7865+
: "v8::NamedPropertyDescriptorCallback",
7866+
"Invalid property descriptor.");
7867+
7868+
return Just(true);
7869+
}
7870+
7871+
it->Next();
78717872
return Just(false);
78727873
}
78737874
} // namespace

‎test/unittests/api/interceptor-unittest.cc‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,180 @@ TEST_F(InterceptorTest, FreezeApiObjectWithInterceptor) {
2929
}
3030

3131
} // namespace
32+
33+
namespace internal {
34+
namespace {
35+
36+
class InterceptorLoggingTest : public TestWithNativeContext {
37+
public:
38+
InterceptorLoggingTest() {}
39+
40+
static const int kTestIndex = 0;
41+
42+
static void NamedPropertyGetter(Local<v8::Name> name,
43+
const v8::PropertyCallbackInfo<Value>& info) {
44+
LogCallback(info, "named getter");
45+
}
46+
47+
static void NamedPropertySetter(Local<v8::Name> name, Local<v8::Value> value,
48+
const v8::PropertyCallbackInfo<Value>& info) {
49+
LogCallback(info, "named setter");
50+
}
51+
52+
static void NamedPropertyQuery(
53+
Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Integer>& info) {
54+
LogCallback(info, "named query");
55+
}
56+
57+
static void NamedPropertyDeleter(
58+
Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Boolean>& info) {
59+
LogCallback(info, "named deleter");
60+
}
61+
62+
static void NamedPropertyEnumerator(
63+
const v8::PropertyCallbackInfo<Array>& info) {
64+
LogCallback(info, "named enumerator");
65+
}
66+
67+
static void NamedPropertyDefiner(
68+
Local<v8::Name> name, const v8::PropertyDescriptor& desc,
69+
const v8::PropertyCallbackInfo<Value>& info) {
70+
LogCallback(info, "named definer");
71+
}
72+
73+
static void NamedPropertyDescriptor(
74+
Local<v8::Name> name, const v8::PropertyCallbackInfo<Value>& info) {
75+
LogCallback(info, "named descriptor");
76+
}
77+
78+
static void IndexedPropertyGetter(
79+
uint32_t index, const v8::PropertyCallbackInfo<Value>& info) {
80+
LogCallback(info, "indexed getter");
81+
}
82+
83+
static void IndexedPropertySetter(
84+
uint32_t index, Local<v8::Value> value,
85+
const v8::PropertyCallbackInfo<Value>& info) {
86+
LogCallback(info, "indexed setter");
87+
}
88+
89+
static void IndexedPropertyQuery(
90+
uint32_t index, const v8::PropertyCallbackInfo<v8::Integer>& info) {
91+
LogCallback(info, "indexed query");
92+
}
93+
94+
static void IndexedPropertyDeleter(
95+
uint32_t index, const v8::PropertyCallbackInfo<v8::Boolean>& info) {
96+
LogCallback(info, "indexed deleter");
97+
}
98+
99+
static void IndexedPropertyEnumerator(
100+
const v8::PropertyCallbackInfo<Array>& info) {
101+
LogCallback(info, "indexed enumerator");
102+
}
103+
104+
static void IndexedPropertyDefiner(
105+
uint32_t index, const v8::PropertyDescriptor& desc,
106+
const v8::PropertyCallbackInfo<Value>& info) {
107+
LogCallback(info, "indexed definer");
108+
}
109+
110+
static void IndexedPropertyDescriptor(
111+
uint32_t index, const v8::PropertyCallbackInfo<Value>& info) {
112+
LogCallback(info, "indexed descriptor");
113+
}
114+
115+
template <class T>
116+
static void LogCallback(const v8::PropertyCallbackInfo<T>& info,
117+
const char* callback_name) {
118+
InterceptorLoggingTest* test = reinterpret_cast<InterceptorLoggingTest*>(
119+
info.This()->GetAlignedPointerFromInternalField(kTestIndex));
120+
test->Log(callback_name);
121+
}
122+
123+
void Log(const char* callback_name) {
124+
if (log_is_empty_) {
125+
log_is_empty_ = false;
126+
} else {
127+
log_ << ", ";
128+
}
129+
log_ << callback_name;
130+
}
131+
132+
protected:
133+
void SetUp() override {
134+
// Set up the object that supports full interceptors.
135+
v8::Local<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(v8_isolate());
136+
templ->SetInternalFieldCount(1);
137+
templ->SetHandler(v8::NamedPropertyHandlerConfiguration(
138+
NamedPropertyGetter, NamedPropertySetter, NamedPropertyQuery,
139+
NamedPropertyDeleter, NamedPropertyEnumerator, NamedPropertyDefiner,
140+
NamedPropertyDescriptor));
141+
templ->SetHandler(v8::IndexedPropertyHandlerConfiguration(
142+
IndexedPropertyGetter, IndexedPropertySetter, IndexedPropertyQuery,
143+
IndexedPropertyDeleter, IndexedPropertyEnumerator,
144+
IndexedPropertyDefiner, IndexedPropertyDescriptor));
145+
v8::Local<v8::Object> instance =
146+
templ->NewInstance(context()).ToLocalChecked();
147+
instance->SetAlignedPointerInInternalField(kTestIndex, this);
148+
SetGlobalProperty("obj", instance);
149+
}
150+
151+
std::string Run(const char* script) {
152+
log_is_empty_ = true;
153+
log_.str(std::string());
154+
log_.clear();
155+
156+
RunJS(script);
157+
return log_.str();
158+
}
159+
160+
private:
161+
bool log_is_empty_ = false;
162+
std::stringstream log_;
163+
};
164+
165+
TEST_F(InterceptorLoggingTest, DispatchTest) {
166+
EXPECT_EQ(Run("for (var p in obj) {}"),
167+
"indexed enumerator, named enumerator");
168+
EXPECT_EQ(Run("Object.keys(obj)"), "indexed enumerator, named enumerator");
169+
170+
EXPECT_EQ(Run("obj.foo"), "named getter");
171+
EXPECT_EQ(Run("obj[42]"), "indexed getter");
172+
173+
EXPECT_EQ(Run("obj.foo = null"), "named setter");
174+
EXPECT_EQ(Run("obj[42] = null"), "indexed setter");
175+
176+
EXPECT_EQ(Run("Object.getOwnPropertyDescriptor(obj, 'foo')"),
177+
"named descriptor");
178+
179+
EXPECT_EQ(Run("Object.getOwnPropertyDescriptor(obj, 42)"),
180+
"indexed descriptor");
181+
182+
EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {value: 42})"),
183+
"named descriptor, named definer, named setter");
184+
EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {get(){} })"),
185+
"named descriptor, named definer");
186+
EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {set(value){}})"),
187+
"named descriptor, named definer");
188+
EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {get(){}, set(value){}})"),
189+
"named descriptor, named definer");
190+
191+
EXPECT_EQ(Run("Object.defineProperty(obj, 42, {value: 'foo'})"),
192+
"indexed descriptor, "
193+
// then attempt definer first and fallback to setter.
194+
"indexed definer, indexed setter");
195+
196+
EXPECT_EQ(Run("Object.prototype.propertyIsEnumerable.call(obj, 'a')"),
197+
"named query");
198+
EXPECT_EQ(Run("Object.prototype.propertyIsEnumerable.call(obj, 42)"),
199+
"indexed query");
200+
201+
EXPECT_EQ(Run("Object.prototype.hasOwnProperty.call(obj, 'a')"),
202+
"named query");
203+
// TODO(cbruni): Fix once hasOnwProperty is fixed (https://crbug.com/872628)
204+
EXPECT_EQ(Run("Object.prototype.hasOwnProperty.call(obj, '42')"), "");
205+
}
206+
} // namespace
207+
} // namespace internal
32208
} // namespace v8

0 commit comments

Comments
 (0)