From e8af938c524d82a1dd3ee8a5d7905528b029ebc7 Mon Sep 17 00:00:00 2001 From: Camillo Bruni Date: Sat, 18 Aug 2018 13:41:51 -0400 Subject: [PATCH] deps: cherry-pick e1a7699 from upstream V8 Original commit message: [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 Reviewed-by: Adam Klein Commit-Queue: Camillo Bruni Cr-Commit-Position: refs/heads/master@{#55142} PR-URL: https://github.com/nodejs/node/pull/22390 Fixes: https://github.com/nodejs/node/issues/17480 Fixes: https://github.com/nodejs/node/issues/17481 Refs: https://github.com/v8/v8/commit/e1a76995ef311eb3ca66e12ef1941ed596034d59 Reviewed-By: Anna Henningsen Reviewed-By: Gus Caplan Reviewed-By: James M Snell --- common.gypi | 2 +- deps/v8/include/v8.h | 39 ++++ deps/v8/src/api.cc | 4 - deps/v8/src/objects.cc | 63 ++++--- .../unittests/api/interceptor-unittest.cc | 176 ++++++++++++++++++ 5 files changed, 248 insertions(+), 36 deletions(-) diff --git a/common.gypi b/common.gypi index db9401ffa1a77a..326563728f0d31 100644 --- a/common.gypi +++ b/common.gypi @@ -29,7 +29,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.16', + 'v8_embedder_string': '-node.17', # Enable disassembler for `--print-code` v8 options 'v8_enable_disassembler': 1, diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index 22a2508fe67f89..ac00c8f9aa5c06 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -5782,6 +5782,26 @@ enum class PropertyHandlerFlags { }; struct NamedPropertyHandlerConfiguration { + NamedPropertyHandlerConfiguration( + GenericNamedPropertyGetterCallback getter, + GenericNamedPropertySetterCallback setter, + GenericNamedPropertyQueryCallback query, + GenericNamedPropertyDeleterCallback deleter, + GenericNamedPropertyEnumeratorCallback enumerator, + GenericNamedPropertyDefinerCallback definer, + GenericNamedPropertyDescriptorCallback descriptor, + Local data = Local(), + PropertyHandlerFlags flags = PropertyHandlerFlags::kNone) + : getter(getter), + setter(setter), + query(query), + deleter(deleter), + enumerator(enumerator), + definer(definer), + descriptor(descriptor), + data(data), + flags(flags) {} + NamedPropertyHandlerConfiguration( /** Note: getter is required */ GenericNamedPropertyGetterCallback getter = 0, @@ -5833,6 +5853,25 @@ struct NamedPropertyHandlerConfiguration { struct IndexedPropertyHandlerConfiguration { + IndexedPropertyHandlerConfiguration( + IndexedPropertyGetterCallback getter, + IndexedPropertySetterCallback setter, IndexedPropertyQueryCallback query, + IndexedPropertyDeleterCallback deleter, + IndexedPropertyEnumeratorCallback enumerator, + IndexedPropertyDefinerCallback definer, + IndexedPropertyDescriptorCallback descriptor, + Local data = Local(), + PropertyHandlerFlags flags = PropertyHandlerFlags::kNone) + : getter(getter), + setter(setter), + query(query), + deleter(deleter), + enumerator(enumerator), + definer(definer), + descriptor(descriptor), + data(data), + flags(flags) {} + IndexedPropertyHandlerConfiguration( /** Note: getter is required */ IndexedPropertyGetterCallback getter = 0, diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc index e1a01c2544e9d0..7412f448b1a3c9 100644 --- a/deps/v8/src/api.cc +++ b/deps/v8/src/api.cc @@ -1803,10 +1803,6 @@ static i::Handle CreateInterceptorInfo( i::Isolate* isolate, Getter getter, Setter setter, Query query, Descriptor descriptor, Deleter remover, Enumerator enumerator, Definer definer, Local data, PropertyHandlerFlags flags) { - // Either intercept attributes or descriptor. - DCHECK(query == nullptr || descriptor == nullptr); - // Only use descriptor callback with definer callback. - DCHECK(query == nullptr || definer == nullptr); auto obj = i::Handle::cast( isolate->factory()->NewStruct(i::INTERCEPTOR_INFO_TYPE, i::TENURED)); obj->set_flags(0); diff --git a/deps/v8/src/objects.cc b/deps/v8/src/objects.cc index 5c43a911a9f12a..6913e68ed9a5f9 100644 --- a/deps/v8/src/objects.cc +++ b/deps/v8/src/objects.cc @@ -7672,41 +7672,42 @@ Maybe GetPropertyDescriptorWithInterceptor(LookupIterator* it, } } - if (it->state() == LookupIterator::INTERCEPTOR) { - Isolate* isolate = it->isolate(); - Handle interceptor = it->GetInterceptor(); - if (!interceptor->descriptor()->IsUndefined(isolate)) { - Handle result; - Handle holder = it->GetHolder(); + if (it->state() != LookupIterator::INTERCEPTOR) return Just(false); - Handle receiver = it->GetReceiver(); - if (!receiver->IsJSReceiver()) { - ASSIGN_RETURN_ON_EXCEPTION_VALUE( - isolate, receiver, Object::ConvertReceiver(isolate, receiver), - Nothing()); - } + Isolate* isolate = it->isolate(); + Handle interceptor = it->GetInterceptor(); + if (interceptor->descriptor()->IsUndefined(isolate)) return Just(false); - PropertyCallbackArguments args(isolate, interceptor->data(), *receiver, - *holder, kDontThrow); - if (it->IsElement()) { - result = args.CallIndexedDescriptor(interceptor, it->index()); - } else { - result = args.CallNamedDescriptor(interceptor, it->name()); - } - if (!result.is_null()) { - // Request successfully intercepted, try to set the property - // descriptor. - Utils::ApiCheck( - PropertyDescriptor::ToPropertyDescriptor(isolate, result, desc), - it->IsElement() ? "v8::IndexedPropertyDescriptorCallback" - : "v8::NamedPropertyDescriptorCallback", - "Invalid property descriptor."); + Handle result; + Handle holder = it->GetHolder(); - return Just(true); - } - it->Next(); - } + Handle receiver = it->GetReceiver(); + if (!receiver->IsJSReceiver()) { + ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, receiver, + Object::ConvertReceiver(isolate, receiver), + Nothing()); + } + + PropertyCallbackArguments args(isolate, interceptor->data(), *receiver, + *holder, kDontThrow); + if (it->IsElement()) { + result = args.CallIndexedDescriptor(interceptor, it->index()); + } else { + result = args.CallNamedDescriptor(interceptor, it->name()); } + if (!result.is_null()) { + // Request successfully intercepted, try to set the property + // descriptor. + Utils::ApiCheck( + PropertyDescriptor::ToPropertyDescriptor(isolate, result, desc), + it->IsElement() ? "v8::IndexedPropertyDescriptorCallback" + : "v8::NamedPropertyDescriptorCallback", + "Invalid property descriptor."); + + return Just(true); + } + + it->Next(); return Just(false); } } // namespace diff --git a/deps/v8/test/unittests/api/interceptor-unittest.cc b/deps/v8/test/unittests/api/interceptor-unittest.cc index 2f9f0e459e6fed..b13384f18adfef 100644 --- a/deps/v8/test/unittests/api/interceptor-unittest.cc +++ b/deps/v8/test/unittests/api/interceptor-unittest.cc @@ -29,4 +29,180 @@ TEST_F(InterceptorTest, FreezeApiObjectWithInterceptor) { } } // namespace + +namespace internal { +namespace { + +class InterceptorLoggingTest : public TestWithNativeContext { + public: + InterceptorLoggingTest() {} + + static const int kTestIndex = 0; + + static void NamedPropertyGetter(Local name, + const v8::PropertyCallbackInfo& info) { + LogCallback(info, "named getter"); + } + + static void NamedPropertySetter(Local name, Local value, + const v8::PropertyCallbackInfo& info) { + LogCallback(info, "named setter"); + } + + static void NamedPropertyQuery( + Local name, const v8::PropertyCallbackInfo& info) { + LogCallback(info, "named query"); + } + + static void NamedPropertyDeleter( + Local name, const v8::PropertyCallbackInfo& info) { + LogCallback(info, "named deleter"); + } + + static void NamedPropertyEnumerator( + const v8::PropertyCallbackInfo& info) { + LogCallback(info, "named enumerator"); + } + + static void NamedPropertyDefiner( + Local name, const v8::PropertyDescriptor& desc, + const v8::PropertyCallbackInfo& info) { + LogCallback(info, "named definer"); + } + + static void NamedPropertyDescriptor( + Local name, const v8::PropertyCallbackInfo& info) { + LogCallback(info, "named descriptor"); + } + + static void IndexedPropertyGetter( + uint32_t index, const v8::PropertyCallbackInfo& info) { + LogCallback(info, "indexed getter"); + } + + static void IndexedPropertySetter( + uint32_t index, Local value, + const v8::PropertyCallbackInfo& info) { + LogCallback(info, "indexed setter"); + } + + static void IndexedPropertyQuery( + uint32_t index, const v8::PropertyCallbackInfo& info) { + LogCallback(info, "indexed query"); + } + + static void IndexedPropertyDeleter( + uint32_t index, const v8::PropertyCallbackInfo& info) { + LogCallback(info, "indexed deleter"); + } + + static void IndexedPropertyEnumerator( + const v8::PropertyCallbackInfo& info) { + LogCallback(info, "indexed enumerator"); + } + + static void IndexedPropertyDefiner( + uint32_t index, const v8::PropertyDescriptor& desc, + const v8::PropertyCallbackInfo& info) { + LogCallback(info, "indexed definer"); + } + + static void IndexedPropertyDescriptor( + uint32_t index, const v8::PropertyCallbackInfo& info) { + LogCallback(info, "indexed descriptor"); + } + + template + static void LogCallback(const v8::PropertyCallbackInfo& info, + const char* callback_name) { + InterceptorLoggingTest* test = reinterpret_cast( + info.This()->GetAlignedPointerFromInternalField(kTestIndex)); + test->Log(callback_name); + } + + void Log(const char* callback_name) { + if (log_is_empty_) { + log_is_empty_ = false; + } else { + log_ << ", "; + } + log_ << callback_name; + } + + protected: + void SetUp() override { + // Set up the object that supports full interceptors. + v8::Local templ = v8::ObjectTemplate::New(v8_isolate()); + templ->SetInternalFieldCount(1); + templ->SetHandler(v8::NamedPropertyHandlerConfiguration( + NamedPropertyGetter, NamedPropertySetter, NamedPropertyQuery, + NamedPropertyDeleter, NamedPropertyEnumerator, NamedPropertyDefiner, + NamedPropertyDescriptor)); + templ->SetHandler(v8::IndexedPropertyHandlerConfiguration( + IndexedPropertyGetter, IndexedPropertySetter, IndexedPropertyQuery, + IndexedPropertyDeleter, IndexedPropertyEnumerator, + IndexedPropertyDefiner, IndexedPropertyDescriptor)); + v8::Local instance = + templ->NewInstance(context()).ToLocalChecked(); + instance->SetAlignedPointerInInternalField(kTestIndex, this); + SetGlobalProperty("obj", instance); + } + + std::string Run(const char* script) { + log_is_empty_ = true; + log_.str(std::string()); + log_.clear(); + + RunJS(script); + return log_.str(); + } + + private: + bool log_is_empty_ = false; + std::stringstream log_; +}; + +TEST_F(InterceptorLoggingTest, DispatchTest) { + EXPECT_EQ(Run("for (var p in obj) {}"), + "indexed enumerator, named enumerator"); + EXPECT_EQ(Run("Object.keys(obj)"), "indexed enumerator, named enumerator"); + + EXPECT_EQ(Run("obj.foo"), "named getter"); + EXPECT_EQ(Run("obj[42]"), "indexed getter"); + + EXPECT_EQ(Run("obj.foo = null"), "named setter"); + EXPECT_EQ(Run("obj[42] = null"), "indexed setter"); + + EXPECT_EQ(Run("Object.getOwnPropertyDescriptor(obj, 'foo')"), + "named descriptor"); + + EXPECT_EQ(Run("Object.getOwnPropertyDescriptor(obj, 42)"), + "indexed descriptor"); + + EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {value: 42})"), + "named descriptor, named definer, named setter"); + EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {get(){} })"), + "named descriptor, named definer"); + EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {set(value){}})"), + "named descriptor, named definer"); + EXPECT_EQ(Run("Object.defineProperty(obj, 'foo', {get(){}, set(value){}})"), + "named descriptor, named definer"); + + EXPECT_EQ(Run("Object.defineProperty(obj, 42, {value: 'foo'})"), + "indexed descriptor, " + // then attempt definer first and fallback to setter. + "indexed definer, indexed setter"); + + EXPECT_EQ(Run("Object.prototype.propertyIsEnumerable.call(obj, 'a')"), + "named query"); + EXPECT_EQ(Run("Object.prototype.propertyIsEnumerable.call(obj, 42)"), + "indexed query"); + + EXPECT_EQ(Run("Object.prototype.hasOwnProperty.call(obj, 'a')"), + "named query"); + // TODO(cbruni): Fix once hasOnwProperty is fixed (https://crbug.com/872628) + EXPECT_EQ(Run("Object.prototype.hasOwnProperty.call(obj, '42')"), ""); +} +} // namespace +} // namespace internal } // namespace v8