From 1a74e279943bf525b35b1b1228a5558285a375a6 Mon Sep 17 00:00:00 2001 From: "rafaelw@chromium.org" Date: Wed, 6 Nov 2013 16:32:47 +0000 Subject: [PATCH] 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= R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/61883002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17529 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/handles.cc | 8 ++----- src/objects.cc | 4 +++- src/objects.h | 1 + src/runtime.cc | 72 ++++++++++++++++++++++++++++------------------------------ src/runtime.h | 2 +- 5 files changed, 42 insertions(+), 45 deletions(-) diff --git a/src/handles.cc b/src/handles.cc index f3928eb..42e53c6 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -178,12 +178,8 @@ Handle ForceSetProperty(Handle object, Handle key, Handle 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); } diff --git a/src/objects.cc b/src/objects.cc index 6618b83..6ed25f6 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -12545,6 +12545,7 @@ Handle JSObject::SetElement(Handle object, Handle value, PropertyAttributes attr, StrictModeFlag strict_mode, + bool check_prototype, SetPropertyMode set_mode) { if (object->HasExternalArrayElements()) { if (!value->IsNumber() && !value->IsUndefined()) { @@ -12557,7 +12558,8 @@ Handle JSObject::SetElement(Handle 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); } diff --git a/src/objects.h b/src/objects.h index f71aec1..4639407 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2376,6 +2376,7 @@ class JSObject: public JSReceiver { Handle value, PropertyAttributes attr, StrictModeFlag strict_mode, + bool check_prototype = true, SetPropertyMode set_mode = SET_PROPERTY); // A Failure object is returned if GC is needed. diff --git a/src/runtime.cc b/src/runtime.cc index 107297c..8047995 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(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 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 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 result = Runtime::ForceSetObjectProperty(isolate, js_object, + name, + obj_value, + attr); + RETURN_IF_EMPTY_HANDLE(isolate, result); + return *result; } @@ -5241,13 +5242,11 @@ MaybeObject* Runtime::SetObjectProperty(Isolate* isolate, } -MaybeObject* Runtime::ForceSetObjectProperty(Isolate* isolate, - Handle js_object, - Handle key, - Handle value, - PropertyAttributes attr) { - HandleScope scope(isolate); - +Handle Runtime::ForceSetObjectProperty(Isolate* isolate, + Handle js_object, + Handle key, + Handle value, + PropertyAttributes attr) { // Check if the given key is an array index. uint32_t index; if (key->ToArrayIndex(&index)) { @@ -5259,24 +5258,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 = Handle::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::cast(name)->TryFlatten(); - Handle 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 +5283,16 @@ MaybeObject* Runtime::ForceSetObjectProperty(Isolate* isolate, bool has_pending_exception = false; Handle converted = Execution::ToString(isolate, key, &has_pending_exception); - if (has_pending_exception) return Failure::Exception(); + if (has_pending_exception) return Handle(); // exception Handle name = Handle::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 result = JSObject::SetLocalPropertyIgnoreAttributes( - js_object, name, value, attr); - RETURN_IF_EMPTY_HANDLE(isolate, result); - return *result; + return JSObject::SetLocalPropertyIgnoreAttributes(js_object, name, value, + attr); } } diff --git a/src/runtime.h b/src/runtime.h index 55276f8..07aa0b4 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 ForceSetObjectProperty( Isolate* isolate, Handle object, Handle key, -- 2.7.4