From 39296e679fc4d760087db784397a1ab93e92a5f4 Mon Sep 17 00:00:00 2001 From: neis Date: Mon, 30 Nov 2015 01:59:17 -0800 Subject: [PATCH] [proxies] Use IsRevoked where possible, remove has_handler. R=jkummerow@chromium.org BUG= Review URL: https://codereview.chromium.org/1486553002 Cr-Commit-Position: refs/heads/master@{#32394} --- src/messages.h | 2 ++ src/objects-inl.h | 1 - src/objects.cc | 54 +++++++++++++++++------------------- src/objects.h | 4 +-- src/runtime/runtime-proxy.cc | 11 ++------ 5 files changed, 32 insertions(+), 40 deletions(-) diff --git a/src/messages.h b/src/messages.h index e5fa5fc31a..501c7166f0 100644 --- a/src/messages.h +++ b/src/messages.h @@ -200,6 +200,8 @@ class CallSite { "Proxy handler % returned non-configurable descriptor for property '%' " \ "from '%' trap") \ T(ProxyRepeatedPropName, "Trap '%' returned repeated property name '%'") \ + T(ProxyHandlerOrTargetRevoked, \ + "Cannot create proxy with a revoked proxy as handler or target") \ T(ProxyRevoked, "Cannot perform '%' on a proxy that has been revoked") \ T(ProxyTargetNotExtensible, "Proxy target is not extensible") \ T(ProxyTargetNonObject, "Proxy target is non-object") \ diff --git a/src/objects-inl.h b/src/objects-inl.h index 9cb1481582..0beca26cb2 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -6331,7 +6331,6 @@ int JSFunction::NumberOfLiterals() { ACCESSORS(JSProxy, target, Object, kTargetOffset) ACCESSORS(JSProxy, handler, Object, kHandlerOffset) ACCESSORS(JSProxy, hash, Object, kHashOffset) -bool JSProxy::has_handler() { return !handler()->IsNull(); } ACCESSORS(JSFunctionProxy, call_trap, JSReceiver, kCallTrapOffset) ACCESSORS(JSFunctionProxy, construct_trap, Object, kConstructTrapOffset) diff --git a/src/objects.cc b/src/objects.cc index c87e962bcf..554473e991 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -910,11 +910,11 @@ MaybeHandle JSProxy::GetPrototype(Handle proxy) { } -bool JSProxy::IsRevoked(Handle proxy) { +bool JSProxy::IsRevoked() { // TODO(neis): Decide on how to represent revocation. For now, revocation is // unsupported. - DCHECK(proxy->target()->IsJSReceiver()); - DCHECK(proxy->handler()->IsJSReceiver()); + DCHECK(target()->IsJSReceiver()); + DCHECK(handler()->IsJSReceiver()); return false; } @@ -4529,7 +4529,7 @@ Maybe JSProxy::HasProperty(Isolate* isolate, Handle proxy, // 2. Let handler be the value of the [[ProxyHandler]] internal slot of O. Handle handler(proxy->handler(), isolate); // 3. If handler is null, throw a TypeError exception. - if (JSProxy::IsRevoked(proxy)) { + if (proxy->IsRevoked()) { isolate->Throw(*isolate->factory()->NewTypeError( MessageTemplate::kProxyRevoked, isolate->factory()->has_string())); return Nothing(); @@ -4705,7 +4705,7 @@ Maybe JSProxy::DeletePropertyOrElement(Handle proxy, Factory* factory = isolate->factory(); Handle trap_name = factory->deleteProperty_string(); - if (IsRevoked(proxy)) { + if (proxy->IsRevoked()) { isolate->Throw( *factory->NewTypeError(MessageTemplate::kProxyRevoked, trap_name)); return Nothing(); @@ -4750,7 +4750,7 @@ Maybe JSProxy::DeletePropertyOrElement(Handle proxy, // static MaybeHandle JSProxy::GetFunctionRealm(Handle proxy) { DCHECK(proxy->map()->is_constructor()); - if (JSProxy::IsRevoked(proxy)) { + if (proxy->IsRevoked()) { THROW_NEW_ERROR(proxy->GetIsolate(), NewTypeError(MessageTemplate::kProxyRevoked), Context); } @@ -4805,7 +4805,7 @@ Maybe JSProxy::GetPropertyAttributes(LookupIterator* it) { MaybeHandle JSProxy::GetTrap(Handle proxy, Handle trap) { - DCHECK(!IsRevoked(proxy)); + DCHECK(!proxy->IsRevoked()); Isolate* isolate = proxy->GetIsolate(); Handle handler(JSReceiver::cast(proxy->handler()), isolate); return Object::GetMethod(handler, trap); @@ -6781,33 +6781,31 @@ bool JSArray::ArraySetLength(Isolate* isolate, Handle a, // ES6 9.5.6 // static -bool JSProxy::DefineOwnProperty(Isolate* isolate, Handle object, +bool JSProxy::DefineOwnProperty(Isolate* isolate, Handle proxy, Handle key, PropertyDescriptor* desc, ShouldThrow should_throw) { + Handle trap_name = isolate->factory()->defineProperty_string(); // 1. Assert: IsPropertyKey(P) is true. DCHECK(key->IsName() || key->IsNumber()); // 2. Let handler be the value of the [[ProxyHandler]] internal slot of O. - Handle handler(object->handler(), isolate); + Handle handler(proxy->handler(), isolate); // 3. If handler is null, throw a TypeError exception. - // TODO(jkummerow): Use "IsRevoked()" instead once we have it. - if (handler->IsNull()) { + if (proxy->IsRevoked()) { isolate->Throw(*isolate->factory()->NewTypeError( - MessageTemplate::kProxyHandlerNonObject)); + MessageTemplate::kProxyRevoked, trap_name)); return false; } // 4. Assert: Type(handler) is Object. DCHECK(handler->IsJSReceiver()); // If the handler is not null, the target can't be null either. - DCHECK(object->target()->IsSpecObject()); + DCHECK(proxy->target()->IsSpecObject()); // 5. Let target be the value of the [[ProxyTarget]] internal slot of O. - Handle target(JSReceiver::cast(object->target()), isolate); + Handle target(JSReceiver::cast(proxy->target()), isolate); // 6. Let trap be ? GetMethod(handler, "defineProperty"). Handle trap; ASSIGN_RETURN_ON_EXCEPTION_VALUE( isolate, trap, - Object::GetMethod(Handle::cast(handler), - isolate->factory()->defineProperty_string()), - false); + Object::GetMethod(Handle::cast(handler), trap_name), false); // 7. If trap is undefined, then: if (trap->IsUndefined()) { // 7a. Return target.[[DefineOwnProperty]](P, Desc). @@ -6968,16 +6966,18 @@ bool JSReceiver::GetOwnPropertyDescriptor(LookupIterator* it, // static bool JSProxy::GetOwnPropertyDescriptor(LookupIterator* it, PropertyDescriptor* desc) { - DCHECK(it->GetHolder()->IsJSProxy()); + Handle proxy = it->GetHolder(); Isolate* isolate = it->isolate(); + Handle trap_name = + isolate->factory()->getOwnPropertyDescriptor_string(); Handle property_name = it->GetName(); // 1. (Assert) // 2. Let handler be the value of the [[ProxyHandler]] internal slot of O. - Handle handler(it->GetHolder()->handler(), isolate); + Handle handler(proxy->handler(), isolate); // 3. If handler is null, throw a TypeError exception. - if (handler->IsNull()) { + if (proxy->IsRevoked()) { isolate->Throw(*isolate->factory()->NewTypeError( - MessageTemplate::kProxyHandlerNonObject)); + MessageTemplate::kProxyRevoked, trap_name)); return false; } // 4. Assert: Type(handler) is Object. @@ -6991,9 +6991,7 @@ bool JSProxy::GetOwnPropertyDescriptor(LookupIterator* it, Handle trap; ASSIGN_RETURN_ON_EXCEPTION_VALUE( isolate, trap, - Object::GetMethod(Handle::cast(handler), - isolate->factory()->getOwnPropertyDescriptor_string()), - false); + Object::GetMethod(Handle::cast(handler), trap_name), false); // 7. If trap is undefined, then if (trap->IsUndefined()) { // 7a. Return target.[[GetOwnProperty]](P). @@ -7233,7 +7231,7 @@ Maybe JSProxy::PreventExtensions(Handle proxy, Factory* factory = isolate->factory(); Handle trap_name = factory->preventExtensions_string(); - if (IsRevoked(proxy)) { + if (proxy->IsRevoked()) { isolate->Throw( *factory->NewTypeError(MessageTemplate::kProxyRevoked, trap_name)); return Nothing(); @@ -7343,7 +7341,7 @@ Maybe JSProxy::IsExtensible(Handle proxy) { Factory* factory = isolate->factory(); Handle trap_name = factory->isExtensible_string(); - if (IsRevoked(proxy)) { + if (proxy->IsRevoked()) { isolate->Throw( *factory->NewTypeError(MessageTemplate::kProxyRevoked, trap_name)); return Nothing(); @@ -8208,7 +8206,7 @@ bool JSProxy::Enumerate(Isolate* isolate, Handle receiver, // 1. Let handler be the value of the [[ProxyHandler]] internal slot of O. Handle handler(proxy->handler(), isolate); // 2. If handler is null, throw a TypeError exception. - if (IsRevoked(proxy)) { + if (proxy->IsRevoked()) { isolate->Throw(*isolate->factory()->NewTypeError( MessageTemplate::kProxyRevoked, isolate->factory()->enumerate_string())); @@ -8316,7 +8314,7 @@ bool JSProxy::OwnPropertyKeys(Isolate* isolate, Handle receiver, // 1. Let handler be the value of the [[ProxyHandler]] internal slot of O. Handle handler(proxy->handler(), isolate); // 2. If handler is null, throw a TypeError exception. - if (IsRevoked(proxy)) { + if (proxy->IsRevoked()) { isolate->Throw(*isolate->factory()->NewTypeError( MessageTemplate::kProxyRevoked, isolate->factory()->ownKeys_string())); return false; diff --git a/src/objects.h b/src/objects.h index b0efea4801..4a61935a27 100644 --- a/src/objects.h +++ b/src/objects.h @@ -9493,11 +9493,9 @@ class JSProxy: public JSReceiver { static MaybeHandle GetFunctionRealm(Handle proxy); - inline bool has_handler(); - DECLARE_CAST(JSProxy) - static bool IsRevoked(Handle proxy); + bool IsRevoked(); // ES6 9.5.1 static MaybeHandle GetPrototype(Handle receiver); diff --git a/src/runtime/runtime-proxy.cc b/src/runtime/runtime-proxy.cc index 2183c88cb8..72517195d9 100644 --- a/src/runtime/runtime-proxy.cc +++ b/src/runtime/runtime-proxy.cc @@ -20,19 +20,14 @@ RUNTIME_FUNCTION(Runtime_CreateJSProxy) { THROW_NEW_ERROR_RETURN_FAILURE( isolate, NewTypeError(MessageTemplate::kProxyTargetNonObject)); } - if (target->IsJSProxy() && !JSProxy::cast(*target)->has_handler()) { - // TODO(cbruni): Use better error message. - THROW_NEW_ERROR_RETURN_FAILURE( - isolate, NewTypeError(MessageTemplate::kProxyTargetNonObject)); - } if (!handler->IsSpecObject()) { THROW_NEW_ERROR_RETURN_FAILURE( isolate, NewTypeError(MessageTemplate::kProxyHandlerNonObject)); } - if (handler->IsJSProxy() && !JSProxy::cast(*handler)->has_handler()) { - // TODO(cbruni): Use better error message. + if ((target->IsJSProxy() && JSProxy::cast(*target)->IsRevoked()) || + (handler->IsJSProxy() && JSProxy::cast(*handler)->IsRevoked())) { THROW_NEW_ERROR_RETURN_FAILURE( - isolate, NewTypeError(MessageTemplate::kProxyHandlerNonObject)); + isolate, NewTypeError(MessageTemplate::kProxyHandlerOrTargetRevoked)); } return *isolate->factory()->NewJSProxy(Handle::cast(target), Handle::cast(handler));