[proxies] Teach ToPropertyDescriptor to deal with Proxies

BUG=v8:1543
LOG=n

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

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

src/property-descriptor.cc

@@ -20,11 +20,11 @@ bool GetPropertyIfPresent(Handle<Object> obj, Handle<String> name,
                           Handle<Object>* value) {
   LookupIterator it(obj, name);
   // 4. Let hasEnumerable be HasProperty(Obj, "enumerable").
-  Maybe<PropertyAttributes> maybe_attr = JSReceiver::GetPropertyAttributes(&it);
+  Maybe<bool> has_property = JSReceiver::HasProperty(&it);
   // 5. ReturnIfAbrupt(hasEnumerable).
-  if (!maybe_attr.IsJust()) return false;
+  if (has_property.IsNothing()) return false;
   // 6. If hasEnumerable is true, then
-  if (maybe_attr.FromJust() != ABSENT) {
+  if (has_property.FromJust() == true) {
     // 6a. Let enum be ToBoolean(Get(Obj, "enumerable")).
     // 6b. ReturnIfAbrupt(enum).
     if (!JSObject::GetProperty(&it).ToHandle(value)) return false;
@@ -160,11 +160,7 @@ bool PropertyDescriptor::ToPropertyDescriptor(Isolate* isolate,
     return true;
   }
 
-  // TODO(jkummerow): Implement JSProxy support.
-  // Specifically, instead of taking the attributes != ABSENT shortcut, we
-  // have to implement proper HasProperty for proxies.
-  if (!obj->IsJSProxy()) {
-    {  // enumerable?
+  // enumerable?
   Handle<Object> enumerable;
   // 4 through 6b.
   if (!GetPropertyIfPresent(obj, isolate->factory()->enumerable_string(),
@@ -175,8 +171,8 @@ bool PropertyDescriptor::ToPropertyDescriptor(Isolate* isolate,
   if (!enumerable.is_null()) {
     desc->set_enumerable(enumerable->BooleanValue());
   }
-    }
-    {  // configurable?
+
+  // configurable?
   Handle<Object> configurable;
   // 7 through 9b.
   if (!GetPropertyIfPresent(obj, isolate->factory()->configurable_string(),
@@ -187,17 +183,17 @@ bool PropertyDescriptor::ToPropertyDescriptor(Isolate* isolate,
   if (!configurable.is_null()) {
     desc->set_configurable(configurable->BooleanValue());
   }
-    }
-    {  // value?
+
+  // value?
   Handle<Object> value;
   // 10 through 12b.
-      if (!GetPropertyIfPresent(obj, isolate->factory()->value_string(),
-                                &value))
+  if (!GetPropertyIfPresent(obj, isolate->factory()->value_string(), &value)) {
     return false;
+  }
   // 12c. Set the [[Value]] field of desc to value.
   if (!value.is_null()) desc->set_value(value);
-    }
-    {  // writable?
+
+  // writable?
   Handle<Object> writable;
   // 13 through 15b.
   if (!GetPropertyIfPresent(obj, isolate->factory()->writable_string(),
@@ -206,12 +202,13 @@ bool PropertyDescriptor::ToPropertyDescriptor(Isolate* isolate,
   }
   // 15c. Set the [[Writable]] field of desc to writable.
   if (!writable.is_null()) desc->set_writable(writable->BooleanValue());
-    }
-    {  // getter?
+
+  // getter?
   Handle<Object> getter;
   // 16 through 18b.
-      if (!GetPropertyIfPresent(obj, isolate->factory()->get_string(), &getter))
+  if (!GetPropertyIfPresent(obj, isolate->factory()->get_string(), &getter)) {
     return false;
+  }
   if (!getter.is_null()) {
     // 18c. If IsCallable(getter) is false and getter is not undefined,
     // throw a TypeError exception.
@@ -223,12 +220,12 @@ bool PropertyDescriptor::ToPropertyDescriptor(Isolate* isolate,
     // 18d. Set the [[Get]] field of desc to getter.
     desc->set_get(getter);
   }
-      {  // setter?
+  // setter?
   Handle<Object> setter;
   // 19 through 21b.
-        if (!GetPropertyIfPresent(obj, isolate->factory()->set_string(),
-                                  &setter))
+  if (!GetPropertyIfPresent(obj, isolate->factory()->set_string(), &setter)) {
     return false;
+  }
   if (!setter.is_null()) {
     // 21c. If IsCallable(setter) is false and setter is not undefined,
     // throw a TypeError exception.
@@ -240,7 +237,7 @@ bool PropertyDescriptor::ToPropertyDescriptor(Isolate* isolate,
     // 21d. Set the [[Set]] field of desc to setter.
     desc->set_set(setter);
   }
-      }
+
   // 22. If either desc.[[Get]] or desc.[[Set]] is present, then
   // 22a. If either desc.[[Value]] or desc.[[Writable]] is present,
   // throw a TypeError exception.
@@ -250,19 +247,54 @@ bool PropertyDescriptor::ToPropertyDescriptor(Isolate* isolate,
         MessageTemplate::kValueAndAccessor, obj));
     return false;
   }
-    }
-  } else {
-    DCHECK(obj->IsJSProxy());
-    // Having an UNIMPLEMENTED() here would upset ClusterFuzz, because
-    // --harmony-proxies makes it possible to reach this branch.
-    isolate->Throw(
-        *isolate->factory()->NewTypeError(MessageTemplate::kUnsupported));
-    return false;
-  }
+
   // 23. Return desc.
   return true;
 }
 
 
+// ES6 6.2.4.6
+// static
+void PropertyDescriptor::CompletePropertyDescriptor(Isolate* isolate,
+                                                    PropertyDescriptor* desc) {
+  // 1. ReturnIfAbrupt(Desc).
+  // 2. Assert: Desc is a Property Descriptor.
+  // 3. Let like be Record{
+  //        [[Value]]: undefined, [[Writable]]: false,
+  //        [[Get]]: undefined, [[Set]]: undefined,
+  //        [[Enumerable]]: false, [[Configurable]]: false}.
+  // 4. If either IsGenericDescriptor(Desc) or IsDataDescriptor(Desc) is true,
+  // then:
+  if (!IsAccessorDescriptor(desc)) {
+    // 4a. If Desc does not have a [[Value]] field, set Desc.[[Value]] to
+    //     like.[[Value]].
+    if (!desc->has_value()) {
+      desc->set_value(isolate->factory()->undefined_value());
+    }
+    // 4b. If Desc does not have a [[Writable]] field, set Desc.[[Writable]]
+    //     to like.[[Writable]].
+    if (!desc->has_writable()) desc->set_writable(false);
+  } else {
+    // 5. Else,
+    // 5a. If Desc does not have a [[Get]] field, set Desc.[[Get]] to
+    //     like.[[Get]].
+    if (!desc->has_get()) {
+      desc->set_get(isolate->factory()->undefined_value());
+    }
+    // 5b. If Desc does not have a [[Set]] field, set Desc.[[Set]] to
+    //     like.[[Set]].
+    if (!desc->has_set()) {
+      desc->set_set(isolate->factory()->undefined_value());
+    }
+  }
+  // 6. If Desc does not have an [[Enumerable]] field, set
+  //    Desc.[[Enumerable]] to like.[[Enumerable]].
+  if (!desc->has_enumerable()) desc->set_enumerable(false);
+  // 7. If Desc does not have a [[Configurable]] field, set
+  //    Desc.[[Configurable]] to like.[[Configurable]].
+  if (!desc->has_configurable()) desc->set_configurable(false);
+  // 8. Return Desc.
+}
+
 }  // namespace internal
 }  // namespace v8

src/property-descriptor.h

@@ -41,6 +41,17 @@ class PropertyDescriptor {
     return !IsAccessorDescriptor(desc) && !IsDataDescriptor(desc);
   }
 
+  // ES6 6.2.4.4
+  Handle<Object> ToObject(Isolate* isolate);
+
+  // ES6 6.2.4.5
+  static bool ToPropertyDescriptor(Isolate* isolate, Handle<Object> obj,
+                                   PropertyDescriptor* desc);
+
+  // ES6 6.2.4.6
+  static void CompletePropertyDescriptor(Isolate* isolate,
+                                         PropertyDescriptor* desc);
+
   bool is_empty() const {
     return !has_enumerable() && !has_configurable() && !has_writable() &&
            !has_value() && !has_get() && !has_set();
@@ -89,11 +100,6 @@ class PropertyDescriptor {
         (has_writable() && !writable() ? READ_ONLY : NONE));
   }
 
-  Handle<Object> ToObject(Isolate* isolate);
-
-  static bool ToPropertyDescriptor(Isolate* isolate, Handle<Object> obj,
-                                   PropertyDescriptor* desc);
-
  private:
   bool enumerable_ : 1;
   bool has_enumerable_ : 1;

src/runtime/runtime-object.cc

@@ -251,6 +251,9 @@ MUST_USE_RESULT static MaybeHandle<Object> GetOwnProperty(Isolate* isolate,
 //  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().
   HandleScope scope(isolate);
   DCHECK(args.length() == 2);
   CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0);
@@ -656,6 +659,8 @@ RUNTIME_FUNCTION(Runtime_HasOwnProperty) {
     // Fast case: either the key is a real named property or it is not
     // an array index and there are no interceptors or hidden
     // prototypes.
+    // TODO(jkummerow): Make JSReceiver::HasOwnProperty fast enough to
+    // handle all cases directly (without this custom fast path).
     Maybe<bool> maybe = Nothing<bool>();
     if (key_is_array_index) {
       maybe = JSObject::HasOwnElement(js_obj, index);
@@ -757,6 +762,8 @@ RUNTIME_FUNCTION(Runtime_GetPropertyNamesFast) {
 // Return the names of the own named properties.
 // args[0]: object
 // args[1]: PropertyAttributes as int
+// TODO(cbruni/jkummerow): Use JSReceiver::GetKeys() internally, merge with
+// Runtime_GetOwnElementNames.
 RUNTIME_FUNCTION(Runtime_GetOwnPropertyNames) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 2);