Reviewers: Michael Starzinger,
Description:
Handlify ForceSetObjectProperty
Note that I've left the layering as is to make the diffs clear. Is it worth
moving ForceSetObjectProperty to objects.cc? This code is clearly
implementing
part of the DefineOrRedefine steps from the spec, but it's still odd that it
lives in Runtime. Note that handles.cc exposes a ForceSetProperty which just
performs a CALL_HEAP_FUNCTION on the Runtime::ForceSetObjectProperty --
which is
exposed to the api as v8::Object::ForceSet
BUG=
Please review this at https://codereview.chromium.org/61883002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+42, -43 lines):
M src/handles.cc
M src/objects.h
M src/objects.cc
M src/runtime.h
M src/runtime.cc
Index: src/handles.cc
diff --git a/src/handles.cc b/src/handles.cc
index
f3928ebf779f7d98c404a032aa43c4bfdda6591e..42e53c68f4877721c913b8a9058a956fa9d0e0da
100644
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -178,12 +178,8 @@ Handle<Object> ForceSetProperty(Handle<JSObject>
object,
Handle<Object> key,
Handle<Object> value,
PropertyAttributes attributes) {
- Isolate* isolate = object->GetIsolate();
- CALL_HEAP_FUNCTION(
- isolate,
- Runtime::ForceSetObjectProperty(
- isolate, object, key, value, attributes),
- Object);
+ return Runtime::ForceSetObjectProperty(object->GetIsolate(), object, key,
+ value, attributes);
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
d5f0443df5cdfed65278c5cb90c8a0f77f0550b1..22d61fe125bbf99b96d0a2a2f0d34f1fb321f093
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -12492,6 +12492,7 @@ Handle<Object>
JSObject::SetElement(Handle<JSObject> object,
Handle<Object> value,
PropertyAttributes attr,
StrictModeFlag strict_mode,
+ bool check_prototype,
SetPropertyMode set_mode) {
if (object->HasExternalArrayElements()) {
if (!value->IsNumber() && !value->IsUndefined()) {
@@ -12504,7 +12505,8 @@ Handle<Object>
JSObject::SetElement(Handle<JSObject> object,
}
CALL_HEAP_FUNCTION(
object->GetIsolate(),
- object->SetElement(index, *value, attr, strict_mode, true, set_mode),
+ object->SetElement(index, *value, attr, strict_mode, check_prototype,
+ set_mode),
Object);
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
aa91ac15e0cef397453e7289a14d427d223f901c..91bde5c9de85e61624e8cd02ee7bd3b10d4e5c60
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -2376,6 +2376,7 @@ class JSObject: public JSReceiver {
Handle<Object> value,
PropertyAttributes attr,
StrictModeFlag strict_mode,
+ bool check_prototype = true,
SetPropertyMode set_mode = SET_PROPERTY);
// A Failure object is returned if GC is needed.
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
dd36a53929aa18c5cd0207d3db7561e1196cfd41..42313e5479e11ad72c5f0d92f82329a8a071a82e
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5031,12 +5031,12 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_DefineOrRedefineDataProperty) {
RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) ==
0);
PropertyAttributes attr = static_cast<PropertyAttributes>(unchecked);
- LookupResult result(isolate);
- js_object->LocalLookupRealNamedProperty(*name, &result);
+ LookupResult lookup(isolate);
+ js_object->LocalLookupRealNamedProperty(*name, &lookup);
// Special case for callback properties.
- if (result.IsPropertyCallbacks()) {
- Object* callback = result.GetCallbackObject();
+ if (lookup.IsPropertyCallbacks()) {
+ Handle<Object> callback(lookup.GetCallbackObject(), isolate);
// To be compatible with Safari we do not change the value on API
objects
// in Object.defineProperty(). Firefox disagrees here, and actually
changes
// the value.
@@ -5047,13 +5047,13 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_DefineOrRedefineDataProperty) {
// setter to update the value instead.
// TODO(mstarzinger): So far this only works if property attributes
don't
// change, this should be fixed once we cleanup the underlying code.
- if (callback->IsForeign() && result.GetAttributes() == attr) {
+ if (callback->IsForeign() && lookup.GetAttributes() == attr) {
Handle<Object> result_object =
JSObject::SetPropertyWithCallback(js_object,
- handle(callback, isolate),
+ callback,
name,
obj_value,
- handle(result.holder()),
+ handle(lookup.holder()),
kStrictMode);
RETURN_IF_EMPTY_HANDLE(isolate, result_object);
return *result_object;
@@ -5066,8 +5066,8 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_DefineOrRedefineDataProperty) {
// map. The current version of SetObjectProperty does not handle
attributes
// correctly in the case where a property is a field and is reset with
// new attributes.
- if (result.IsFound() &&
- (attr != result.GetAttributes() || result.IsPropertyCallbacks())) {
+ if (lookup.IsFound() &&
+ (attr != lookup.GetAttributes() || lookup.IsPropertyCallbacks())) {
// New attributes - normalize to avoid writing to instance descriptor
if (js_object->IsJSGlobalProxy()) {
// Since the result is a property, the prototype will exist so
@@ -5083,11 +5083,12 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_DefineOrRedefineDataProperty) {
return *result;
}
- return Runtime::ForceSetObjectProperty(isolate,
- js_object,
- name,
- obj_value,
- attr);
+ Handle<Object> result = Runtime::ForceSetObjectProperty(isolate,
js_object,
+ name,
+ obj_value,
+ attr);
+ RETURN_IF_EMPTY_HANDLE(isolate, result);
+ return *result;
}
@@ -5241,11 +5242,11 @@ MaybeObject* Runtime::SetObjectProperty(Isolate*
isolate,
}
-MaybeObject* Runtime::ForceSetObjectProperty(Isolate* isolate,
- Handle<JSObject> js_object,
- Handle<Object> key,
- Handle<Object> value,
- PropertyAttributes attr) {
+Handle<Object> Runtime::ForceSetObjectProperty(Isolate* isolate,
+ Handle<JSObject> js_object,
+ Handle<Object> key,
+ Handle<Object> value,
+ PropertyAttributes attr) {
HandleScope scope(isolate);
// Check if the given key is an array index.
@@ -5259,24 +5260,24 @@ MaybeObject*
Runtime::ForceSetObjectProperty(Isolate* isolate,
// string does nothing with the assignment then we can ignore such
// assignments.
if (js_object->IsStringObjectWithCharacterAt(index)) {
- return *value;
+ return value;
}
- return js_object->SetElement(
- index, *value, attr, kNonStrictMode, false, DEFINE_PROPERTY);
+ return JSObject::SetElement(js_object, index, value, attr,
kNonStrictMode,
+ false,
+ DEFINE_PROPERTY);
}
if (key->IsName()) {
Handle<Name> name = Handle<Name>::cast(key);
if (name->AsArrayIndex(&index)) {
- return js_object->SetElement(
- index, *value, attr, kNonStrictMode, false, DEFINE_PROPERTY);
+ return JSObject::SetElement(js_object, index, value, attr,
kNonStrictMode,
+ false,
+ DEFINE_PROPERTY);
} else {
if (name->IsString()) Handle<String>::cast(name)->TryFlatten();
- Handle<Object> result = JSObject::SetLocalPropertyIgnoreAttributes(
- js_object, name, value, attr);
- RETURN_IF_EMPTY_HANDLE(isolate, result);
- return *result;
+ return JSObject::SetLocalPropertyIgnoreAttributes(js_object, name,
+ value, attr);
}
}
@@ -5284,17 +5285,16 @@ MaybeObject*
Runtime::ForceSetObjectProperty(Isolate* isolate,
bool has_pending_exception = false;
Handle<Object> converted =
Execution::ToString(isolate, key, &has_pending_exception);
- if (has_pending_exception) return Failure::Exception();
+ if (has_pending_exception) return Handle<Object>(); // exception
Handle<String> name = Handle<String>::cast(converted);
if (name->AsArrayIndex(&index)) {
- return js_object->SetElement(
- index, *value, attr, kNonStrictMode, false, DEFINE_PROPERTY);
+ return JSObject::SetElement(js_object, index, value, attr,
kNonStrictMode,
+ false,
+ DEFINE_PROPERTY);
} else {
- Handle<Object> result = JSObject::SetLocalPropertyIgnoreAttributes(
- js_object, name, value, attr);
- RETURN_IF_EMPTY_HANDLE(isolate, result);
- return *result;
+ return JSObject::SetLocalPropertyIgnoreAttributes(js_object, name,
value,
+ attr);
}
}
Index: src/runtime.h
diff --git a/src/runtime.h b/src/runtime.h
index
55276f803969eadf76dde57bd9c4a4d92df32f12..07aa0b4c6b84045585107bbd240a740d528a0623
100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -792,7 +792,7 @@ class Runtime : public AllStatic {
PropertyAttributes attr,
StrictModeFlag strict_mode);
- MUST_USE_RESULT static MaybeObject* ForceSetObjectProperty(
+ static Handle<Object> ForceSetObjectProperty(
Isolate* isolate,
Handle<JSObject> object,
Handle<Object> key,
--
--
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/groups/opt_out.
