Reland "Use C++ implementation of Object.definePropert{y,ies}"
This reverts commit 581ead5c8ceeb0e4ee2049d78972418236f5d3ba.
Review URL: https://codereview.chromium.org/1413123004
Cr-Commit-Position: refs/heads/master@{#31473}
diff --git a/src/js/v8natives.js b/src/js/v8natives.js
index 7c2ea48..3c1e175 100644
--- a/src/js/v8natives.js
+++ b/src/js/v8natives.js
@@ -1038,42 +1038,47 @@
// ES5 section 15.2.3.6.
function ObjectDefineProperty(obj, p, attributes) {
- if (!IS_SPEC_OBJECT(obj)) {
- throw MakeTypeError(kCalledOnNonObject, "Object.defineProperty");
- }
- var name = TO_NAME(p);
- 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 = { __proto__: null };
- for (var a in attributes) {
- attributesClone[a] = attributes[a];
+ // The new pure-C++ implementation doesn't support Proxies yet, nor O.o.
+ // TODO(jkummerow): Implement missing features and remove fallback path.
+ if (%_IsJSProxy(obj) || %IsObserved(obj)) {
+ if (!IS_SPEC_OBJECT(obj)) {
+ throw MakeTypeError(kCalledOnNonObject, "Object.defineProperty");
}
- 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 (!(%HasOwnProperty(standardNames, N))) {
- var attr = GetOwnPropertyJS(attributes, N);
- DefineOwnProperty(descObj, N, attr, true);
+ var name = TO_NAME(p);
+ 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 = { __proto__: null };
+ 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 (!(%HasOwnProperty(standardNames, N))) {
+ var attr = GetOwnPropertyJS(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);
}
- // 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;
}
- return obj;
+ return %ObjectDefineProperty(obj, p, attributes);
}
@@ -1101,19 +1106,24 @@
// ES5 section 15.2.3.7.
function ObjectDefineProperties(obj, properties) {
- if (!IS_SPEC_OBJECT(obj)) {
- throw MakeTypeError(kCalledOnNonObject, "Object.defineProperties");
+ // The new pure-C++ implementation doesn't support Proxies yet, nor O.o.
+ // TODO(jkummerow): Implement missing features and remove fallback path.
+ if (%_IsJSProxy(obj) || %_IsJSProxy(properties) || %IsObserved(obj)) {
+ if (!IS_SPEC_OBJECT(obj)) {
+ throw MakeTypeError(kCalledOnNonObject, "Object.defineProperties");
+ }
+ var props = TO_OBJECT(properties);
+ var names = GetOwnEnumerablePropertyNames(props);
+ var descriptors = new InternalArray();
+ for (var i = 0; i < names.length; i++) {
+ descriptors.push(ToPropertyDescriptor(props[names[i]]));
+ }
+ for (var i = 0; i < names.length; i++) {
+ DefineOwnProperty(obj, names[i], descriptors[i], true);
+ }
+ return obj;
}
- var props = TO_OBJECT(properties);
- var names = GetOwnEnumerablePropertyNames(props);
- var descriptors = new InternalArray();
- for (var i = 0; i < names.length; i++) {
- descriptors.push(ToPropertyDescriptor(props[names[i]]));
- }
- for (var i = 0; i < names.length; i++) {
- DefineOwnProperty(obj, names[i], descriptors[i], true);
- }
- return obj;
+ return %ObjectDefineProperties(obj, properties);
}
diff --git a/src/objects.cc b/src/objects.cc
index 5382b41..49e8b52 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -6024,6 +6024,17 @@
LookupIterator it = LookupIterator::PropertyOrElement(
isolate, object, key, &success, LookupIterator::HIDDEN);
DCHECK(success); // ...so creating a LookupIterator can't fail.
+
+ // Deal with access checks first.
+ if (it.state() == LookupIterator::ACCESS_CHECK) {
+ if (!it.HasAccess()) {
+ isolate->ReportFailedAccessCheck(it.GetHolder<JSObject>());
+ RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate, false);
+ return false;
+ }
+ it.Next();
+ }
+
return OrdinaryDefineOwnProperty(&it, desc, should_throw);
}
@@ -6063,6 +6074,13 @@
}
return false;
}
+ // We have to use a fresh LookupIterator to handle interceptors properly.
+ LookupIterator lookup_for_store =
+ it->IsElement() ? LookupIterator(isolate, it->GetReceiver(),
+ it->index(), LookupIterator::HIDDEN)
+ : LookupIterator(it->GetReceiver(), it->name(),
+ LookupIterator::HIDDEN);
+
// 2c. If IsGenericDescriptor(Desc) or IsDataDescriptor(Desc) is true, then:
// (This is equivalent to !IsAccessorDescriptor(desc).)
DCHECK((desc_is_generic_descriptor || desc_is_data_descriptor) ==
@@ -6082,8 +6100,9 @@
? desc->value()
: Handle<Object>::cast(isolate->factory()->undefined_value()));
MaybeHandle<Object> result =
- JSObject::DefineOwnPropertyIgnoreAttributes(it, value,
- desc->ToAttributes());
+ JSObject::DefineOwnPropertyIgnoreAttributes(
+ &lookup_for_store, value, desc->ToAttributes(),
+ JSObject::DONT_FORCE_FIELD);
if (result.is_null()) return false;
}
} else {
@@ -6105,8 +6124,8 @@
desc->has_set()
? desc->set()
: Handle<Object>::cast(isolate->factory()->undefined_value()));
- MaybeHandle<Object> result =
- JSObject::DefineAccessor(it, getter, setter, desc->ToAttributes());
+ MaybeHandle<Object> result = JSObject::DefineAccessor(
+ &lookup_for_store, getter, setter, desc->ToAttributes());
if (result.is_null()) return false;
}
}
@@ -6515,7 +6534,7 @@
if (!success && should_throw == THROW_ON_ERROR) {
isolate->Throw(*isolate->factory()->NewTypeError(
MessageTemplate::kStrictDeleteProperty,
- isolate->factory()->NewNumberFromUint(actual_new_len - 1)));
+ isolate->factory()->NewNumberFromUint(actual_new_len - 1), a));
}
return success;
}
@@ -7784,7 +7803,10 @@
DCHECK(filter == INCLUDE_SYMBOLS);
PropertyAttributes attr_filter =
static_cast<PropertyAttributes>(DONT_ENUM | PRIVATE_SYMBOL);
- JSObject::CollectOwnElementKeys(current, &accumulator, attr_filter);
+ Handle<FixedArray> property_keys = isolate->factory()->NewFixedArray(
+ current->NumberOfOwnProperties(attr_filter));
+ current->GetOwnPropertyNames(*property_keys, 0, attr_filter);
+ accumulator.AddKeys(property_keys);
}
// Add the property keys from the interceptor.
diff --git a/src/property-descriptor.cc b/src/property-descriptor.cc
index 442d056..2ecf75a 100644
--- a/src/property-descriptor.cc
+++ b/src/property-descriptor.cc
@@ -4,6 +4,7 @@
#include "src/property-descriptor.h"
+#include "src/bootstrapper.h"
#include "src/factory.h"
#include "src/isolate-inl.h"
#include "src/lookup.h"
@@ -43,6 +44,9 @@
if (map->instance_type() != JS_OBJECT_TYPE) return false;
if (map->is_access_check_needed()) return false;
if (map->prototype() != *isolate->initial_object_prototype()) return false;
+ // During bootstrapping, the object_function_prototype_map hasn't been
+ // set up yet.
+ if (isolate->bootstrapper()->IsActive()) return false;
if (JSObject::cast(map->prototype())->map() !=
isolate->native_context()->object_function_prototype_map()) {
return false;
@@ -210,7 +214,11 @@
}
} else {
DCHECK(obj->IsJSProxy());
- UNIMPLEMENTED();
+ // 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;
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index e4fcf4a..8ad8a77 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -8727,10 +8727,12 @@
CompileRun("other.accessible_prop = 42");
CHECK_EQ(42, g_echo_value);
- v8::Handle<Value> value;
- CompileRun("Object.defineProperty(other, 'accessible_prop', {value: -1})");
- value = CompileRun("other.accessible_prop == 42");
- CHECK(value->IsTrue());
+ // [[DefineOwnProperty]] always throws for access-checked objects.
+ CHECK(
+ CompileRun("Object.defineProperty(other, 'accessible_prop', {value: 43})")
+ .IsEmpty());
+ CHECK(CompileRun("other.accessible_prop == 42")->IsTrue());
+ CHECK_EQ(42, g_echo_value); // Make sure we didn't call the setter.
}
diff --git a/test/test262/test262.status b/test/test262/test262.status
index 1093327..01def65 100644
--- a/test/test262/test262.status
+++ b/test/test262/test262.status
@@ -33,30 +33,6 @@
'intl402/11.2.3_b': [FAIL],
'intl402/12.2.3_b': [FAIL],
- # BUG(v8:4267)
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-112': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-113': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-164': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-165': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-166': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-168': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-169': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-170': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-172': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-173': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-175': [FAIL],
- 'built-ins/Object/defineProperties/15.2.3.7-6-a-176': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-116': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-117': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-168': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-169': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-170': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-172': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-173': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-174': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-176': [FAIL],
- 'built-ins/Object/defineProperty/15.2.3.6-4-177': [FAIL],
-
# Unicode canonicalization is not available with i18n turned off.
'built-ins/String/prototype/localeCompare/15.5.4.9_CE': [['no_i18n', SKIP]],
