[JSC] Object.getOwnPropertyDescriptors should not add undefined props to result

https://bugs.webkit.org/show_bug.cgi?id=159409

Reviewed by Geoffrey Garen.

* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorGetOwnPropertyDescriptors):
* tests/es6.yaml:
* tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js:
(testPropertiesIndexedSetterOnPrototypeThrows.set get var): Deleted.
(testPropertiesIndexedSetterOnPrototypeThrows): Deleted.
* tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js: Renamed from Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js.
* tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors.js: Renamed from Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@203747 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index d431092..0c42b39 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
+2016-07-26  Caitlin Potter  <caitp@igalia.com>
+
+        [JSC] Object.getOwnPropertyDescriptors should not add undefined props to result
+        https://bugs.webkit.org/show_bug.cgi?id=159409
+
+        Reviewed by Geoffrey Garen.
+
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorGetOwnPropertyDescriptors):
+        * tests/es6.yaml:
+        * tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js:
+        (testPropertiesIndexedSetterOnPrototypeThrows.set get var): Deleted.
+        (testPropertiesIndexedSetterOnPrototypeThrows): Deleted.
+        * tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js: Renamed from Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js.
+        * tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors.js: Renamed from Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js.
+
 2016-07-26  Mark Lam  <mark.lam@apple.com>
 
         Remove unused DEBUG_WITH_BREAKPOINT configuration.

diff --git a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
index c795049..26a236c 100644 (file)
--- a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
+++ b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
@@ -263,10 +263,18 @@ JSValue objectConstructorGetOwnPropertyDescriptors(ExecState* exec, JSObject* ob
         return jsUndefined();
 
     for (auto& propertyName : properties) {
-        JSValue fromDescriptor = objectConstructorGetOwnPropertyDescriptor(exec, object, propertyName);
+        PropertyDescriptor descriptor;
+        bool didGetDescriptor = object->getOwnPropertyDescriptor(exec, propertyName, descriptor);
         if (exec->hadException())
             return jsUndefined();
 
+        if (!didGetDescriptor)
+            continue;
+
+        JSObject* fromDescriptor = constructObjectFromPropertyDescriptor(exec, descriptor);
+        if (!fromDescriptor)
+            return jsUndefined();
+
         PutPropertySlot slot(descriptors);
         descriptors->putOwnDataPropertyMayBeIndex(exec, propertyName, fromDescriptor, slot);
         ASSERT(!exec->hadException());

diff --git a/Source/JavaScriptCore/tests/es6.yaml b/Source/JavaScriptCore/tests/es6.yaml
index 3a5cd2d..84c3740 100644 (file)
--- a/Source/JavaScriptCore/tests/es6.yaml
+++ b/Source/JavaScriptCore/tests/es6.yaml
@@ -1232,9 +1232,3 @@
   cmd: runES6 :normal
 - path: es6/well-known_symbols_Symbol.match_String.prototype.includes.js
   cmd: runES6 :normal
-# Late-stage proposals for a future ECMAScript standard
-# FIXME: move these to a new directory?
-- path: es6/Object_static_methods_Object.getOwnPropertyDescriptors.js
-  cmd: runES6 :normal
-- path: es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js
-  cmd: runES6 :fail

diff --git a/Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js b/Source/JavaScriptCore/tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js
similarity index 67%
rename from Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js
rename to Source/JavaScriptCore/tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js
index 22781dc..1d09086 100644 (file)
--- a/Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js
+++ b/Source/JavaScriptCore/tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors-proxy.js
@@ -44,17 +44,17 @@ function shouldBeDataProperty(expected, value, name) {
     });
 
     var result = Object.getOwnPropertyDescriptors(P);
-    shouldBe('ownKeys()|getOwnPropertyDescriptor(0)|getOwnPropertyDescriptor(1)|getOwnPropertyDescriptor(a)|getOwnPropertyDescriptor(Symbol(foo))', log.join('|'));
+    shouldBe('ownKeys()|getOwnPropertyDescriptor(0)|getOwnPropertyDescriptor(1)|getOwnPropertyDescriptor(a)|getOwnPropertyDescriptor(Symbol(test))', log.join('|'));
     shouldBeDataProperty(result[0], 0, 'result[0]');
     shouldBeDataProperty(result[1], 1, 'result[1]');
-    shouldBeDataProperty(result.a, 1, 'result["a"]');
-    shouldBeDataProperty(result[sym], 1, 'result[Symbol(foo)]');
+    shouldBeDataProperty(result.a, 2, 'result["a"]');
+    shouldBeDataProperty(result[sym], 3, 'result[Symbol(test)]');
 
     var result2 = Object.getOwnPropertyDescriptors(O);
     shouldBeDataProperty(result2[0], 0, 'result2[0]');
     shouldBeDataProperty(result2[1], 1, 'result2[1]');
-    shouldBeDataProperty(result2.a, 1, 'result2["a"]');
-    shouldBeDataProperty(result2[sym], 1, 'result2[Symbol(foo)]');
+    shouldBeDataProperty(result2.a, 2, 'result2["a"]');
+    shouldBeDataProperty(result2[sym], 3, 'result2[Symbol(test)]');
 })();
 
 (function testDuplicatePropertyNames() {
@@ -81,7 +81,33 @@ function shouldBeDataProperty(expected, value, name) {
   });
 
   var result = Object.getOwnPropertyDescriptors(P);
-  shouldBe(void 0, result.A);
+  shouldBe(true, result.A.configurable, 'for result.A.configurable');
+  shouldBe(false, result.A.writable, 'for result.A.writable');
+  shouldBe('VALUE', result.A.value, 'for result.A.value');
+  shouldBe(false, result.A.enumerable, 'for result.A.enumerable');
   shouldBe(true, Object.hasOwnProperty.call(result, 'A'));
   shouldBe('ownKeys()|getOwnPropertyDescriptor(A)|getOwnPropertyDescriptor(A)', log.join('|'));
 })();
+
+(function testUndefinedPropertyDescriptor() {
+    var log = [];
+    var P = new Proxy({}, {
+        ownKeys() {
+            log.push(`ownKeys()`);
+            return ['fakeProperty'];
+        },
+        getOwnPropertyDescriptor(t, name) {
+            log.push(`getOwnPropertyDescriptor(${name})`);
+            return undefined;
+        },
+        get() { throw new Error('[[Get]] trap should be unreachable'); },
+        set() { throw new Error('[[Set]] trap should be unreachable'); },
+        deleteProperty() { throw new Error('[[Delete]] trap should be unreachable'); },
+        defineProperty() { throw new Error('[[DefineOwnProperty]] trap should be unreachable'); }
+    });
+
+    var result = Object.getOwnPropertyDescriptors(P);
+    shouldBe(false, result.hasOwnProperty('fakeProperty'));
+    shouldBe(false, 'fakeProperty' in result);
+    shouldBe('ownKeys()|getOwnPropertyDescriptor(fakeProperty)', log.join('|'));
+})();

diff --git a/Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js b/Source/JavaScriptCore/tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors.js
similarity index 100%
rename from Source/JavaScriptCore/tests/es6/Object_static_methods_Object.getOwnPropertyDescriptors.js
rename to Source/JavaScriptCore/tests/stress/Object_static_methods_Object.getOwnPropertyDescriptors.js