Implement Object.defineProperty for proxies.

R=kmillikin@chromium.org
BUG=
TEST=

Review URL: http://codereview.chromium.org/7314003

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

diff --git a/src/messages.js b/src/messages.js
index 841c518..b928107 100644 (file)
--- a/src/messages.js
+++ b/src/messages.js
@@ -195,6 +195,7 @@ function FormatMessage(message) {
       non_extensible_proto:         ["%0", " is not extensible"],
       handler_non_object:           ["Proxy.", "%0", " called with non-object as handler"],
       handler_trap_missing:         ["Proxy handler ", "%0", " has no '", "%1", "' trap"],
+      handler_failed:               ["Proxy handler ", "%0", " returned false for '", "%1", "' trap"],
       proxy_prop_not_configurable:  ["Trap ", "%1", " of proxy handler ", "%0", " returned non-configurable descriptor for property ", "%2"],
       proxy_non_object_prop_names:  ["Trap ", "%1", " returned non-object ", "%0"],
       proxy_repeated_prop_name:     ["Trap ", "%1", " returned repeated property name ", "%2"],

diff --git a/src/runtime.cc b/src/runtime.cc
index 3131581..bddc37d 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -628,11 +628,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetPrototype) {
   NoHandleAllocation ha;
   ASSERT(args.length() == 1);
   Object* obj = args[0];
-  obj = obj->GetPrototype();
-  while (obj->IsJSObject() &&
-         JSObject::cast(obj)->map()->is_hidden_prototype()) {
+  do {
     obj = obj->GetPrototype();
-  }
+  } while (obj->IsJSObject() &&
+           JSObject::cast(obj)->map()->is_hidden_prototype());
   return obj;
 }
 

