Revision: 22095
Author: [email protected]
Date: Mon Jun 30 13:48:57 2014 UTC
Log: Wrap InitializeProperty around SetOwnPropertyIgnoreAttributes and
switch over uses
This is a step in the direction of disentangling all uses of
SetOwnPropertyIgnoreAttributes so we can provide a more specific
implementation for those usecases, and reduce the capabilities of those
clients, avoiding subtle bugs.
InitializeProperty only supports adding properties to extensible objects
that do not contain the property yet. JSGlobalProxies cannot have
properties themselves, so are not supported either.
BUG=
[email protected]
Review URL: https://codereview.chromium.org/352813002
http://code.google.com/p/v8/source/detail?r=22095
Modified:
/branches/bleeding_edge/src/bootstrapper.cc
/branches/bleeding_edge/src/factory.cc
/branches/bleeding_edge/src/isolate.cc
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Mon Jun 30 13:25:46 2014 UTC
+++ /branches/bleeding_edge/src/bootstrapper.cc Mon Jun 30 13:48:57 2014 UTC
@@ -379,8 +379,7 @@
} else {
attributes = DONT_ENUM;
}
- JSObject::SetOwnPropertyIgnoreAttributes(
- target, internalized_name, function, attributes).Check();
+ JSObject::AddProperty(target, internalized_name, function, attributes);
if (target->IsJSGlobalObject()) {
function->shared()->set_instance_class_name(*internalized_name);
}
@@ -889,9 +888,8 @@
Heap* heap = isolate->heap();
Handle<String> object_name = factory->Object_string();
- JSObject::SetOwnPropertyIgnoreAttributes(
- inner_global, object_name,
- isolate->object_function(), DONT_ENUM).Check();
+ JSObject::AddProperty(
+ inner_global, object_name, isolate->object_function(), DONT_ENUM);
Handle<JSObject> global(native_context()->global_object());
@@ -1090,8 +1088,7 @@
cons->SetInstanceClassName(*name);
Handle<JSObject> json_object = factory->NewJSObject(cons, TENURED);
ASSERT(json_object->IsJSObject());
- JSObject::SetOwnPropertyIgnoreAttributes(
- global, name, json_object, DONT_ENUM).Check();
+ JSObject::AddProperty(global, name, json_object, DONT_ENUM);
native_context()->set_json_object(*json_object);
}
@@ -1156,14 +1153,14 @@
native_context()->set_sloppy_arguments_boilerplate(*result);
// Note: length must be added as the first property and
// callee must be added as the second property.
- JSObject::SetOwnPropertyIgnoreAttributes(
+ JSObject::AddProperty(
result, factory->length_string(),
factory->undefined_value(), DONT_ENUM,
- Object::FORCE_TAGGED, FORCE_FIELD).Check();
- JSObject::SetOwnPropertyIgnoreAttributes(
+ Object::FORCE_TAGGED, FORCE_FIELD);
+ JSObject::AddProperty(
result, factory->callee_string(),
factory->undefined_value(), DONT_ENUM,
- Object::FORCE_TAGGED, FORCE_FIELD).Check();
+ Object::FORCE_TAGGED, FORCE_FIELD);
#ifdef DEBUG
LookupResult lookup(isolate);
@@ -1262,11 +1259,6 @@
Handle<JSObject> result = factory->NewJSObjectFromMap(map);
native_context()->set_strict_arguments_boilerplate(*result);
- // Add length property only for strict mode boilerplate.
- JSObject::SetOwnPropertyIgnoreAttributes(
- result, factory->length_string(),
- factory->undefined_value(), DONT_ENUM).Check();
-
#ifdef DEBUG
LookupResult lookup(isolate);
result->LookupOwn(factory->length_string(), &lookup);
@@ -1274,6 +1266,10 @@
ASSERT(lookup.GetFieldIndex().property_index() ==
Heap::kArgumentsLengthIndex);
+ Handle<Object> length_value = Object::GetProperty(
+ result, factory->length_string()).ToHandleChecked();
+ ASSERT_EQ(heap->undefined_value(), *length_value);
+
ASSERT(result->map()->inobject_properties() >
Heap::kArgumentsLengthIndex);
// Check the state of the object.
@@ -1736,12 +1732,10 @@
Handle<String> global_string =
factory()->InternalizeOneByteString(STATIC_ASCII_VECTOR("global"));
Handle<Object> global_obj(native_context()->global_object(), isolate());
- JSObject::SetOwnPropertyIgnoreAttributes(
- builtins, global_string, global_obj, attributes).Check();
+ JSObject::AddProperty(builtins, global_string, global_obj, attributes);
Handle<String> builtins_string =
factory()->InternalizeOneByteString(STATIC_ASCII_VECTOR("builtins"));
- JSObject::SetOwnPropertyIgnoreAttributes(
- builtins, builtins_string, builtins, attributes).Check();
+ JSObject::AddProperty(builtins, builtins_string, builtins, attributes);
// Set up the reference from the global object to the builtins object.
JSGlobalObject::cast(native_context()->global_object())->
@@ -2454,16 +2448,14 @@
ASSERT(!descs->GetDetails(i).representation().IsDouble());
Handle<Object> value =
Handle<Object>(from->RawFastPropertyAt(index),
isolate());
- JSObject::SetOwnPropertyIgnoreAttributes(
- to, key, value, details.attributes()).Check();
+ JSObject::AddProperty(to, key, value, details.attributes());
break;
}
case CONSTANT: {
HandleScope inner(isolate());
Handle<Name> key = Handle<Name>(descs->GetKey(i));
Handle<Object> constant(descs->GetConstant(i), isolate());
- JSObject::SetOwnPropertyIgnoreAttributes(
- to, key, constant, details.attributes()).Check();
+ JSObject::AddProperty(to, key, constant, details.attributes());
break;
}
case CALLBACKS: {
@@ -2513,8 +2505,7 @@
isolate());
}
PropertyDetails details = properties->DetailsAt(i);
- JSObject::SetOwnPropertyIgnoreAttributes(
- to, key, value, details.attributes()).Check();
+ JSObject::AddProperty(to, key, value, details.attributes());
}
}
}
=======================================
--- /branches/bleeding_edge/src/factory.cc Mon Jun 30 10:19:31 2014 UTC
+++ /branches/bleeding_edge/src/factory.cc Mon Jun 30 13:48:57 2014 UTC
@@ -1333,10 +1333,7 @@
Handle<JSObject> prototype = NewJSObjectFromMap(new_map);
if (!function->shared()->is_generator()) {
- JSObject::SetOwnPropertyIgnoreAttributes(prototype,
- constructor_string(),
- function,
- DONT_ENUM).Assert();
+ JSObject::AddProperty(prototype, constructor_string(), function,
DONT_ENUM);
}
return prototype;
@@ -2159,11 +2156,19 @@
return result;
}
- JSObject::SetOwnPropertyIgnoreAttributes(
- handle(JSObject::cast(result->prototype())),
- constructor_string(),
- result,
- DONT_ENUM).Assert();
+ if (prototype->IsTheHole()) {
+#ifdef DEBUG
+ LookupIterator it(handle(JSObject::cast(result->prototype())),
+ constructor_string(),
+ LookupIterator::CHECK_OWN_REAL);
+ MaybeHandle<Object> maybe_prop = Object::GetProperty(&it);
+ ASSERT(it.IsFound());
+ ASSERT(maybe_prop.ToHandleChecked().is_identical_to(result));
+#endif
+ } else {
+ JSObject::AddProperty(handle(JSObject::cast(result->prototype())),
+ constructor_string(), result, DONT_ENUM);
+ }
// Down from here is only valid for API functions that can be used as a
// constructor (don't set the "remove prototype" flag).
=======================================
--- /branches/bleeding_edge/src/isolate.cc Mon Jun 30 13:25:46 2014 UTC
+++ /branches/bleeding_edge/src/isolate.cc Mon Jun 30 13:48:57 2014 UTC
@@ -494,31 +494,29 @@
// tag.
column_offset += script->column_offset()->value();
}
- JSObject::SetOwnPropertyIgnoreAttributes(
+ JSObject::AddProperty(
stack_frame, column_key,
- Handle<Smi>(Smi::FromInt(column_offset + 1), this),
NONE).Check();
+ handle(Smi::FromInt(column_offset + 1), this), NONE);
}
- JSObject::SetOwnPropertyIgnoreAttributes(
+ JSObject::AddProperty(
stack_frame, line_key,
- Handle<Smi>(Smi::FromInt(line_number + 1), this),
NONE).Check();
+ handle(Smi::FromInt(line_number + 1), this), NONE);
}
if (options & StackTrace::kScriptId) {
- Handle<Smi> script_id(script->id(), this);
- JSObject::SetOwnPropertyIgnoreAttributes(
- stack_frame, script_id_key, script_id, NONE).Check();
+ JSObject::AddProperty(
+ stack_frame, script_id_key, handle(script->id(), this), NONE);
}
if (options & StackTrace::kScriptName) {
- Handle<Object> script_name(script->name(), this);
- JSObject::SetOwnPropertyIgnoreAttributes(
- stack_frame, script_name_key, script_name, NONE).Check();
+ JSObject::AddProperty(
+ stack_frame, script_name_key, handle(script->name(), this),
NONE);
}
if (options & StackTrace::kScriptNameOrSourceURL) {
Handle<Object> result = Script::GetNameOrSourceURL(script);
- JSObject::SetOwnPropertyIgnoreAttributes(
- stack_frame, script_name_or_source_url_key, result,
NONE).Check();
+ JSObject::AddProperty(
+ stack_frame, script_name_or_source_url_key, result, NONE);
}
if (options & StackTrace::kFunctionName) {
@@ -526,23 +524,21 @@
if (!fun_name->BooleanValue()) {
fun_name = Handle<Object>(fun->shared()->inferred_name(), this);
}
- JSObject::SetOwnPropertyIgnoreAttributes(
- stack_frame, function_key, fun_name, NONE).Check();
+ JSObject::AddProperty(stack_frame, function_key, fun_name, NONE);
}
if (options & StackTrace::kIsEval) {
Handle<Object> is_eval =
script->compilation_type() == Script::COMPILATION_TYPE_EVAL ?
factory()->true_value() : factory()->false_value();
- JSObject::SetOwnPropertyIgnoreAttributes(
- stack_frame, eval_key, is_eval, NONE).Check();
+ JSObject::AddProperty(stack_frame, eval_key, is_eval, NONE);
}
if (options & StackTrace::kIsConstructor) {
Handle<Object> is_constructor = (frames[i].is_constructor()) ?
factory()->true_value() : factory()->false_value();
- JSObject::SetOwnPropertyIgnoreAttributes(
- stack_frame, constructor_key, is_constructor, NONE).Check();
+ JSObject::AddProperty(
+ stack_frame, constructor_key, is_constructor, NONE);
}
FixedArray::cast(stack_trace->elements())->set(frames_seen,
*stack_frame);
=======================================
--- /branches/bleeding_edge/src/objects.cc Mon Jun 30 13:25:46 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc Mon Jun 30 13:48:57 2014 UTC
@@ -1873,7 +1873,7 @@
}
-MaybeHandle<Object> JSObject::AddProperty(
+MaybeHandle<Object> JSObject::AddPropertyInternal(
Handle<JSObject> object,
Handle<Name> name,
Handle<Object> value,
@@ -3926,10 +3926,10 @@
PropertyDetails details = descriptors->GetDetails(descriptor);
if (details.type() == CALLBACKS || attributes != details.attributes()) {
- // AddProperty will either normalize the object, or create a new fast
copy
- // of the map. If we get a fast copy of the map, all field
representations
- // will be tagged since the transition is omitted.
- return JSObject::AddProperty(
+ // AddPropertyInternal will either normalize the object, or create a
new
+ // fast copy of the map. If we get a fast copy of the map, all field
+ // representations will be tagged since the transition is omitted.
+ return JSObject::AddPropertyInternal(
object, name, value, attributes, SLOPPY,
JSReceiver::CERTAINLY_NOT_STORE_FROM_KEYED,
JSReceiver::OMIT_EXTENSIBILITY_CHECK,
@@ -4093,7 +4093,7 @@
if (!lookup->IsFound()) {
// Neither properties nor transitions found.
- return AddProperty(
+ return AddPropertyInternal(
object, name, value, attributes, strict_mode, store_mode);
}
@@ -4171,6 +4171,28 @@
return result;
}
+
+
+void JSObject::AddProperty(
+ Handle<JSObject> object,
+ Handle<Name> name,
+ Handle<Object> value,
+ PropertyAttributes attributes,
+ ValueType value_type,
+ StoreMode store_mode) {
+#ifdef DEBUG
+ uint32_t index;
+ ASSERT(!object->IsJSProxy());
+ ASSERT(!name->AsArrayIndex(&index));
+ LookupIterator it(object, name, LookupIterator::CHECK_OWN_REAL);
+ GetPropertyAttributes(&it);
+ ASSERT(!it.IsFound());
+ ASSERT(object->map()->is_extensible());
+#endif
+ SetOwnPropertyIgnoreAttributes(
+ object, name, value, attributes, value_type, store_mode,
+ OMIT_EXTENSIBILITY_CHECK).Check();
+}
// Set a real own property, even if it is READ_ONLY. If the property is
not
@@ -4228,7 +4250,7 @@
TransitionFlag flag = lookup.IsFound()
? OMIT_TRANSITION : INSERT_TRANSITION;
// Neither properties nor transitions found.
- return AddProperty(object, name, value, attributes, SLOPPY,
+ return AddPropertyInternal(object, name, value, attributes, SLOPPY,
store_from_keyed, extensibility_check, value_type, mode, flag);
}
@@ -5154,13 +5176,8 @@
}
JSObject::SetOwnPropertyIgnoreAttributes(
- object,
- isolate->factory()->hidden_string(),
- hashtable,
- DONT_ENUM,
- OPTIMAL_REPRESENTATION,
- ALLOW_AS_CONSTANT,
- OMIT_EXTENSIBILITY_CHECK).Assert();
+ object, isolate->factory()->hidden_string(),
+ hashtable, DONT_ENUM).Assert();
return hashtable;
}
=======================================
--- /branches/bleeding_edge/src/objects.h Mon Jun 30 13:25:46 2014 UTC
+++ /branches/bleeding_edge/src/objects.h Mon Jun 30 13:48:57 2014 UTC
@@ -2153,6 +2153,13 @@
StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED,
ExecutableAccessorInfoHandling handling = DEFAULT_HANDLING);
+ static void AddProperty(Handle<JSObject> object,
+ Handle<Name> key,
+ Handle<Object> value,
+ PropertyAttributes attributes,
+ ValueType value_type = OPTIMAL_REPRESENTATION,
+ StoreMode mode = ALLOW_AS_CONSTANT);
+
// Extend the receiver with a single fast property appeared first in the
// passed map. This also extends the property backing store if necessary.
static void AllocateStorageForMap(Handle<JSObject> object, Handle<Map>
map);
@@ -2750,7 +2757,7 @@
StrictMode strict_mode);
// Add a property to an object.
- MUST_USE_RESULT static MaybeHandle<Object> AddProperty(
+ MUST_USE_RESULT static MaybeHandle<Object> AddPropertyInternal(
Handle<JSObject> object,
Handle<Name> name,
Handle<Object> value,
=======================================
--- /branches/bleeding_edge/src/runtime.cc Mon Jun 30 13:25:46 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc Mon Jun 30 13:48:57 2014 UTC
@@ -303,8 +303,7 @@
const char* str = DoubleToCString(num, buffer);
Handle<String> name =
isolate->factory()->NewStringFromAsciiChecked(str);
maybe_result = JSObject::SetOwnPropertyIgnoreAttributes(
- boilerplate, name, value, NONE,
- value_type, mode);
+ boilerplate, name, value, NONE, value_type, mode);
}
// If setting the property on the boilerplate throws an
// exception, the exception is converted to an empty handle in
@@ -2396,10 +2395,7 @@
if (!lookup.IsFound()) {
HandleScope handle_scope(isolate);
Handle<GlobalObject> global(isolate->context()->global_object());
- RETURN_FAILURE_ON_EXCEPTION(
- isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(global, name, value,
- attributes));
+ JSObject::AddProperty(global, name, value, attributes);
return *value;
}
@@ -8133,8 +8129,8 @@
static_cast<PropertyAttributes>(DONT_DELETE | DONT_ENUM | READ_ONLY);
RETURN_FAILURE_ON_EXCEPTION(
isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(bound_function,
length_string,
- new_length, attr));
+ JSObject::SetOwnPropertyIgnoreAttributes(
+ bound_function, length_string, new_length, attr));
return *bound_function;
}
@@ -13809,18 +13805,10 @@
}
Handle<JSObject> result =
factory->NewJSObject(isolate->object_function());
- RETURN_FAILURE_ON_EXCEPTION(isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(
- result,
- maximized,
- factory->NewStringFromAsciiChecked(base_max_locale),
- NONE));
- RETURN_FAILURE_ON_EXCEPTION(isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(
- result,
- base,
- factory->NewStringFromAsciiChecked(base_locale),
- NONE));
+ Handle<String> value =
factory->NewStringFromAsciiChecked(base_max_locale);
+ JSObject::AddProperty(result, maximized, value, NONE);
+ value = factory->NewStringFromAsciiChecked(base_locale);
+ JSObject::AddProperty(result, base, value, NONE);
output->set(i, *result);
}
@@ -13937,12 +13925,10 @@
local_object->SetInternalField(0, reinterpret_cast<Smi*>(date_format));
- RETURN_FAILURE_ON_EXCEPTION(isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(
- local_object,
- isolate->factory()->NewStringFromStaticAscii("dateFormat"),
- isolate->factory()->NewStringFromStaticAscii("valid"),
- NONE));
+ Factory* factory = isolate->factory();
+ Handle<String> key = factory->NewStringFromStaticAscii("dateFormat");
+ Handle<String> value = factory->NewStringFromStaticAscii("valid");
+ JSObject::AddProperty(local_object, key, value, NONE);
// Make object handle weak so we can delete the data format once GC
kicks in.
Handle<Object> wrapper =
isolate->global_handles()->Create(*local_object);
@@ -14036,12 +14022,10 @@
local_object->SetInternalField(0, reinterpret_cast<Smi*>(number_format));
- RETURN_FAILURE_ON_EXCEPTION(isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(
- local_object,
- isolate->factory()->NewStringFromStaticAscii("numberFormat"),
- isolate->factory()->NewStringFromStaticAscii("valid"),
- NONE));
+ Factory* factory = isolate->factory();
+ Handle<String> key = factory->NewStringFromStaticAscii("numberFormat");
+ Handle<String> value = factory->NewStringFromStaticAscii("valid");
+ JSObject::AddProperty(local_object, key, value, NONE);
Handle<Object> wrapper =
isolate->global_handles()->Create(*local_object);
GlobalHandles::MakeWeak(wrapper.location(),
@@ -14144,12 +14128,10 @@
local_object->SetInternalField(0, reinterpret_cast<Smi*>(collator));
- RETURN_FAILURE_ON_EXCEPTION(isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(
- local_object,
- isolate->factory()->NewStringFromStaticAscii("collator"),
- isolate->factory()->NewStringFromStaticAscii("valid"),
- NONE));
+ Factory* factory = isolate->factory();
+ Handle<String> key = factory->NewStringFromStaticAscii("collator");
+ Handle<String> value = factory->NewStringFromStaticAscii("valid");
+ JSObject::AddProperty(local_object, key, value, NONE);
Handle<Object> wrapper =
isolate->global_handles()->Create(*local_object);
GlobalHandles::MakeWeak(wrapper.location(),
@@ -14250,12 +14232,10 @@
// Make sure that the pointer to adopted text is NULL.
local_object->SetInternalField(1, reinterpret_cast<Smi*>(NULL));
- RETURN_FAILURE_ON_EXCEPTION(isolate,
- JSObject::SetOwnPropertyIgnoreAttributes(
- local_object,
- isolate->factory()->NewStringFromStaticAscii("breakIterator"),
- isolate->factory()->NewStringFromStaticAscii("valid"),
- NONE));
+ Factory* factory = isolate->factory();
+ Handle<String> key = factory->NewStringFromStaticAscii("breakIterator");
+ Handle<String> value = factory->NewStringFromStaticAscii("valid");
+ JSObject::AddProperty(local_object, key, value, NONE);
// Make object handle weak so we can delete the break iterator once GC
kicks
// in.
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
