[proxies] Wire up Object.getOwnPropertyDescriptor

using the new JSReceiver::GetOwnPropertyDescriptor implementation.

BUG=v8:1543
LOG=n
R=cbruni@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#32037}

src/js/v8natives.js

@@ -309,24 +309,6 @@ function IsInconsistentDescriptor(desc) {
 }
 
 
-// ES5 8.10.4
-function FromPropertyDescriptor(desc) {
-  if (IS_UNDEFINED(desc)) return desc;
-
-  if (IsDataDescriptor(desc)) {
-    return { value: desc.getValue(),
-             writable: desc.isWritable(),
-             enumerable: desc.isEnumerable(),
-             configurable: desc.isConfigurable() };
-  }
-  // Must be an AccessorDescriptor then. We never return a generic descriptor.
-  return { get: desc.getGet(),
-           set: desc.getSet(),
-           enumerable: desc.isEnumerable(),
-           configurable: desc.isConfigurable() };
-}
-
-
 // Harmony Proxies
 function FromGenericPropertyDescriptor(desc) {
   if (IS_UNDEFINED(desc)) return desc;
@@ -569,6 +551,8 @@ function CallTrap2(handler, name, defaultTrap, x, y) {
 
 
 // ES5 section 8.12.1.
+// TODO(jkummerow): Deprecated. Migrate all callers to
+// ObjectGetOwnPropertyDescriptor and delete this.
 function GetOwnPropertyJS(obj, v) {
   var p = TO_NAME(v);
   if (%_IsJSProxy(obj)) {
@@ -590,7 +574,7 @@ function GetOwnPropertyJS(obj, v) {
   // GetOwnProperty returns an array indexed by the constants
   // defined in macros.py.
   // If p is not a property on obj undefined is returned.
-  var props = %GetOwnProperty(TO_OBJECT(obj), p);
+  var props = %GetOwnProperty_Legacy(TO_OBJECT(obj), p);
 
   return ConvertDescriptorArrayToDescriptor(props);
 }
@@ -641,7 +625,7 @@ function DefineProxyProperty(obj, p, attributes, should_throw) {
 
 // ES5 8.12.9.
 function DefineObjectProperty(obj, p, desc, should_throw) {
-  var current_array = %GetOwnProperty(obj, TO_NAME(p));
+  var current_array = %GetOwnProperty_Legacy(obj, TO_NAME(p));
   var current = ConvertDescriptorArrayToDescriptor(current_array);
   var extensible = %IsExtensible(obj);
 
@@ -889,8 +873,7 @@ function ObjectSetPrototypeOf(obj, proto) {
 
 // ES6 section 19.1.2.6
 function ObjectGetOwnPropertyDescriptor(obj, p) {
-  var desc = GetOwnPropertyJS(TO_OBJECT(obj), p);
-  return FromPropertyDescriptor(desc);
+  return %GetOwnProperty(obj, p);
 }
 
 

src/objects.cc

@@ -6171,11 +6171,12 @@ bool JSReceiver::OrdinaryDefineOwnProperty(Isolate* isolate,
 bool JSReceiver::OrdinaryDefineOwnProperty(LookupIterator* it,
                                            PropertyDescriptor* desc,
                                            ShouldThrow should_throw) {
+  Isolate* isolate = it->isolate();
   // 1. Let current be O.[[GetOwnProperty]](P).
   // 2. ReturnIfAbrupt(current).
   PropertyDescriptor current;
   if (!GetOwnPropertyDescriptor(it, &current) &&
-      it->isolate()->has_pending_exception()) {
+      isolate->has_pending_exception()) {
     return false;
   }
   // TODO(jkummerow/verwaest): It would be nice if we didn't have to reset
@@ -6187,35 +6188,35 @@ bool JSReceiver::OrdinaryDefineOwnProperty(LookupIterator* it,
   Handle<JSObject> object = Handle<JSObject>::cast(it->GetReceiver());
   bool extensible = JSObject::IsExtensible(object);
 
-  return ValidateAndApplyPropertyDescriptor(it, extensible, desc, &current,
-                                            should_throw);
+  return ValidateAndApplyPropertyDescriptor(isolate, it, extensible, desc,
+                                            &current, should_throw);
 }
 
 
 // ES6 9.1.6.2
 // static
-bool JSReceiver::IsCompatiblePropertyDescriptor(bool extensible,
+bool JSReceiver::IsCompatiblePropertyDescriptor(Isolate* isolate,
+                                                bool extensible,
                                                 PropertyDescriptor* desc,
                                                 PropertyDescriptor* current,
                                                 Handle<Name> property_name) {
   // 1. Return ValidateAndApplyPropertyDescriptor(undefined, undefined,
   //    Extensible, Desc, Current).
-  return ValidateAndApplyPropertyDescriptor(NULL, extensible, desc, current,
-                                            THROW_ON_ERROR, property_name);
+  return ValidateAndApplyPropertyDescriptor(
+      isolate, NULL, extensible, desc, current, THROW_ON_ERROR, property_name);
 }
 
 
 // ES6 9.1.6.3
 // static
 bool JSReceiver::ValidateAndApplyPropertyDescriptor(
-    LookupIterator* it, bool extensible, PropertyDescriptor* desc,
-    PropertyDescriptor* current, ShouldThrow should_throw,
-    Handle<Name> property_name) {
+    Isolate* isolate, LookupIterator* it, bool extensible,
+    PropertyDescriptor* desc, PropertyDescriptor* current,
+    ShouldThrow should_throw, Handle<Name> property_name) {
   // We either need a LookupIterator, or a property name.
   DCHECK((it == NULL) != property_name.is_null());
   Handle<JSObject> object;
   if (it != NULL) object = Handle<JSObject>::cast(it->GetReceiver());
-  Isolate* isolate = it->isolate();
   bool desc_is_data_descriptor = PropertyDescriptor::IsDataDescriptor(desc);
   bool desc_is_accessor_descriptor =
       PropertyDescriptor::IsAccessorDescriptor(desc);
@@ -6877,7 +6878,7 @@ bool JSProxy::GetOwnPropertyDescriptor(LookupIterator* it,
   PropertyDescriptor::CompletePropertyDescriptor(isolate, desc);
   // 15. Let valid be IsCompatiblePropertyDescriptor (extensibleTarget,
   //     resultDesc, targetDesc).
-  bool valid = IsCompatiblePropertyDescriptor(extensible_target, desc,
+  bool valid = IsCompatiblePropertyDescriptor(isolate, extensible_target, desc,
                                               &target_desc, property_name);
   // 16. If valid is false, throw a TypeError exception.
   if (!valid) {

src/objects.h

@@ -1847,7 +1847,7 @@ class JSReceiver: public HeapObject {
                                         PropertyDescriptor* desc,
                                         ShouldThrow should_throw);
   // ES6 9.1.6.2
-  static bool IsCompatiblePropertyDescriptor(bool extensible,
+  static bool IsCompatiblePropertyDescriptor(Isolate* isolate, bool extensible,
                                              PropertyDescriptor* desc,
                                              PropertyDescriptor* current,
                                              Handle<Name> property_name);
@@ -1855,9 +1855,9 @@ class JSReceiver: public HeapObject {
   // |it| can be NULL in cases where the ES spec passes |undefined| as the
   // receiver. Exactly one of |it| and |property_name| must be provided.
   static bool ValidateAndApplyPropertyDescriptor(
-      LookupIterator* it, bool extensible, PropertyDescriptor* desc,
-      PropertyDescriptor* current, ShouldThrow should_throw,
-      Handle<Name> property_name = Handle<Name>());
+      Isolate* isolate, LookupIterator* it, bool extensible,
+      PropertyDescriptor* desc, PropertyDescriptor* current,
+      ShouldThrow should_throw, Handle<Name> property_name = Handle<Name>());
 
   static bool GetOwnPropertyDescriptor(Isolate* isolate,
                                        Handle<JSReceiver> object,

src/runtime/runtime-object.cc

@@ -9,6 +9,7 @@
 #include "src/debug/debug.h"
 #include "src/isolate-inl.h"
 #include "src/messages.h"
+#include "src/property-descriptor.h"
 #include "src/runtime/runtime.h"
 
 namespace v8 {
@@ -253,10 +254,8 @@ MUST_USE_RESULT static MaybeHandle<Object> GetOwnProperty(Isolate* isolate,
 //         [false, value, Writeable, Enumerable, Configurable]
 //  if args[1] is an accessor on args[0]
 //         [true, GetFunction, SetFunction, Enumerable, Configurable]
-RUNTIME_FUNCTION(Runtime_GetOwnProperty) {
-  // TODO(jkummerow): Support Proxies.
-  // TODO(jkummerow): Use JSReceiver::GetOwnPropertyDescriptor() and
-  //                  PropertyDescriptor::ToObject().
+// TODO(jkummerow): Deprecated. Remove all callers and delete.
+RUNTIME_FUNCTION(Runtime_GetOwnProperty_Legacy) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 2);
   CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0);
@@ -268,6 +267,31 @@ RUNTIME_FUNCTION(Runtime_GetOwnProperty) {
 }
 
 
+// ES6 19.1.2.6
+RUNTIME_FUNCTION(Runtime_GetOwnProperty) {
+  HandleScope scope(isolate);
+  DCHECK(args.length() == 2);
+  CONVERT_ARG_HANDLE_CHECKED(Object, object, 0);
+  CONVERT_ARG_HANDLE_CHECKED(Object, raw_name, 1);
+  // 1. Let obj be ? ToObject(O).
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, object,
+                                     Execution::ToObject(isolate, object));
+  // 2. Let key be ? ToPropertyKey(P).
+  Handle<Name> key;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, key,
+                                     Object::ToName(isolate, raw_name));
+
+  // 3. Let desc be ? obj.[[GetOwnProperty]](key).
+  PropertyDescriptor desc;
+  bool found = JSReceiver::GetOwnPropertyDescriptor(
+      isolate, Handle<JSReceiver>::cast(object), key, &desc);
+  if (isolate->has_pending_exception()) return isolate->heap()->exception();
+  // 4. Return FromPropertyDescriptor(desc).
+  if (!found) return isolate->heap()->undefined_value();
+  return *desc.ToObject(isolate);
+}
+
+
 RUNTIME_FUNCTION(Runtime_PreventExtensions) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 1);

src/runtime/runtime.h

@@ -431,6 +431,7 @@ namespace internal {
   F(InternalSetPrototype, 2, 1)                      \
   F(SetPrototype, 2, 1)                              \
   F(GetOwnProperty, 2, 1)                            \
+  F(GetOwnProperty_Legacy, 2, 1)                     \
   F(PreventExtensions, 1, 1)                         \
   F(IsExtensible, 1, 1)                              \
   F(OptimizeObjectForAddingMultipleProperties, 2, 1) \

test/mjsunit/harmony/proxies-get-own-property-descriptor.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-proxies
+
+var target = {};
+var configurable_desc = {
+  value: 123,
+  configurable: true,
+  writable: true,
+  enumerable: false,
+};
+Object.defineProperty(target, "configurable", configurable_desc);
+var nonconfigurable_desc = {
+  value: 234,
+  configurable: false,
+  writable: false,
+  enumerable: true
+}
+Object.defineProperty(target, "nonconfigurable", nonconfigurable_desc);
+
+var proxied_desc = {
+  value: 345,
+  configurable: true
+};
+
+var handler = {
+  "getOwnPropertyDescriptor": function(target, name) {
+    if (name === "proxied") {
+      return proxied_desc;
+    }
+    if (name === "return_null") {
+      return null;
+    }
+    return Object.getOwnPropertyDescriptor(target, name);
+  }
+};
+
+var proxy = new Proxy(target, handler);
+var proxy_without_handler = new Proxy(target, {});
+
+// Checking basic functionality:
+
+assertEquals(configurable_desc,
+             Object.getOwnPropertyDescriptor(proxy, "configurable"));
+assertEquals(nonconfigurable_desc,
+             Object.getOwnPropertyDescriptor(proxy, "nonconfigurable"));
+assertEquals({ value: proxied_desc.value,
+               configurable: proxied_desc.configurable,
+               enumerable: false,
+               writable: false },
+             Object.getOwnPropertyDescriptor(proxy, "proxied"));
+assertEquals(configurable_desc,
+             Object.getOwnPropertyDescriptor(proxy_without_handler,
+                                             "configurable"));
+assertEquals(nonconfigurable_desc,
+             Object.getOwnPropertyDescriptor(proxy_without_handler,
+                                             "nonconfigurable"));
+
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "return_null")');
+
+handler.getOwnPropertyDescriptor = undefined;
+assertEquals(configurable_desc,
+             Object.getOwnPropertyDescriptor(proxy, "configurable"));
+
+// Checking invariants mentioned explicitly by the ES spec:
+
+// (Inv-1) "A property cannot be reported as non-existent, if it exists as a
+// non-configurable own property of the target object."
+handler.getOwnPropertyDescriptor = function(target, name) { return undefined; };
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "nonconfigurable")');
+assertEquals(undefined, Object.getOwnPropertyDescriptor(proxy, "configurable"));
+
+// (Inv-2) "A property cannot be reported as non-configurable, if it does not
+// exist as an own property of the target object or if it exists as a
+// configurable own property of the target object."
+handler.getOwnPropertyDescriptor = function(target, name) {
+  return {value: 234, configurable: false, enumerable: true};
+};
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "nonexistent")');
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "configurable")');
+assertEquals(
+    false,
+    Object.getOwnPropertyDescriptor(proxy, "nonconfigurable").configurable);
+
+// (Inv-3) "A property cannot be reported as non-existent, if it exists as an
+// own property of the target object and the target object is not extensible."
+Object.seal(target);
+handler.getOwnPropertyDescriptor = function(target, name) { return undefined; };
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "configurable")');
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "nonconfigurable")');
+assertEquals(undefined, Object.getOwnPropertyDescriptor(proxy, "nonexistent"));
+
+// (Inv-4) "A property cannot be reported as existent, if it does not exist as
+// an own property of the target object and the target object is not
+// extensible."
+var existent_desc = {value: "yes"};
+handler.getOwnPropertyDescriptor = function() { return existent_desc; };
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "nonexistent")');
+assertEquals(
+    {value: "yes", writable: false, enumerable: false, configurable: false},
+    Object.getOwnPropertyDescriptor(proxy, "configurable"));
+
+// Checking individual bailout points in the implementation:
+
+// Step 6: Trap is not callable.
+handler.getOwnPropertyDescriptor = {};
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "configurable")');
+
+// Step 8: Trap throws.
+handler.getOwnPropertyDescriptor = function() { throw "ball"; };
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "configurable")');
+
+// Step 9: Trap result is neither undefined nor an object.
+handler.getOwnPropertyDescriptor = function() { return 1; }
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "configurable")');
+
+// Step 11b: See (Inv-1) above.
+// Step 11e: See (Inv-3) above.
+
+// Step 16: Incompatible PropertyDescriptor; a non-configurable property
+// cannot be reported as configurable. (Inv-4) above checks more cases.
+handler.getOwnPropertyDescriptor = function(target, name) {
+  return {value: 456, configurable: true, writable: true}
+};
+assertThrows('Object.getOwnPropertyDescriptor(proxy, "nonconfigurable")');
+
+// Step 17: See (Inv-2) above.