From 615c34869c06b894346b06fd0ab7357e3ea7b507 Mon Sep 17 00:00:00 2001 From: "dcarney@chromium.org" Date: Wed, 4 Sep 2013 07:45:36 +0000 Subject: [PATCH] Push SetAccessor to Template R=mstarzinger@chromium.org BUG= Review URL: https://codereview.chromium.org/23182003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16515 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 61 ++++++++++++--- src/api.cc | 121 +++++++++++++++++------------- src/factory.cc | 61 ++++++++++++--- src/macros.py | 4 +- src/objects-debug.cc | 2 +- src/objects-inl.h | 3 +- src/objects-printer.cc | 2 + src/objects.cc | 97 ++++++++++++++++++------ src/objects.h | 16 ++-- test/cctest/test-accessors.cc | 21 +++++- test/cctest/test-declarative-accessors.cc | 6 +- tools/v8heapconst.py | 4 +- 12 files changed, 284 insertions(+), 114 deletions(-) diff --git a/include/v8.h b/include/v8.h index 1b676de..53e5929 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2123,10 +2123,10 @@ class V8_EXPORT Object : public Value { PropertyAttribute attribute = None); // This function is not yet stable and should not be used at this time. - bool SetAccessor(Handle name, - Handle descriptor, - AccessControl settings = DEFAULT, - PropertyAttribute attribute = None); + bool SetDeclaredAccessor(Local name, + Local descriptor, + PropertyAttribute attribute = None, + AccessControl settings = DEFAULT); /** * Returns an array containing the names of the enumerable properties @@ -2981,6 +2981,51 @@ class V8_EXPORT Template : public Data { PropertyAttribute attribute = None, AccessControl settings = DEFAULT); + /** + * Whenever the property with the given name is accessed on objects + * created from this Template the getter and setter callbacks + * are called instead of getting and setting the property directly + * on the JavaScript object. + * + * \param name The name of the property for which an accessor is added. + * \param getter The callback to invoke when getting the property. + * \param setter The callback to invoke when setting the property. + * \param data A piece of data that will be passed to the getter and setter + * callbacks whenever they are invoked. + * \param settings Access control settings for the accessor. This is a bit + * field consisting of one of more of + * DEFAULT = 0, ALL_CAN_READ = 1, or ALL_CAN_WRITE = 2. + * The default is to not allow cross-context access. + * ALL_CAN_READ means that all cross-context reads are allowed. + * ALL_CAN_WRITE means that all cross-context writes are allowed. + * The combination ALL_CAN_READ | ALL_CAN_WRITE can be used to allow all + * cross-context access. + * \param attribute The attributes of the property for which an accessor + * is added. + * \param signature The signature describes valid receivers for the accessor + * and is used to perform implicit instance checks against them. If the + * receiver is incompatible (i.e. is not an instance of the constructor as + * defined by FunctionTemplate::HasInstance()), an implicit TypeError is + * thrown and no callback is invoked. + */ + void SetNativeDataProperty(Local name, + AccessorGetterCallback getter, + AccessorSetterCallback setter = 0, + // TODO(dcarney): gcc can't handle Local below + Handle data = Handle(), + PropertyAttribute attribute = None, + Local signature = + Local(), + AccessControl settings = DEFAULT); + + // This function is not yet stable and should not be used at this time. + bool SetDeclaredAccessor(Local name, + Local descriptor, + PropertyAttribute attribute = None, + Local signature = + Local(), + AccessControl settings = DEFAULT); + private: Template(); @@ -3447,14 +3492,6 @@ class V8_EXPORT ObjectTemplate : public Template { Handle signature = Handle()); - // This function is not yet stable and should not be used at this time. - bool SetAccessor(Handle name, - Handle descriptor, - AccessControl settings = DEFAULT, - PropertyAttribute attribute = None, - Handle signature = - Handle()); - /** * Sets a named property handler on the object template. * diff --git a/src/api.cc b/src/api.cc index e2f3fcd..1a422fb 100644 --- a/src/api.cc +++ b/src/api.cc @@ -1438,55 +1438,95 @@ Local ObjectTemplate::New( // Ensure that the object template has a constructor. If no // constructor is available we create one. -static void EnsureConstructor(ObjectTemplate* object_template) { - if (Utils::OpenHandle(object_template)->constructor()->IsUndefined()) { - Local templ = FunctionTemplate::New(); - i::Handle constructor = Utils::OpenHandle(*templ); - constructor->set_instance_template(*Utils::OpenHandle(object_template)); - Utils::OpenHandle(object_template)->set_constructor(*constructor); +static i::Handle EnsureConstructor( + ObjectTemplate* object_template) { + i::Object* obj = Utils::OpenHandle(object_template)->constructor(); + if (!obj ->IsUndefined()) { + i::FunctionTemplateInfo* info = i::FunctionTemplateInfo::cast(obj); + return i::Handle(info, info->GetIsolate()); } + Local templ = FunctionTemplate::New(); + i::Handle constructor = Utils::OpenHandle(*templ); + constructor->set_instance_template(*Utils::OpenHandle(object_template)); + Utils::OpenHandle(object_template)->set_constructor(*constructor); + return constructor; } -static inline void AddPropertyToFunctionTemplate( - i::Handle cons, +static inline void AddPropertyToTemplate( + i::Handle info, i::Handle obj) { - i::Handle list(cons->property_accessors(), cons->GetIsolate()); + i::Handle list(info->property_accessors(), info->GetIsolate()); if (list->IsUndefined()) { list = NeanderArray().value(); - cons->set_property_accessors(*list); + info->set_property_accessors(*list); } NeanderArray array(list); array.add(obj); } -template -static bool ObjectTemplateSetAccessor( - ObjectTemplate* object_template, - v8::Handle name, +static inline i::Handle GetTemplateInfo( + Template* template_obj) { + return Utils::OpenHandle(template_obj); +} + + +// TODO(dcarney): remove this with ObjectTemplate::SetAccessor +static inline i::Handle GetTemplateInfo( + ObjectTemplate* object_template) { + EnsureConstructor(object_template); + return Utils::OpenHandle(object_template); +} + + +template +static bool TemplateSetAccessor( + Template* template_obj, + v8::Local name, Getter getter, Setter setter, Data data, AccessControl settings, PropertyAttribute attribute, - v8::Handle signature) { - i::Isolate* isolate = Utils::OpenHandle(object_template)->GetIsolate(); + v8::Local signature) { + i::Isolate* isolate = Utils::OpenHandle(template_obj)->GetIsolate(); if (IsDeadCheck(isolate, "v8::ObjectTemplate::SetAccessor()")) return false; ENTER_V8(isolate); i::HandleScope scope(isolate); - EnsureConstructor(object_template); - i::FunctionTemplateInfo* constructor = i::FunctionTemplateInfo::cast( - Utils::OpenHandle(object_template)->constructor()); - i::Handle cons(constructor); i::Handle obj = MakeAccessorInfo( name, getter, setter, data, settings, attribute, signature); if (obj.is_null()) return false; - AddPropertyToFunctionTemplate(cons, obj); + i::Handle info = GetTemplateInfo(template_obj); + AddPropertyToTemplate(info, obj); return true; } +bool Template::SetDeclaredAccessor( + Local name, + Local descriptor, + PropertyAttribute attribute, + Local signature, + AccessControl settings) { + void* null = NULL; + return TemplateSetAccessor( + this, name, descriptor, null, null, settings, attribute, signature); +} + + +void Template::SetNativeDataProperty(v8::Local name, + AccessorGetterCallback getter, + AccessorSetterCallback setter, + v8::Handle data, + PropertyAttribute attribute, + v8::Local signature, + AccessControl settings) { + TemplateSetAccessor( + this, name, getter, setter, data, settings, attribute, signature); +} + + void ObjectTemplate::SetAccessor(v8::Handle name, AccessorGetterCallback getter, AccessorSetterCallback setter, @@ -1494,22 +1534,11 @@ void ObjectTemplate::SetAccessor(v8::Handle name, AccessControl settings, PropertyAttribute attribute, v8::Handle signature) { - ObjectTemplateSetAccessor( + TemplateSetAccessor( this, name, getter, setter, data, settings, attribute, signature); } -bool ObjectTemplate::SetAccessor(Handle name, - Handle descriptor, - AccessControl settings, - PropertyAttribute attribute, - Handle signature) { - void* null = NULL; - return ObjectTemplateSetAccessor( - this, name, descriptor, null, null, settings, attribute, signature); -} - - void ObjectTemplate::SetNamedPropertyHandler( NamedPropertyGetterCallback getter, NamedPropertySetterCallback setter, @@ -3638,10 +3667,10 @@ bool Object::SetAccessor(Handle name, } -bool Object::SetAccessor(Handle name, - Handle descriptor, - AccessControl settings, - PropertyAttribute attributes) { +bool Object::SetDeclaredAccessor(Local name, + Local descriptor, + PropertyAttribute attributes, + AccessControl settings) { void* null = NULL; return ObjectSetAccessor( this, name, descriptor, null, null, settings, attributes); @@ -5314,18 +5343,6 @@ const char* v8::V8::GetVersion() { } -static i::Handle - EnsureConstructor(i::Handle templ) { - if (templ->constructor()->IsUndefined()) { - Local constructor = FunctionTemplate::New(); - Utils::OpenHandle(*constructor)->set_instance_template(*templ); - templ->set_constructor(*Utils::OpenHandle(*constructor)); - } - return i::Handle( - i::FunctionTemplateInfo::cast(templ->constructor())); -} - - static i::Handle CreateEnvironment( i::Isolate* isolate, v8::ExtensionConfiguration* extensions, @@ -5342,13 +5359,11 @@ static i::Handle CreateEnvironment( if (!global_template.IsEmpty()) { // Make sure that the global_template has a constructor. - global_constructor = - EnsureConstructor(Utils::OpenHandle(*global_template)); + global_constructor = EnsureConstructor(*global_template); // Create a fresh template for the global proxy object. proxy_template = ObjectTemplate::New(); - proxy_constructor = - EnsureConstructor(Utils::OpenHandle(*proxy_template)); + proxy_constructor = EnsureConstructor(*proxy_template); // Set the global template to be the prototype template of // global proxy template. diff --git a/src/factory.cc b/src/factory.cc index 7dde2a7..3d94921 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -1460,15 +1460,29 @@ Handle Factory::CreateApiFunction( result->shared()->set_construct_stub(*construct_stub); result->shared()->DontAdaptArguments(); - // Recursively copy parent templates' accessors, 'data' may be modified. + // Recursively copy parent instance templates' accessors, + // 'data' may be modified. int max_number_of_additional_properties = 0; + int max_number_of_static_properties = 0; FunctionTemplateInfo* info = *obj; while (true) { - Object* props = info->property_accessors(); - if (!props->IsUndefined()) { - Handle props_handle(props, isolate()); - NeanderArray props_array(props_handle); - max_number_of_additional_properties += props_array.length(); + if (!info->instance_template()->IsUndefined()) { + Object* props = + ObjectTemplateInfo::cast( + info->instance_template())->property_accessors(); + if (!props->IsUndefined()) { + Handle props_handle(props, isolate()); + NeanderArray props_array(props_handle); + max_number_of_additional_properties += props_array.length(); + } + } + if (!info->property_accessors()->IsUndefined()) { + Object* props = info->property_accessors(); + if (!props->IsUndefined()) { + Handle props_handle(props, isolate()); + NeanderArray props_array(props_handle); + max_number_of_static_properties += props_array.length(); + } } Object* parent = info->parent_template(); if (parent->IsUndefined()) break; @@ -1477,17 +1491,44 @@ Handle Factory::CreateApiFunction( Map::EnsureDescriptorSlack(map, max_number_of_additional_properties); + // Use a temporary FixedArray to acculumate static accessors + int valid_descriptors = 0; + Handle array; + if (max_number_of_static_properties > 0) { + array = NewFixedArray(max_number_of_static_properties); + } + while (true) { - Handle props = Handle(obj->property_accessors(), - isolate()); - if (!props->IsUndefined()) { - Map::AppendCallbackDescriptors(map, props); + // Install instance descriptors + if (!obj->instance_template()->IsUndefined()) { + Handle instance = + Handle( + ObjectTemplateInfo::cast(obj->instance_template()), isolate()); + Handle props = Handle(instance->property_accessors(), + isolate()); + if (!props->IsUndefined()) { + Map::AppendCallbackDescriptors(map, props); + } + } + // Accumulate static accessors + if (!obj->property_accessors()->IsUndefined()) { + Handle props = Handle(obj->property_accessors(), + isolate()); + valid_descriptors = + AccessorInfo::AppendUnique(props, array, valid_descriptors); } + // Climb parent chain Handle parent = Handle(obj->parent_template(), isolate()); if (parent->IsUndefined()) break; obj = Handle::cast(parent); } + // Install accumulated static accessors + for (int i = 0; i < valid_descriptors; i++) { + Handle accessor(AccessorInfo::cast(array->get(i))); + JSObject::SetAccessor(result, accessor); + } + ASSERT(result->shared()->IsApiFunction()); return result; } diff --git a/src/macros.py b/src/macros.py index f1a2130..38b9a08 100644 --- a/src/macros.py +++ b/src/macros.py @@ -42,8 +42,8 @@ const SETTER = 1; # These definitions must match the index of the properties in objects.h. const kApiTagOffset = 0; const kApiPropertyListOffset = 1; -const kApiSerialNumberOffset = 2; -const kApiConstructorOffset = 2; +const kApiSerialNumberOffset = 3; +const kApiConstructorOffset = 3; const kApiPrototypeTemplateOffset = 5; const kApiParentTemplateOffset = 6; const kApiFlagOffset = 14; diff --git a/src/objects-debug.cc b/src/objects-debug.cc index 8ab793e..3eb7817 100644 --- a/src/objects-debug.cc +++ b/src/objects-debug.cc @@ -889,6 +889,7 @@ void CallHandlerInfo::CallHandlerInfoVerify() { void TemplateInfo::TemplateInfoVerify() { VerifyPointer(tag()); VerifyPointer(property_list()); + VerifyPointer(property_accessors()); } @@ -897,7 +898,6 @@ void FunctionTemplateInfo::FunctionTemplateInfoVerify() { TemplateInfoVerify(); VerifyPointer(serial_number()); VerifyPointer(call_code()); - VerifyPointer(property_accessors()); VerifyPointer(prototype_template()); VerifyPointer(parent_template()); VerifyPointer(named_property_handler()); diff --git a/src/objects-inl.h b/src/objects-inl.h index 90bd2ec..8353695 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -4466,11 +4466,10 @@ ACCESSORS(CallHandlerInfo, data, Object, kDataOffset) ACCESSORS(TemplateInfo, tag, Object, kTagOffset) ACCESSORS(TemplateInfo, property_list, Object, kPropertyListOffset) +ACCESSORS(TemplateInfo, property_accessors, Object, kPropertyAccessorsOffset) ACCESSORS(FunctionTemplateInfo, serial_number, Object, kSerialNumberOffset) ACCESSORS(FunctionTemplateInfo, call_code, Object, kCallCodeOffset) -ACCESSORS(FunctionTemplateInfo, property_accessors, Object, - kPropertyAccessorsOffset) ACCESSORS(FunctionTemplateInfo, prototype_template, Object, kPrototypeTemplateOffset) ACCESSORS(FunctionTemplateInfo, parent_template, Object, kParentTemplateOffset) diff --git a/src/objects-printer.cc b/src/objects-printer.cc index 9ea060f..0b8fdfd 100644 --- a/src/objects-printer.cc +++ b/src/objects-printer.cc @@ -1070,6 +1070,8 @@ void ObjectTemplateInfo::ObjectTemplateInfoPrint(FILE* out) { tag()->ShortPrint(out); PrintF(out, "\n - property_list: "); property_list()->ShortPrint(out); + PrintF(out, "\n - property_accessors: "); + property_accessors()->ShortPrint(out); PrintF(out, "\n - constructor: "); constructor()->ShortPrint(out); PrintF(out, "\n - internal_field_count: "); diff --git a/src/objects.cc b/src/objects.cc index 7490927..bed3834 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3001,48 +3001,101 @@ void Map::EnsureDescriptorSlack(Handle map, int slack) { } -void Map::AppendCallbackDescriptors(Handle map, - Handle descriptors) { - Isolate* isolate = map->GetIsolate(); - Handle array(map->instance_descriptors()); - NeanderArray callbacks(descriptors); - int nof_callbacks = callbacks.length(); - - ASSERT(array->NumberOfSlackDescriptors() >= nof_callbacks); +template +static int AppendUniqueCallbacks(NeanderArray* callbacks, + Handle array, + int valid_descriptors) { + int nof_callbacks = callbacks->length(); + Isolate* isolate = array->GetIsolate(); // Ensure the keys are unique names before writing them into the // instance descriptor. Since it may cause a GC, it has to be done before we // temporarily put the heap in an invalid state while appending descriptors. for (int i = 0; i < nof_callbacks; ++i) { - Handle entry(AccessorInfo::cast(callbacks.get(i))); - if (!entry->name()->IsUniqueName()) { - Handle key = - isolate->factory()->InternalizedStringFromString( - Handle(String::cast(entry->name()))); - entry->set_name(*key); - } + Handle entry(AccessorInfo::cast(callbacks->get(i))); + if (entry->name()->IsUniqueName()) continue; + Handle key = + isolate->factory()->InternalizedStringFromString( + Handle(String::cast(entry->name()))); + entry->set_name(*key); } - int nof = map->NumberOfOwnDescriptors(); - // Fill in new callback descriptors. Process the callbacks from // back to front so that the last callback with a given name takes // precedence over previously added callbacks with that name. for (int i = nof_callbacks - 1; i >= 0; i--) { - AccessorInfo* entry = AccessorInfo::cast(callbacks.get(i)); + AccessorInfo* entry = AccessorInfo::cast(callbacks->get(i)); Name* key = Name::cast(entry->name()); // Check if a descriptor with this name already exists before writing. - if (array->Search(key, nof) == DescriptorArray::kNotFound) { - CallbacksDescriptor desc(key, entry, entry->property_attributes()); - array->Append(&desc); - nof += 1; + if (!T::Contains(key, entry, valid_descriptors, array)) { + T::Insert(key, entry, valid_descriptors, array); + valid_descriptors++; + } + } + + return valid_descriptors; +} + +struct DescriptorArrayAppender { + typedef DescriptorArray Array; + static bool Contains(Name* key, + AccessorInfo* entry, + int valid_descriptors, + Handle array) { + return array->Search(key, valid_descriptors) != DescriptorArray::kNotFound; + } + static void Insert(Name* key, + AccessorInfo* entry, + int valid_descriptors, + Handle array) { + CallbacksDescriptor desc(key, entry, entry->property_attributes()); + array->Append(&desc); + } +}; + + +struct FixedArrayAppender { + typedef FixedArray Array; + static bool Contains(Name* key, + AccessorInfo* entry, + int valid_descriptors, + Handle array) { + for (int i = 0; i < valid_descriptors; i++) { + if (key == AccessorInfo::cast(array->get(i))->name()) return true; } + return false; } + static void Insert(Name* key, + AccessorInfo* entry, + int valid_descriptors, + Handle array) { + array->set(valid_descriptors, entry); + } +}; + +void Map::AppendCallbackDescriptors(Handle map, + Handle descriptors) { + int nof = map->NumberOfOwnDescriptors(); + Handle array(map->instance_descriptors()); + NeanderArray callbacks(descriptors); + ASSERT(array->NumberOfSlackDescriptors() >= callbacks.length()); + nof = AppendUniqueCallbacks(&callbacks, array, nof); map->SetNumberOfOwnDescriptors(nof); } +int AccessorInfo::AppendUnique(Handle descriptors, + Handle array, + int valid_descriptors) { + NeanderArray callbacks(descriptors); + ASSERT(array->length() >= callbacks.length() + valid_descriptors); + return AppendUniqueCallbacks(&callbacks, + array, + valid_descriptors); +} + + static bool ContainsMap(MapHandleList* maps, Handle map) { ASSERT(!map.is_null()); for (int i = 0; i < maps->length(); ++i) { diff --git a/src/objects.h b/src/objects.h index 4e0f53e..d7339d6 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9489,6 +9489,11 @@ class AccessorInfo: public Struct { // Dispatched behavior. DECLARE_VERIFIER(AccessorInfo) + // Append all descriptors to the array that are not already there. + // Return number added. + static int AppendUnique(Handle descriptors, + Handle array, + int valid_descriptors); static const int kNameOffset = HeapObject::kHeaderSize; static const int kFlagOffset = kNameOffset + kPointerSize; @@ -9795,12 +9800,15 @@ class TemplateInfo: public Struct { public: DECL_ACCESSORS(tag, Object) DECL_ACCESSORS(property_list, Object) + DECL_ACCESSORS(property_accessors, Object) DECLARE_VERIFIER(TemplateInfo) - static const int kTagOffset = HeapObject::kHeaderSize; + static const int kTagOffset = HeapObject::kHeaderSize; static const int kPropertyListOffset = kTagOffset + kPointerSize; - static const int kHeaderSize = kPropertyListOffset + kPointerSize; + static const int kPropertyAccessorsOffset = + kPropertyListOffset + kPointerSize; + static const int kHeaderSize = kPropertyAccessorsOffset + kPointerSize; private: DISALLOW_IMPLICIT_CONSTRUCTORS(TemplateInfo); @@ -9811,7 +9819,6 @@ class FunctionTemplateInfo: public TemplateInfo { public: DECL_ACCESSORS(serial_number, Object) DECL_ACCESSORS(call_code, Object) - DECL_ACCESSORS(property_accessors, Object) DECL_ACCESSORS(prototype_template, Object) DECL_ACCESSORS(parent_template, Object) DECL_ACCESSORS(named_property_handler, Object) @@ -9843,9 +9850,8 @@ class FunctionTemplateInfo: public TemplateInfo { static const int kSerialNumberOffset = TemplateInfo::kHeaderSize; static const int kCallCodeOffset = kSerialNumberOffset + kPointerSize; - static const int kPropertyAccessorsOffset = kCallCodeOffset + kPointerSize; static const int kPrototypeTemplateOffset = - kPropertyAccessorsOffset + kPointerSize; + kCallCodeOffset + kPointerSize; static const int kParentTemplateOffset = kPrototypeTemplateOffset + kPointerSize; static const int kNamedPropertyHandlerOffset = diff --git a/test/cctest/test-accessors.cc b/test/cctest/test-accessors.cc index 2f7e7f4..b68b0b0 100644 --- a/test/cctest/test-accessors.cc +++ b/test/cctest/test-accessors.cc @@ -49,6 +49,12 @@ static void handle_property(Local name, info.GetReturnValue().Set(v8_num(900)); } +static void handle_property_2(Local name, + const v8::PropertyCallbackInfo& info) { + ApiTestFuzzer::Fuzz(); + info.GetReturnValue().Set(v8_num(902)); +} + static void handle_property(const v8::FunctionCallbackInfo& info) { ApiTestFuzzer::Fuzz(); @@ -67,16 +73,27 @@ THREADED_TEST(PropertyHandler) { getter_templ->SetLength(0); fun_templ-> InstanceTemplate()->SetAccessorProperty(v8_str("bar"), getter_templ); + fun_templ->InstanceTemplate()-> + SetNativeDataProperty(v8_str("instance_foo"), handle_property); + fun_templ->SetNativeDataProperty(v8_str("object_foo"), handle_property_2); Local fun = fun_templ->GetFunction(); env->Global()->Set(v8_str("Fun"), fun); - Local