From 4b548dd15a750ed28f4517280576e06fd4efc4fd Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 1 Jun 2015 00:26:38 -0700 Subject: [PATCH] Also expose DefineOwnProperty In contrast to CreateDataProperty, this will always call out to JS BUG=475206 R=adamk@chromium.org,verwaest@chromium.org LOG=y Review URL: https://codereview.chromium.org/1167473002 Cr-Commit-Position: refs/heads/master@{#28712} --- include/v8.h | 10 ++++ src/api.cc | 44 ++++++++++++++---- src/v8natives.js | 26 +++++------ test/cctest/test-api.cc | 119 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 22 deletions(-) diff --git a/include/v8.h b/include/v8.h index 04622cb..3bd3dfb 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2600,6 +2600,16 @@ class V8_EXPORT Object : public Value { uint32_t index, Local value); + // Implements DefineOwnProperty. + // + // In general, CreateDataProperty will be faster, however, does not allow + // for specifying attributes. + // + // Returns true on success. + V8_WARN_UNUSED_RESULT Maybe DefineOwnProperty( + Local context, Local key, Local value, + PropertyAttribute attributes = None); + // Sets an own property on this object bypassing interceptors and // overriding accessors or read-only properties. // diff --git a/src/api.cc b/src/api.cc index 9ecedf2..349c702 100644 --- a/src/api.cc +++ b/src/api.cc @@ -3558,15 +3558,9 @@ Maybe v8::Object::CreateDataProperty(v8::Local context, 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()); + return DefineOwnProperty( + context, Utils::ToLocal(isolate->factory()->Uint32ToString(index)), + value, v8::None); } } @@ -3584,6 +3578,38 @@ Maybe v8::Object::CreateDataProperty(v8::Local context, } +Maybe v8::Object::DefineOwnProperty(v8::Local context, + v8::Local key, + v8::Local value, + v8::PropertyAttribute attributes) { + PREPARE_FOR_EXECUTION_PRIMITIVE(context, "v8::Object::DefineOwnProperty()", + 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(); + } + + i::Handle desc = isolate->factory()->NewFixedArray(3); + desc->set(0, isolate->heap()->ToBoolean(!(attributes & v8::ReadOnly))); + desc->set(1, isolate->heap()->ToBoolean(!(attributes & v8::DontEnum))); + desc->set(2, isolate->heap()->ToBoolean(!(attributes & v8::DontDelete))); + i::Handle desc_array = + isolate->factory()->NewJSArrayWithElements(desc, i::FAST_ELEMENTS, 3); + i::Handle args[] = {self, key_obj, value_obj, desc_array}; + i::Handle result; + has_pending_exception = + !CallV8HeapFunction(isolate, "$objectDefineOwnProperty", + isolate->factory()->undefined_value(), + arraysize(args), args).ToHandle(&result); + RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool); + return Just(result->BooleanValue()); +} + + Maybe v8::Object::ForceSet(v8::Local context, v8::Local key, v8::Local value, v8::PropertyAttribute attribs) { diff --git a/src/v8natives.js b/src/v8natives.js index 552e426..0cc5078 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -4,7 +4,7 @@ var $functionSourceString; var $globalEval; -var $objectDefineArrayProperty; +var $objectDefineOwnProperty; var $objectGetOwnPropertyDescriptor; var $toCompletePropertyDescriptor; @@ -901,17 +901,6 @@ 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)) { @@ -928,6 +917,17 @@ function DefineOwnProperty(obj, p, desc, should_throw) { } +function DefineOwnPropertyFromAPI(obj, p, value, desc) { + return DefineOwnProperty(obj, p, ToPropertyDescriptor({ + value: value, + writable: desc[0], + enumerable: desc[1], + configurable: desc[2] + }), + false); +} + + // ES6 section 19.1.2.9 function ObjectGetPrototypeOf(obj) { return %_GetPrototype(TO_OBJECT_INLINE(obj)); @@ -1845,7 +1845,7 @@ function GetIterator(obj, method) { $functionSourceString = FunctionSourceString; $globalEval = GlobalEval; -$objectDefineArrayProperty = DefineArrayPropertyFromAPI; +$objectDefineOwnProperty = DefineOwnPropertyFromAPI; $objectGetOwnPropertyDescriptor = ObjectGetOwnPropertyDescriptor; $toCompletePropertyDescriptor = ToCompletePropertyDescriptor; diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 896115d..eb5f9a7 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -13429,6 +13429,125 @@ TEST(CreateDataProperty) { } +TEST(DefineOwnProperty) { + 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->DefineOwnProperty(env.local(), v8_str("foo"), + v8::Integer::New(isolate, 42)).FromJust()); + CHECK(!try_catch.HasCaught()); + CHECK(obj->DefineOwnProperty(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->DefineOwnProperty(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->DefineOwnProperty(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->DefineOwnProperty(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->DefineOwnProperty(env.local(), v8_str("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->DefineOwnProperty(env.local(), v8_str("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()); + } + + { + // Set a non-writable property. + v8::TryCatch try_catch(isolate); + CHECK(obj->DefineOwnProperty(env.local(), v8_str("lala"), + v8::Integer::New(isolate, 42), + v8::ReadOnly).FromJust()); + CHECK(!try_catch.HasCaught()); + v8::Local val = + obj->Get(env.local(), v8_str("lala")).ToLocalChecked(); + CHECK(val->IsNumber()); + CHECK_EQ(42.0, val->NumberValue(env.local()).FromJust()); + CHECK_EQ(v8::ReadOnly, obj->GetPropertyAttributes( + env.local(), v8_str("lala")).FromJust()); + CHECK(!try_catch.HasCaught()); + } + + CompileRun("Object.freeze(a);"); + { + // Can't change non-extensible objects. + v8::TryCatch try_catch(isolate); + CHECK(!obj->DefineOwnProperty(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->DefineOwnProperty(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