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
R=dcarney@chromium.org, jochen@chromium.org, rossberg@chromium.org
BUG=269562

Review URL: https://codereview.chromium.org/116533003

Patch from Sigbjorn Finne <sof@opera.com>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18448 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/runtime.cc b/src/runtime.cc
index a175836..713d450 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5801,32 +5801,55 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetLocalPropertyNames) {
 
   // 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);

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 7e699af..916a1d7 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -10063,6 +10063,60 @@ 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(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) {
   LocalContext context;
   v8::Isolate* isolate = context->GetIsolate();