From 6b712555dd250284fdc3a0ac7f84d27097183f11 Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Wed, 28 May 2014 09:29:27 +0000 Subject: [PATCH] Cleanup GetPropertyWithCallback / SetPropertyWithCallback API BUG= R=ishell@chromium.org Review URL: https://codereview.chromium.org/305513004 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21555 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects.cc | 245 ++++++++++++++++++++++++++++----------------------------- src/objects.h | 38 ++++----- src/runtime.cc | 9 +-- 3 files changed, 139 insertions(+), 153 deletions(-) diff --git a/src/objects.cc b/src/objects.cc index 9fe82ed..3d53f95 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -361,10 +361,24 @@ Handle JSObject::EnsureWritableFastElements( } -MaybeHandle JSObject::GetPropertyWithCallback(Handle object, - Handle receiver, - Handle structure, - Handle name) { +MaybeHandle JSProxy::GetPropertyWithHandler(Handle proxy, + Handle receiver, + Handle name) { + Isolate* isolate = proxy->GetIsolate(); + + // TODO(rossberg): adjust once there is a story for symbols vs proxies. + if (name->IsSymbol()) return isolate->factory()->undefined_value(); + + Handle args[] = { receiver, name }; + return CallTrap( + proxy, "get", isolate->derived_get_trap(), ARRAY_SIZE(args), args); +} + + +MaybeHandle Object::GetPropertyWithCallback(Handle receiver, + Handle name, + Handle holder, + Handle structure) { Isolate* isolate = name->GetIsolate(); ASSERT(!structure->IsForeign()); // api style callbacks. @@ -395,8 +409,8 @@ MaybeHandle JSObject::GetPropertyWithCallback(Handle object, if (call_fun == NULL) return isolate->factory()->undefined_value(); Handle key = Handle::cast(name); - LOG(isolate, ApiNamedPropertyAccess("load", *object, *name)); - PropertyCallbackArguments args(isolate, data->data(), *receiver, *object); + LOG(isolate, ApiNamedPropertyAccess("load", *holder, *name)); + PropertyCallbackArguments args(isolate, data->data(), *receiver, *holder); v8::Handle result = args.Call(call_fun, v8::Utils::ToLocal(key)); RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); @@ -415,29 +429,79 @@ MaybeHandle JSObject::GetPropertyWithCallback(Handle object, if (getter->IsSpecFunction()) { // TODO(rossberg): nicer would be to cast to some JSCallable here... return Object::GetPropertyWithDefinedGetter( - object, receiver, Handle::cast(getter)); + receiver, Handle::cast(getter)); } // Getter is not a function. return isolate->factory()->undefined_value(); } -MaybeHandle JSProxy::GetPropertyWithHandler(Handle proxy, - Handle receiver, - Handle name) { - Isolate* isolate = proxy->GetIsolate(); +MaybeHandle Object::SetPropertyWithCallback(Handle receiver, + Handle name, + Handle value, + Handle holder, + Handle structure, + StrictMode strict_mode) { + Isolate* isolate = name->GetIsolate(); - // TODO(rossberg): adjust once there is a story for symbols vs proxies. - if (name->IsSymbol()) return isolate->factory()->undefined_value(); + // We should never get here to initialize a const with the hole + // value since a const declaration would conflict with the setter. + ASSERT(!value->IsTheHole()); + ASSERT(!structure->IsForeign()); + if (structure->IsExecutableAccessorInfo()) { + // api style callbacks + ExecutableAccessorInfo* data = ExecutableAccessorInfo::cast(*structure); + if (!data->IsCompatibleReceiver(*receiver)) { + Handle args[2] = { name, receiver }; + Handle error = + isolate->factory()->NewTypeError("incompatible_method_receiver", + HandleVector(args, + ARRAY_SIZE(args))); + return isolate->Throw(error); + } + // TODO(rossberg): Support symbols in the API. + if (name->IsSymbol()) return value; + Object* call_obj = data->setter(); + v8::AccessorSetterCallback call_fun = + v8::ToCData(call_obj); + if (call_fun == NULL) return value; + Handle key = Handle::cast(name); + LOG(isolate, ApiNamedPropertyAccess("store", *holder, *name)); + PropertyCallbackArguments args(isolate, data->data(), *receiver, *holder); + args.Call(call_fun, + v8::Utils::ToLocal(key), + v8::Utils::ToLocal(value)); + RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); + return value; + } - Handle args[] = { receiver, name }; - return CallTrap( - proxy, "get", isolate->derived_get_trap(), ARRAY_SIZE(args), args); + if (structure->IsAccessorPair()) { + Handle setter(AccessorPair::cast(*structure)->setter(), isolate); + if (setter->IsSpecFunction()) { + // TODO(rossberg): nicer would be to cast to some JSCallable here... + return SetPropertyWithDefinedSetter( + receiver, Handle::cast(setter), value); + } else { + if (strict_mode == SLOPPY) return value; + Handle args[2] = { name, holder }; + Handle error = + isolate->factory()->NewTypeError("no_setter_in_callback", + HandleVector(args, 2)); + return isolate->Throw(error); + } + } + + // TODO(dcarney): Handle correctly. + if (structure->IsDeclaredAccessorInfo()) { + return value; + } + + UNREACHABLE(); + return MaybeHandle(); } MaybeHandle Object::GetPropertyWithDefinedGetter( - Handle object, Handle receiver, Handle getter) { Isolate* isolate = getter->GetIsolate(); @@ -453,6 +517,29 @@ MaybeHandle Object::GetPropertyWithDefinedGetter( } +MaybeHandle Object::SetPropertyWithDefinedSetter( + Handle receiver, + Handle setter, + Handle value) { + Isolate* isolate = setter->GetIsolate(); + + Debug* debug = isolate->debug(); + // Handle stepping into a setter if step into is active. + // TODO(rossberg): should this apply to getters that are function proxies? + if (debug->StepInActive() && setter->IsJSFunction()) { + debug->HandleStepIn( + Handle::cast(setter), Handle::null(), 0, false); + } + + Handle argv[] = { value }; + RETURN_ON_EXCEPTION( + isolate, + Execution::Call(isolate, setter, receiver, ARRAY_SIZE(argv), argv), + Object); + return value; +} + + // Only deal with CALLBACKS and INTERCEPTOR MaybeHandle JSObject::GetPropertyWithFailedAccessCheck( Handle object, @@ -477,7 +564,7 @@ MaybeHandle JSObject::GetPropertyWithFailedAccessCheck( break; } Handle holder(result->holder(), isolate); - return GetPropertyWithCallback(holder, receiver, callback_obj, name); + return GetPropertyWithCallback(receiver, name, holder, callback_obj); } case NORMAL: case FIELD: @@ -791,11 +878,9 @@ MaybeHandle Object::GetProperty(Handle object, case CONSTANT: return handle(result->GetConstant(), isolate); case CALLBACKS: - return JSObject::GetPropertyWithCallback( - handle(result->holder(), isolate), - receiver, - handle(result->GetCallbackObject(), isolate), - name); + return GetPropertyWithCallback( + receiver, name, handle(result->holder(), isolate), + handle(result->GetCallbackObject(), isolate)); case HANDLER: return JSProxy::GetPropertyWithHandler( handle(result->proxy(), isolate), receiver, name); @@ -3005,94 +3090,6 @@ MaybeHandle JSReceiver::SetProperty(Handle object, } -MaybeHandle JSObject::SetPropertyWithCallback(Handle object, - Handle structure, - Handle name, - Handle value, - Handle holder, - StrictMode strict_mode) { - Isolate* isolate = object->GetIsolate(); - - // We should never get here to initialize a const with the hole - // value since a const declaration would conflict with the setter. - ASSERT(!value->IsTheHole()); - ASSERT(!structure->IsForeign()); - if (structure->IsExecutableAccessorInfo()) { - // api style callbacks - ExecutableAccessorInfo* data = ExecutableAccessorInfo::cast(*structure); - if (!data->IsCompatibleReceiver(*object)) { - Handle args[2] = { name, object }; - Handle error = - isolate->factory()->NewTypeError("incompatible_method_receiver", - HandleVector(args, - ARRAY_SIZE(args))); - return isolate->Throw(error); - } - // TODO(rossberg): Support symbols in the API. - if (name->IsSymbol()) return value; - Object* call_obj = data->setter(); - v8::AccessorSetterCallback call_fun = - v8::ToCData(call_obj); - if (call_fun == NULL) return value; - Handle key = Handle::cast(name); - LOG(isolate, ApiNamedPropertyAccess("store", *object, *name)); - PropertyCallbackArguments args(isolate, data->data(), *object, *holder); - args.Call(call_fun, - v8::Utils::ToLocal(key), - v8::Utils::ToLocal(value)); - RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); - return value; - } - - if (structure->IsAccessorPair()) { - Handle setter(AccessorPair::cast(*structure)->setter(), isolate); - if (setter->IsSpecFunction()) { - // TODO(rossberg): nicer would be to cast to some JSCallable here... - return SetPropertyWithDefinedSetter( - object, Handle::cast(setter), value); - } else { - if (strict_mode == SLOPPY) return value; - Handle args[2] = { name, holder }; - Handle error = - isolate->factory()->NewTypeError("no_setter_in_callback", - HandleVector(args, 2)); - return isolate->Throw(error); - } - } - - // TODO(dcarney): Handle correctly. - if (structure->IsDeclaredAccessorInfo()) { - return value; - } - - UNREACHABLE(); - return MaybeHandle(); -} - - -MaybeHandle JSReceiver::SetPropertyWithDefinedSetter( - Handle object, - Handle setter, - Handle value) { - Isolate* isolate = object->GetIsolate(); - - Debug* debug = isolate->debug(); - // Handle stepping into a setter if step into is active. - // TODO(rossberg): should this apply to getters that are function proxies? - if (debug->StepInActive() && setter->IsJSFunction()) { - debug->HandleStepIn( - Handle::cast(setter), Handle::null(), 0, false); - } - - Handle argv[] = { value }; - RETURN_ON_EXCEPTION( - isolate, - Execution::Call(isolate, setter, object, ARRAY_SIZE(argv), argv), - Object); - return value; -} - - MaybeHandle JSObject::SetElementWithCallbackSetterInPrototypes( Handle object, uint32_t index, @@ -3166,8 +3163,9 @@ MaybeHandle JSObject::SetPropertyViaPrototypes( *done = true; if (!result.IsReadOnly()) { Handle callback_object(result.GetCallbackObject(), isolate); - return SetPropertyWithCallback(object, callback_object, name, value, - handle(result.holder()), strict_mode); + return SetPropertyWithCallback(object, name, value, + handle(result.holder()), + callback_object, strict_mode); } break; } @@ -3625,22 +3623,16 @@ MaybeHandle JSObject::SetPropertyWithFailedAccessCheck( if (obj->IsAccessorInfo()) { Handle info(AccessorInfo::cast(obj)); if (info->all_can_write()) { - return SetPropertyWithCallback(object, - info, - name, - value, + return SetPropertyWithCallback(object, name, value, handle(result->holder()), - strict_mode); + info, strict_mode); } } else if (obj->IsAccessorPair()) { Handle pair(AccessorPair::cast(obj)); if (pair->all_can_read()) { - return SetPropertyWithCallback(object, - pair, - name, - value, + return SetPropertyWithCallback(object, name, value, handle(result->holder()), - strict_mode); + pair, strict_mode); } } break; @@ -4300,8 +4292,9 @@ MaybeHandle JSObject::SetPropertyForResult( break; case CALLBACKS: { Handle callback_object(lookup->GetCallbackObject(), isolate); - return SetPropertyWithCallback(object, callback_object, name, value, - handle(lookup->holder()), strict_mode); + return SetPropertyWithCallback(object, name, value, + handle(lookup->holder()), + callback_object, strict_mode); } case INTERCEPTOR: maybe_result = SetPropertyWithInterceptor( @@ -12581,7 +12574,7 @@ MaybeHandle JSObject::GetElementWithCallback( if (getter->IsSpecFunction()) { // TODO(rossberg): nicer would be to cast to some JSCallable here... return GetPropertyWithDefinedGetter( - object, receiver, Handle::cast(getter)); + receiver, Handle::cast(getter)); } // Getter is not a function. return isolate->factory()->undefined_value(); diff --git a/src/objects.h b/src/objects.h index 9059eda..cde3bf7 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1477,10 +1477,26 @@ class Object { Handle key, PropertyAttributes* attributes); + MUST_USE_RESULT static MaybeHandle GetPropertyWithCallback( + Handle receiver, + Handle name, + Handle holder, + Handle structure); + MUST_USE_RESULT static MaybeHandle SetPropertyWithCallback( + Handle receiver, + Handle name, + Handle value, + Handle holder, + Handle structure, + StrictMode strict_mode); + MUST_USE_RESULT static MaybeHandle GetPropertyWithDefinedGetter( - Handle object, Handle receiver, Handle getter); + MUST_USE_RESULT static MaybeHandle SetPropertyWithDefinedSetter( + Handle receiver, + Handle setter, + Handle value); MUST_USE_RESULT static inline MaybeHandle GetElement( Isolate* isolate, @@ -1994,12 +2010,6 @@ class JSReceiver: public HeapObject { Handle object, KeyCollectionType type); - protected: - MUST_USE_RESULT static MaybeHandle SetPropertyWithDefinedSetter( - Handle object, - Handle setter, - Handle value); - private: static PropertyAttributes GetPropertyAttributeForResult( Handle object, @@ -2128,20 +2138,6 @@ class JSObject: public JSReceiver { static Handle PrepareSlowElementsForSort(Handle object, uint32_t limit); - MUST_USE_RESULT static MaybeHandle GetPropertyWithCallback( - Handle object, - Handle receiver, - Handle structure, - Handle name); - - MUST_USE_RESULT static MaybeHandle SetPropertyWithCallback( - Handle object, - Handle structure, - Handle name, - Handle value, - Handle holder, - StrictMode strict_mode); - MUST_USE_RESULT static MaybeHandle SetPropertyWithInterceptor( Handle object, Handle name, diff --git a/src/runtime.cc b/src/runtime.cc index 351a4ca..68a730b 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -5188,12 +5188,9 @@ RUNTIME_FUNCTION(Runtime_DefineOrRedefineDataProperty) { Handle result_object; ASSIGN_RETURN_FAILURE_ON_EXCEPTION( isolate, result_object, - JSObject::SetPropertyWithCallback(js_object, - callback, - name, - obj_value, + JSObject::SetPropertyWithCallback(js_object, name, obj_value, handle(lookup.holder()), - STRICT)); + callback, STRICT)); return *result_object; } } @@ -10789,7 +10786,7 @@ static Handle DebugLookupResultValue(Isolate* isolate, ASSERT(!structure->IsForeign()); if (structure->IsAccessorInfo()) { MaybeHandle obj = JSObject::GetPropertyWithCallback( - handle(result->holder(), isolate), receiver, structure, name); + receiver, name, handle(result->holder(), isolate), structure); if (!obj.ToHandle(&value)) { value = handle(isolate->pending_exception(), isolate); isolate->clear_pending_exception(); -- 2.7.4