Revision: 18448
Author: [email protected]
Date: Fri Jan 3 11:19:13 2014 UTC
Log: Avoid duplication of a hidden & inherited prototype's properties.
In Runtime_GetLocalPropertyNames(), the hidden prototypes of an object
are also consulted when deriving the property name set. However, if
given a function object and its template was inherited from the
template of one of its hidden prototypes, that hidden prototype's
property accessors will be present on the object already. Unwanted
duplicates will therefore appear.
Hence, go through the property names that the hidden prototypes
contribute and remove any already occurring ones.
Assumed to be a rare constellation, so the cost of this extra pass is
considered acceptable.
LOG=N
[email protected], [email protected], [email protected]
BUG=269562
Review URL: https://codereview.chromium.org/116533003
Patch from Sigbjorn Finne <[email protected]>.
http://code.google.com/p/v8/source/detail?r=18448
Modified:
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/runtime.cc Mon Dec 23 14:30:35 2013 UTC
+++ /branches/bleeding_edge/src/runtime.cc Fri Jan 3 11:19:13 2014 UTC
@@ -5801,32 +5801,55 @@
// Get the property names.
jsproto = obj;
- int proto_with_hidden_properties = 0;
int next_copy_index = 0;
+ int hidden_strings = 0;
for (int i = 0; i < length; i++) {
jsproto->GetLocalPropertyNames(*names, next_copy_index, filter);
+ if (i > 0) {
+ // Names from hidden prototypes may already have been added
+ // for inherited function template instances. Count the duplicates
+ // and stub them out; the final copy pass at the end ignores holes.
+ for (int j = next_copy_index;
+ j < next_copy_index + local_property_count[i];
+ j++) {
+ Object* name_from_hidden_proto = names->get(j);
+ for (int k = 0; k < next_copy_index; k++) {
+ if (names->get(k) != isolate->heap()->hidden_string()) {
+ Object* name = names->get(k);
+ if (name_from_hidden_proto == name) {
+ names->set(j, isolate->heap()->hidden_string());
+ hidden_strings++;
+ break;
+ }
+ }
+ }
+ }
+ }
next_copy_index += local_property_count[i];
if (jsproto->HasHiddenProperties()) {
- proto_with_hidden_properties++;
+ hidden_strings++;
}
if (i < length - 1) {
jsproto = Handle<JSObject>(JSObject::cast(jsproto->GetPrototype()));
}
}
- // Filter out name of hidden properties object.
- if (proto_with_hidden_properties > 0) {
+ // Filter out name of hidden properties object and
+ // hidden prototype duplicates.
+ if (hidden_strings > 0) {
Handle<FixedArray> old_names = names;
names = isolate->factory()->NewFixedArray(
- names->length() - proto_with_hidden_properties);
+ names->length() - hidden_strings);
int dest_pos = 0;
for (int i = 0; i < total_property_count; i++) {
Object* name = old_names->get(i);
if (name == isolate->heap()->hidden_string()) {
+ hidden_strings--;
continue;
}
names->set(dest_pos++, name);
}
+ ASSERT_EQ(0, hidden_strings);
}
return *isolate->factory()->NewJSArrayWithElements(names);
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Mon Dec 23 12:37:56
2013 UTC
+++ /branches/bleeding_edge/test/cctest/test-api.cc Fri Jan 3 11:19:13
2014 UTC
@@ -10061,6 +10061,60 @@
ExpectTrue("names.indexOf(\"fuz2\") >= 0");
ExpectFalse("names[1005] == undefined");
}
+
+
+// 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(context->GetIsolate());
+ 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(context->GetIsolate());
+ 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));
+
+ v8::Local<v8::Symbol> sym = v8::Symbol::New(context->GetIsolate(), "s1");
+ o1->Set(sym, v8_num(3));
+ o1->SetHiddenValue(v8_str("h1"), v8::Integer::New(2013));
+
+ // Call the runtime version of GetLocalPropertyNames() on
+ // the natively created object through JavaScript.
+ context->Global()->Set(v8_str("obj"), o2);
+ context->Global()->Set(v8_str("sym"), sym);
+ CompileRun("var names = %GetLocalPropertyNames(obj, true);");
+
+ ExpectInt32("names.length", 7);
+ ExpectTrue("names.indexOf(\"foo\") >= 0");
+ ExpectTrue("names.indexOf(\"bar\") >= 0");
+ ExpectTrue("names.indexOf(\"baz\") >= 0");
+ ExpectTrue("names.indexOf(\"n1\") >= 0");
+ ExpectTrue("names.indexOf(\"n2\") >= 0");
+ ExpectTrue("names.indexOf(sym) >= 0");
+ ExpectTrue("names.indexOf(\"mine\") >= 0");
+}
THREADED_TEST(FunctionReadOnlyPrototype) {
--
--
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.
