From 2a058de88fa6cfa6d7a76783269f39043f0d8afc Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 27 May 2015 08:03:28 -0700 Subject: [PATCH] Introduce v8::Object::CreateDataProperty Also deprecate ForceSet BUG=chromium:475206 R=adamk@chromium.org,verwaest@chromium.org LOG=y Review URL: https://codereview.chromium.org/1154233003 Cr-Commit-Position: refs/heads/master@{#28660} --- include/v8.h | 23 ++++++++-- src/api.cc | 100 ++++++++++++++++++++++++++++++++++++++++ src/objects.cc | 11 +++++ src/objects.h | 4 +- src/runtime/runtime-object.cc | 8 +--- src/v8natives.js | 13 ++++++ test/cctest/test-api.cc | 103 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 250 insertions(+), 12 deletions(-) diff --git a/include/v8.h b/include/v8.h index 83baf8a..04622cb 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2586,6 +2586,20 @@ class V8_EXPORT Object : public Value { V8_WARN_UNUSED_RESULT Maybe Set(Local context, uint32_t index, Local value); + // Implements CreateDataProperty (ECMA-262, 7.3.4). + // + // Defines a configurable, writable, enumerable property with the given value + // on the object unless the property already exists and is not configurable + // or the object is not extensible. + // + // Returns true on success. + V8_WARN_UNUSED_RESULT Maybe CreateDataProperty(Local context, + Local key, + Local value); + V8_WARN_UNUSED_RESULT Maybe CreateDataProperty(Local context, + uint32_t index, + Local value); + // Sets an own property on this object bypassing interceptors and // overriding accessors or read-only properties. // @@ -2594,12 +2608,13 @@ class V8_EXPORT Object : public Value { // will only be returned if the interceptor doesn't return a value. // // Note also that this only works for named properties. - V8_DEPRECATE_SOON("Use maybe version", + V8_DEPRECATE_SOON("Use CreateDataProperty", bool ForceSet(Handle key, Handle value, PropertyAttribute attribs = None)); - // TODO(dcarney): mark V8_WARN_UNUSED_RESULT - Maybe ForceSet(Local context, Local key, - Local value, PropertyAttribute attribs = None); + V8_DEPRECATE_SOON("Use CreateDataProperty", + Maybe ForceSet(Local context, + Local key, Local value, + PropertyAttribute attribs = None)); V8_DEPRECATE_SOON("Use maybe version", Local Get(Handle key)); V8_WARN_UNUSED_RESULT MaybeLocal Get(Local context, diff --git a/src/api.cc b/src/api.cc index 7e3fa51..c38c054 100644 --- a/src/api.cc +++ b/src/api.cc @@ -3476,6 +3476,106 @@ bool v8::Object::Set(uint32_t index, v8::Handle value) { } +Maybe v8::Object::CreateDataProperty(v8::Local context, + v8::Local key, + v8::Local value) { + PREPARE_FOR_EXECUTION_PRIMITIVE(context, "v8::Object::CreateDataProperty()", + bool); + auto self = Utils::OpenHandle(this); + auto key_obj = Utils::OpenHandle(*key); + auto value_obj = Utils::OpenHandle(*value); + + if (self->IsAccessCheckNeeded() && !isolate->MayAccess(self)) { + isolate->ReportFailedAccessCheck(self); + return Nothing(); + } + + if (!self->IsExtensible()) return Just(false); + + uint32_t index = 0; + if (key_obj->AsArrayIndex(&index)) { + return CreateDataProperty(context, index, value); + } + + // Special case for Array.length. + if (self->IsJSArray() && + key->StrictEquals(Utils::ToLocal(isolate->factory()->length_string()))) { + // Length is not configurable, however, CreateDataProperty always attempts + // to create a configurable property, so we just fail here. + return Just(false); + } + + i::LookupIterator it(self, key_obj, i::LookupIterator::OWN_SKIP_INTERCEPTOR); + if (it.IsFound() && it.state() == i::LookupIterator::ACCESS_CHECK) { + DCHECK(isolate->MayAccess(self)); + it.Next(); + } + + if (it.state() == i::LookupIterator::DATA || + it.state() == i::LookupIterator::ACCESSOR) { + if (!it.IsConfigurable()) return Just(false); + + if (it.state() == i::LookupIterator::ACCESSOR) { + has_pending_exception = i::JSObject::SetOwnPropertyIgnoreAttributes( + self, key_obj, value_obj, NONE, + i::JSObject::DONT_FORCE_FIELD).is_null(); + RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool); + return Just(true); + } + } + + has_pending_exception = i::Runtime::DefineObjectProperty( + self, key_obj, value_obj, NONE).is_null(); + RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool); + return Just(true); +} + + +Maybe v8::Object::CreateDataProperty(v8::Local context, + uint32_t index, + v8::Local value) { + PREPARE_FOR_EXECUTION_PRIMITIVE(context, "v8::Object::CreateDataProperty()", + bool); + auto self = Utils::OpenHandle(this); + auto value_obj = Utils::OpenHandle(*value); + + if (self->IsAccessCheckNeeded() && !isolate->MayAccess(self)) { + isolate->ReportFailedAccessCheck(self); + return Nothing(); + } + + if (!self->IsExtensible()) return Just(false); + + if (self->IsJSArray()) { + size_t length = + i::NumberToSize(isolate, i::Handle::cast(self)->length()); + if (index >= length) { + i::Handle args[] = { + self, isolate->factory()->Uint32ToString(index), value_obj}; + i::Handle result; + has_pending_exception = + !CallV8HeapFunction(isolate, "$objectDefineArrayProperty", + isolate->factory()->undefined_value(), + arraysize(args), args).ToHandle(&result); + RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool); + return Just(result->BooleanValue()); + } + } + + Maybe attributes = + i::JSReceiver::GetOwnElementAttribute(self, index); + if (attributes.IsJust() && attributes.FromJust() & DONT_DELETE) { + return Just(false); + } + + has_pending_exception = i::Runtime::DefineObjectProperty( + self, isolate->factory()->Uint32ToString(index), + value_obj, NONE).is_null(); + RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool); + return Just(true); +} + + Maybe v8::Object::ForceSet(v8::Local context, v8::Local key, v8::Local value, v8::PropertyAttribute attribs) { diff --git a/src/objects.cc b/src/objects.cc index b4dece0..6fe04c7 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -5726,6 +5726,17 @@ MaybeHandle JSObject::PreventExtensions(Handle object) { } +bool JSObject::IsExtensible() { + if (IsJSGlobalProxy()) { + PrototypeIterator iter(GetIsolate(), this); + if (iter.IsAtEnd()) return false; + DCHECK(iter.GetCurrent()->IsJSGlobalObject()); + return JSObject::cast(iter.GetCurrent())->map()->is_extensible(); + } + return map()->is_extensible(); +} + + Handle JSObject::GetNormalizedElementDictionary( Handle object) { DCHECK(!object->elements()->IsDictionary()); diff --git a/src/objects.h b/src/objects.h index c7a9f78..0e78af8 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2155,10 +2155,12 @@ class JSObject: public JSReceiver { // Check whether this object references another object bool ReferencesObject(Object* obj); - // Disalow further properties to be added to the object. + // Disalow further properties to be added to the oject. MUST_USE_RESULT static MaybeHandle PreventExtensions( Handle object); + bool IsExtensible(); + // ES5 Object.seal MUST_USE_RESULT static MaybeHandle Seal(Handle object); diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc index b470c96..dcb1cda 100644 --- a/src/runtime/runtime-object.cc +++ b/src/runtime/runtime-object.cc @@ -457,13 +457,7 @@ RUNTIME_FUNCTION(Runtime_IsExtensible) { SealHandleScope shs(isolate); DCHECK(args.length() == 1); CONVERT_ARG_CHECKED(JSObject, obj, 0); - if (obj->IsJSGlobalProxy()) { - PrototypeIterator iter(isolate, obj); - if (iter.IsAtEnd()) return isolate->heap()->false_value(); - DCHECK(iter.GetCurrent()->IsJSGlobalObject()); - obj = JSObject::cast(iter.GetCurrent()); - } - return isolate->heap()->ToBoolean(obj->map()->is_extensible()); + return isolate->heap()->ToBoolean(obj->IsExtensible()); } diff --git a/src/v8natives.js b/src/v8natives.js index eb25917..39bb2ba 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -4,6 +4,7 @@ var $functionSourceString; var $globalEval; +var $objectDefineArrayProperty; var $objectGetOwnPropertyDescriptor; var $toCompletePropertyDescriptor; @@ -895,6 +896,17 @@ function DefineArrayProperty(obj, p, desc, should_throw) { } +function DefineArrayPropertyFromAPI(obj, p, value) { + return DefineArrayProperty(obj, p, ToPropertyDescriptor({ + value: value, + configurable: true, + enumerable: true, + writable: true + }), + false); +} + + // ES5 section 8.12.9, ES5 section 15.4.5.1 and Harmony proxies. function DefineOwnProperty(obj, p, desc, should_throw) { if (%_IsJSProxy(obj)) { @@ -1825,6 +1837,7 @@ function GetIterator(obj, method) { $functionSourceString = FunctionSourceString; $globalEval = GlobalEval; +$objectDefineArrayProperty = DefineArrayPropertyFromAPI; $objectGetOwnPropertyDescriptor = ObjectGetOwnPropertyDescriptor; $toCompletePropertyDescriptor = ToCompletePropertyDescriptor; diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 2bf3620..e45b29a 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -13305,6 +13305,109 @@ TEST(ForceSetWithInterceptor) { } +TEST(CreateDataProperty) { + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope handle_scope(isolate); + + CompileRun( + "var a = {};" + "var b = [];" + "Object.defineProperty(a, 'foo', {value: 23});" + "Object.defineProperty(a, 'bar', {value: 23, configurable: true});"); + + v8::Local obj = + v8::Local::Cast(env->Global()->Get(v8_str("a"))); + v8::Local arr = + v8::Local::Cast(env->Global()->Get(v8_str("b"))); + { + // Can't change a non-configurable properties. + v8::TryCatch try_catch(isolate); + CHECK(!obj->CreateDataProperty(env.local(), v8_str("foo"), + v8::Integer::New(isolate, 42)).FromJust()); + CHECK(!try_catch.HasCaught()); + CHECK(obj->CreateDataProperty(env.local(), v8_str("bar"), + v8::Integer::New(isolate, 42)).FromJust()); + CHECK(!try_catch.HasCaught()); + v8::Local val = + obj->Get(env.local(), v8_str("bar")).ToLocalChecked(); + CHECK(val->IsNumber()); + CHECK_EQ(42.0, val->NumberValue(env.local()).FromJust()); + } + + { + // Set a regular property. + v8::TryCatch try_catch(isolate); + CHECK(obj->CreateDataProperty(env.local(), v8_str("blub"), + v8::Integer::New(isolate, 42)).FromJust()); + CHECK(!try_catch.HasCaught()); + v8::Local val = + obj->Get(env.local(), v8_str("blub")).ToLocalChecked(); + CHECK(val->IsNumber()); + CHECK_EQ(42.0, val->NumberValue(env.local()).FromJust()); + } + + { + // Set an indexed property. + v8::TryCatch try_catch(isolate); + CHECK(obj->CreateDataProperty(env.local(), v8_str("1"), + v8::Integer::New(isolate, 42)).FromJust()); + CHECK(!try_catch.HasCaught()); + v8::Local val = obj->Get(env.local(), 1).ToLocalChecked(); + CHECK(val->IsNumber()); + CHECK_EQ(42.0, val->NumberValue(env.local()).FromJust()); + } + + { + // Special cases for arrays. + v8::TryCatch try_catch(isolate); + CHECK(!arr->CreateDataProperty(env.local(), v8_str("length"), + v8::Integer::New(isolate, 1)).FromJust()); + CHECK(!try_catch.HasCaught()); + } + { + // Special cases for arrays: index exceeds the array's length + v8::TryCatch try_catch(isolate); + CHECK(arr->CreateDataProperty(env.local(), 1, v8::Integer::New(isolate, 23)) + .FromJust()); + CHECK(!try_catch.HasCaught()); + CHECK_EQ(2, arr->Length()); + v8::Local val = arr->Get(env.local(), 1).ToLocalChecked(); + CHECK(val->IsNumber()); + CHECK_EQ(23.0, val->NumberValue(env.local()).FromJust()); + + // Set an existing entry. + CHECK(arr->CreateDataProperty(env.local(), 0, v8::Integer::New(isolate, 42)) + .FromJust()); + CHECK(!try_catch.HasCaught()); + val = arr->Get(env.local(), 0).ToLocalChecked(); + CHECK(val->IsNumber()); + CHECK_EQ(42.0, val->NumberValue(env.local()).FromJust()); + } + + CompileRun("Object.freeze(a);"); + { + // Can't change non-extensible objects. + v8::TryCatch try_catch(isolate); + CHECK(!obj->CreateDataProperty(env.local(), v8_str("baz"), + v8::Integer::New(isolate, 42)).FromJust()); + CHECK(!try_catch.HasCaught()); + } + + v8::Local templ = v8::ObjectTemplate::New(isolate); + templ->SetAccessCheckCallbacks(AccessAlwaysBlocked, NULL); + v8::Local access_checked = + templ->NewInstance(env.local()).ToLocalChecked(); + { + v8::TryCatch try_catch(isolate); + CHECK(access_checked->CreateDataProperty(env.local(), v8_str("foo"), + v8::Integer::New(isolate, 42)) + .IsNothing()); + CHECK(try_catch.HasCaught()); + } +} + + static v8::Local calling_context0; static v8::Local calling_context1; static v8::Local calling_context2; -- 2.7.4