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}

diff --git a/include/v8.h b/include/v8.h
index 04622cb..3bd3dfb 100644 (file)
--- a/include/v8.h
+++ b/include/v8.h
@@ -2600,6 +2600,16 @@ class V8_EXPORT Object : public Value {
                                                        uint32_t index,
                                                        Local<Value> 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<bool> DefineOwnProperty(
+      Local<Context> context, Local<Name> key, Local<Value> 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 (file)
--- a/src/api.cc
+++ b/src/api.cc
@@ -3558,15 +3558,9 @@ Maybe<bool> v8::Object::CreateDataProperty(v8::Local<v8::Context> context,
     size_t length =
         i::NumberToSize(isolate, i::Handle<i::JSArray>::cast(self)->length());
     if (index >= length) {
-      i::Handle<i::Object> args[] = {
-          self, isolate->factory()->Uint32ToString(index), value_obj};
-      i::Handle<i::Object> 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<bool> v8::Object::CreateDataProperty(v8::Local<v8::Context> context,
 }
 
 
+Maybe<bool> v8::Object::DefineOwnProperty(v8::Local<v8::Context> context,
+                                          v8::Local<Name> key,
+                                          v8::Local<Value> 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<bool>();
+  }
+
+  i::Handle<i::FixedArray> 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<i::JSArray> desc_array =
+      isolate->factory()->NewJSArrayWithElements(desc, i::FAST_ELEMENTS, 3);
+  i::Handle<i::Object> args[] = {self, key_obj, value_obj, desc_array};
+  i::Handle<i::Object> 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<bool> v8::Object::ForceSet(v8::Local<v8::Context> context,
                                  v8::Local<Value> key, v8::Local<Value> value,
                                  v8::PropertyAttribute attribs) {

diff --git a/src/v8natives.js b/src/v8natives.js
index 552e426..0cc5078 100644 (file)
--- 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 (file)
--- 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<v8::Object> obj =
+      v8::Local<v8::Object>::Cast(env->Global()->Get(v8_str("a")));
+  v8::Local<v8::Array> arr =
+      v8::Local<v8::Array>::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<v8::Value> 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<v8::Value> 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<v8::Value> 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<v8::Value> 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<v8::Value> 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<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(isolate);
+  templ->SetAccessCheckCallbacks(AccessAlwaysBlocked, NULL);
+  v8::Local<v8::Object> 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<Context> calling_context0;
 static v8::Local<Context> calling_context1;
 static v8::Local<Context> calling_context2;