Speed up non-interceptor case of Object.getOwnPropertyNames

When there are interceptors on an object, it's possible to
end up with duplicate property names. But when all the names
are provided by v8, a collision is not possible, so we can
fast-path that case by not de-duping.

Also added better test coverage for interceptor API.

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

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

diff --git a/src/v8natives.js b/src/v8natives.js
index 3978e88..8473dd1 100644 (file)
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -1018,9 +1018,13 @@ function ObjectGetOwnPropertyNames(obj) {
 
   // Get the local element names.
   var propertyNames = %GetLocalElementNames(obj);
+  for (var i = 0; i < propertyNames.length; ++i) {
+    propertyNames[i] = %_NumberToString(propertyNames[i]);
+  }
 
   // Get names for indexed interceptor properties.
-  if (%GetInterceptorInfo(obj) & 1) {
+  var interceptorInfo = %GetInterceptorInfo(obj);
+  if ((interceptorInfo & 1) != 0) {
     var indexedInterceptorNames =
         %GetIndexedInterceptorElementNames(obj);
     if (indexedInterceptorNames) {
@@ -1034,8 +1038,7 @@ function ObjectGetOwnPropertyNames(obj) {
   propertyNames = propertyNames.concat(%GetLocalPropertyNames(obj));
 
   // Get names for named interceptor properties if any.
-
-  if (%GetInterceptorInfo(obj) & 2) {
+  if ((interceptorInfo & 2) != 0) {
     var namedInterceptorNames =
         %GetNamedInterceptorPropertyNames(obj);
     if (namedInterceptorNames) {
@@ -1043,21 +1046,24 @@ function ObjectGetOwnPropertyNames(obj) {
     }
   }
 
-  // Property names are expected to be unique strings.
-  var propertySet = { __proto__: null };
-  var j = 0;
-  for (var i = 0; i < propertyNames.length; ++i) {
-    var name = ToString(propertyNames[i]);
-    // We need to check for the exact property value since for intrinsic
-    // properties like toString if(propertySet["toString"]) will always
-    // succeed.
-    if (propertySet[name] === true) {
-      continue;
+  // Property names are expected to be unique strings,
+  // but interceptors can interfere with that assumption.
+  if (interceptorInfo != 0) {
+    var propertySet = { __proto__: null };
+    var j = 0;
+    for (var i = 0; i < propertyNames.length; ++i) {
+      var name = ToString(propertyNames[i]);
+      // We need to check for the exact property value since for intrinsic
+      // properties like toString if(propertySet["toString"]) will always
+      // succeed.
+      if (propertySet[name] === true) {
+        continue;
+      }
+      propertySet[name] = true;
+      propertyNames[j++] = name;
     }
-    propertySet[name] = true;
-    propertyNames[j++] = name;
+    propertyNames.length = j;
   }
-  propertyNames.length = j;
 
   return propertyNames;
 }

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index a6e9343..976c561 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -7554,9 +7554,18 @@ THREADED_TEST(AccessControlGetOwnPropertyNames) {
 }
 
 
+static v8::Handle<v8::Array> IndexedPropertyEnumerator(const AccessorInfo&) {
+  v8::Handle<v8::Array> result = v8::Array::New(2);
+  result->Set(0, v8::Integer::New(7));
+  result->Set(1, v8::Object::New());
+  return result;
+}
+
+
 static v8::Handle<v8::Array> NamedPropertyEnumerator(const AccessorInfo& info) {
-  v8::Handle<v8::Array> result = v8::Array::New(1);
+  v8::Handle<v8::Array> result = v8::Array::New(2);
   result->Set(0, v8_str("x"));
+  result->Set(1, v8::Object::New());
   return result;
 }
 
@@ -7565,7 +7574,10 @@ THREADED_TEST(GetOwnPropertyNamesWithInterceptor) {
   v8::HandleScope handle_scope;
   v8::Handle<v8::ObjectTemplate> obj_template = v8::ObjectTemplate::New();
 
+  obj_template->Set(v8_str("7"), v8::Integer::New(7));
   obj_template->Set(v8_str("x"), v8::Integer::New(42));
+  obj_template->SetIndexedPropertyHandler(NULL, NULL, NULL, NULL,
+                                          IndexedPropertyEnumerator);
   obj_template->SetNamedPropertyHandler(NULL, NULL, NULL, NULL,
                                         NamedPropertyEnumerator);
 
@@ -7573,9 +7585,17 @@ THREADED_TEST(GetOwnPropertyNamesWithInterceptor) {
   v8::Handle<v8::Object> global = context->Global();
   global->Set(v8_str("object"), obj_template->NewInstance());
 
-  v8::Handle<Value> value =
-      CompileRun("Object.getOwnPropertyNames(object).join(',')");
-  CHECK_EQ(v8_str("x"), value);
+  v8::Handle<v8::Value> result =
+      CompileRun("Object.getOwnPropertyNames(object)");
+  CHECK(result->IsArray());
+  v8::Handle<v8::Array> result_array = v8::Handle<v8::Array>::Cast(result);
+  CHECK_EQ(3, result_array->Length());
+  CHECK(result_array->Get(0)->IsString());
+  CHECK(result_array->Get(1)->IsString());
+  CHECK(result_array->Get(2)->IsString());
+  CHECK_EQ(v8_str("7"), result_array->Get(0));
+  CHECK_EQ(v8_str("[object Object]"), result_array->Get(1));
+  CHECK_EQ(v8_str("x"), result_array->Get(2));
 }