Reviewers: Michael Starzinger, rossberg, jochen,
Message:
Please take a look when you next have a spare moment.
The overlap between hidden prototypes and inherited function templates is
unfortunate. Addressing it here makes some sense to me, but there might be
well
be more appropriate ways to address this one (and elsewhere.) Please let me
know
if a trip back to the drawing board is warranted.
Description:
Avoid duplication of a hidden & inherited prototype's properties.
In Runtime_GetLocalPropertyNames(), required steps are taken to also
consider the hidden prototypes of an object when computing the
property name set. Should that hidden prototype also have been a
template for the object, do not consider.
If not, properties that have been copied down and inherited will be
included twice, returning duplicate property names.
LOG=N
R=
BUG=269562
Please review this at https://codereview.chromium.org/116533003/
SVN Base: git://github.com/v8/v8.git@bleeding_edge
Affected files (+62, -1 lines):
M src/runtime.cc
M test/cctest/test-api.cc
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
8d84fdace2562f4f6f834940706f298bfdf481a5..89d49fe78cdf73c6da7d600c5ebb66ea37d313a4
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5726,6 +5726,22 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_GetPropertyNamesFast) {
}
+// Returns true if inherited accessors have been copied down
+// to obj from proto's function template.
+static bool IsInheritedApiFunctionOf(JSObject *obj, Object *proto) {
+ Object *constructor = HeapObject::cast(proto)->map()->constructor();
+ if (!constructor->IsJSFunction()) {
+ return false;
+ }
+ JSFunction *constructor_fun = JSFunction::cast(constructor);
+ if (!constructor_fun->shared()->IsApiFunction()) {
+ return false;
+ }
+ return constructor_fun->shared()->
+ get_api_func_data()->IsTemplateFor(obj);
+}
+
+
// Find the length of the prototype chain that is to to handled as one. If
a
// prototype object is hidden it is to be viewed as part of the the object
it
// is prototype for.
@@ -5733,7 +5749,8 @@ static int LocalPrototypeChainLength(JSObject* obj) {
int count = 1;
Object* proto = obj->GetPrototype();
while (proto->IsJSObject() &&
- JSObject::cast(proto)->map()->is_hidden_prototype()) {
+ JSObject::cast(proto)->map()->is_hidden_prototype() &&
+ !IsInheritedApiFunctionOf(obj, proto)) {
count++;
proto = JSObject::cast(proto)->GetPrototype();
}
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
91f37367dae9d0ec815cbd4738bc62484e1df80f..c33303e3cbdb8965ce7f74ef46502bf1ca86d306
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -9999,6 +9999,50 @@ THREADED_TEST(Regress91517) {
}
+// Getting property names of an object with a hidden and inherited
+// prototype should not duplicate the accessor properties inherited.
+THREADED_TEST(Regress269562) {
+ i::FLAG_allow_natives_syntax = true;
+ LocalContext context;
+ v8::HandleScope handle_scope(context->GetIsolate());
+
+ Local<v8::FunctionTemplate> t1 = v8::FunctionTemplate::New();
+ t1->SetHiddenPrototype(true);
+
+ Local<v8::ObjectTemplate> i1 = t1->InstanceTemplate();
+ i1->SetAccessor(v8_str("foo"),
+ SimpleAccessorGetter, SimpleAccessorSetter);
+ i1->SetAccessor(v8_str("bar"),
+ SimpleAccessorGetter, SimpleAccessorSetter);
+ i1->SetAccessor(v8_str("baz"),
+ SimpleAccessorGetter, SimpleAccessorSetter);
+ i1->Set(v8_str("n1"), v8_num(1));
+ i1->Set(v8_str("n2"), v8_num(2));
+
+ Local<v8::Object> o1 = t1->GetFunction()->NewInstance();
+ Local<v8::FunctionTemplate> t2 = v8::FunctionTemplate::New();
+ t2->SetHiddenPrototype(true);
+
+ // Inherit from t1 and mark prototype as hidden.
+ t2->Inherit(t1);
+ t2->InstanceTemplate()->Set(v8_str("mine"), v8_num(4));
+
+ Local<v8::Object> o2 = t2->GetFunction()->NewInstance();
+ CHECK(o2->SetPrototype(o1));
+
+ // Call the runtime version of GetLocalPropertyNames() on
+ // the natively created object through JavaScript.
+ context->Global()->Set(v8_str("obj"), o2);
+ CompileRun("var names = %GetLocalPropertyNames(obj, true);");
+
+ ExpectInt32("names.length", 4);
+ ExpectTrue("names.indexOf(\"foo\") >= 0");
+ ExpectTrue("names.indexOf(\"bar\") >= 0");
+ ExpectTrue("names.indexOf(\"baz\") >= 0");
+ ExpectTrue("names.indexOf(\"mine\") >= 0");
+}
+
+
THREADED_TEST(FunctionReadOnlyPrototype) {
LocalContext context;
v8::HandleScope handle_scope(context->GetIsolate());
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