diff --git a/src/v8natives.js b/src/v8natives.js
index 0afe231..831dd14 100644 (file)
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -315,14 +315,14 @@ function ObjectKeys(obj) {
 // ES5 8.10.1.
 function IsAccessorDescriptor(desc) {
   if (IS_UNDEFINED(desc)) return false;
-  return desc.hasGetter_ || desc.hasSetter_;
+  return desc.hasGetter() || desc.hasSetter();
 }
 
 
 // ES5 8.10.2.
 function IsDataDescriptor(desc) {
   if (IS_UNDEFINED(desc)) return false;
-  return desc.hasValue_ || desc.hasWritable_;
+  return desc.hasValue() || desc.hasWritable();
 }
 
 
@@ -354,6 +354,19 @@ function FromPropertyDescriptor(desc) {
   return obj;
 }
 
+// Harmony Proxies
+function FromGenericPropertyDescriptor(desc) {
+  if (IS_UNDEFINED(desc)) return desc;
+  var obj = new $Object();
+  if (desc.hasValue()) obj.value = desc.getValue();
+  if (desc.hasWritable()) obj.writable = desc.isWritable();
+  if (desc.hasGetter()) obj.get = desc.getGet();
+  if (desc.hasSetter()) obj.set = desc.getSet();
+  if (desc.hasEnumerable()) obj.enumerable = desc.isEnumerable();
+  if (desc.hasConfigurable()) obj.configurable = desc.isConfigurable();
+  return obj;
+}
+
 // ES5 8.10.5.
 function ToPropertyDescriptor(obj) {
   if (!IS_SPEC_OBJECT(obj)) {
@@ -404,15 +417,15 @@ function ToPropertyDescriptor(obj) {
 function ToCompletePropertyDescriptor(obj) {
   var desc = ToPropertyDescriptor(obj)
   if (IsGenericDescriptor(desc) || IsDataDescriptor(desc)) {
-    if (!("value" in desc)) desc.value = void 0;
-    if (!("writable" in desc)) desc.writable = false;
+    if (!desc.hasValue()) desc.setValue(void 0);
+    if (!desc.hasWritable()) desc.setWritable(false);
   } else {
     // Is accessor descriptor.
-    if (!("get" in desc)) desc.get = void 0;
-    if (!("set" in desc)) desc.set = void 0;
+    if (!desc.hasGetter()) desc.setGet(void 0);
+    if (!desc.hasSetter()) desc.setSet(void 0);
   }
-  if (!("enumerable" in desc)) desc.enumerable = false;
-  if (!("configurable" in desc)) desc.configurable = false;
+  if (!desc.hasEnumerable()) desc.setEnumerable(false);
+  if (!desc.hasConfigurable()) desc.setConfigurable(false);
   return desc;
 }
 
@@ -616,8 +629,32 @@ function GetOwnProperty(obj, p) {
 }
 
 
+// Harmony proxies.
+function DefineProxyProperty(obj, p, attributes, should_throw) {
+  var handler = %GetHandler(obj);
+  var defineProperty = handler.defineProperty;
+  if (IS_UNDEFINED(defineProperty)) {
+    throw MakeTypeError("handler_trap_missing", [handler, "defineProperty"]);
+  }
+  var result = defineProperty.call(handler, p, attributes);
+  if (!ToBoolean(result)) {
+    if (should_throw) {
+      throw MakeTypeError("handler_failed", [handler, "defineProperty"]);
+    } else {
+      return false;
+    }
+  }
+  return true;
+}
+
+
 // ES5 8.12.9.
 function DefineOwnProperty(obj, p, desc, should_throw) {
+  if (%IsJSProxy(obj)) {
+    var attributes = FromGenericPropertyDescriptor(desc);
+    return DefineProxyProperty(obj, p, attributes, should_throw);
+  }
+
   var current_or_access = %GetOwnProperty(ToObject(obj), ToString(p));
   // A false value here means that access checks failed.
   if (current_or_access === false) return void 0;
@@ -900,8 +937,37 @@ function ObjectDefineProperty(obj, p, attributes) {
     throw MakeTypeError("obj_ctor_property_non_object", ["defineProperty"]);
   }
   var name = ToString(p);
-  var desc = ToPropertyDescriptor(attributes);
-  DefineOwnProperty(obj, name, desc, true);
+  if (%IsJSProxy(obj)) {
+    // Clone the attributes object for protection.
+    // TODO(rossberg): not spec'ed yet, so not sure if this should involve
+    // non-own properties as it does (or non-enumerable ones, as it doesn't?).
+    var attributesClone = {}
+    for (var a in attributes) {
+      attributesClone[a] = attributes[a];
+    }
+    DefineProxyProperty(obj, name, attributesClone, true);
+    // The following would implement the spec as in the current proposal,
+    // but after recent comments on es-discuss, is most likely obsolete.
+    /*
+    var defineObj = FromGenericPropertyDescriptor(desc);
+    var names = ObjectGetOwnPropertyNames(attributes);
+    var standardNames =
+      {value: 0, writable: 0, get: 0, set: 0, enumerable: 0, configurable: 0};
+    for (var i = 0; i < names.length; i++) {
+      var N = names[i];
+      if (!(%HasLocalProperty(standardNames, N))) {
+        var attr = GetOwnProperty(attributes, N);
+        DefineOwnProperty(descObj, N, attr, true);
+      }
+    }
+    // This is really confusing the types, but it is what the proxies spec
+    // currently requires:
+    desc = descObj;
+    */
+  } else {
+    var desc = ToPropertyDescriptor(attributes);
+    DefineOwnProperty(obj, name, desc, true);
+  }
   return obj;
 }
 

diff --git a/test/mjsunit/harmony/proxies.js b/test/mjsunit/harmony/proxies.js
index 62bee87..490877c 100644 (file)
--- a/test/mjsunit/harmony/proxies.js
+++ b/test/mjsunit/harmony/proxies.js
@@ -149,6 +149,78 @@ TestSet(Proxy.create({
 
 
 
+// Property definition (Object.defineProperty).
+
+var key
+var desc
+function TestDefine(handler) {
+  var o = Proxy.create(handler)
+  assertEquals(o, Object.defineProperty(o, "a", {value: 44}))
+  assertEquals("a", key)
+  assertEquals(1, Object.getOwnPropertyNames(desc).length)
+  assertEquals(44, desc.value)
+
+  assertEquals(o, Object.defineProperty(o, "b", {value: 45, writable: false}))
+  assertEquals("b", key)
+  assertEquals(2, Object.getOwnPropertyNames(desc).length)
+  assertEquals(45, desc.value)
+  assertEquals(false, desc.writable)
+
+  assertEquals(o, Object.defineProperty(o, "c", {value: 46, enumerable: false}))
+  assertEquals("c", key)
+  assertEquals(2, Object.getOwnPropertyNames(desc).length)
+  assertEquals(46, desc.value)
+  assertEquals(false, desc.enumerable)
+
+  var attributes = {configurable: true, mine: 66, minetoo: 23}
+  assertEquals(o, Object.defineProperty(o, "d", attributes))
+  assertEquals("d", key)
+  // Modifying the attributes object after the fact should have no effect.
+  attributes.configurable = false
+  attributes.mine = 77
+  delete attributes.minetoo
+  assertEquals(3, Object.getOwnPropertyNames(desc).length)
+  assertEquals(true, desc.configurable)
+  assertEquals(66, desc.mine)
+  assertEquals(23, desc.minetoo)
+
+  assertEquals(o, Object.defineProperty(o, "e", {get: function(){ return 5 }}))
+  assertEquals("e", key)
+  assertEquals(1, Object.getOwnPropertyNames(desc).length)
+  assertEquals(5, desc.get())
+
+  assertEquals(o, Object.defineProperty(o, "zzz", {}))
+  assertEquals("zzz", key)
+  assertEquals(0, Object.getOwnPropertyNames(desc).length)
+
+// This test requires [s in proxy] to be implemented first.
+//  var d = Proxy.create({
+//    get: function(r, k) { return (k === "value") ? 77 : void 0 },
+//    getOwnPropertyNames: function() { return ["value"] }
+//  })
+//  assertEquals(1, Object.getOwnPropertyNames(d).length)
+//  assertEquals(77, d.value)
+//  assertEquals(o, Object.defineProperty(o, "p", d))
+//  assertEquals("p", key)
+//  assertEquals(1, Object.getOwnPropertyNames(desc).length)
+//  assertEquals(77, desc.value)
+}
+
+TestDefine({
+  defineProperty: function(k, d) { key = k; desc = d; return true }
+})
+TestDefine({
+  defineProperty: function(k, d) { return this.defineProperty2(k, d) },
+  defineProperty2: function(k, d) { key = k; desc = d; return true }
+})
+TestDefine(Proxy.create({
+  get: function(pr, pk) {
+    return function(k, d) { key = k; desc = d; return true }
+  }
+}))
+
+
+
 // Comparison.
 
 function TestComparison(eq) {