Remove AccessControl from AccessorPairs, as it's an invalid usecase of AllCan*

BUG=
R=dcarney@chromium.org

Review URL: https://codereview.chromium.org/332863003

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21918 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
verwaest@chromium.org 2014-06-23 09:02:16 +00:00
parent d611bd896b
commit d06afb3ce0
13 changed files with 36 additions and 137 deletions

View File

@ -833,6 +833,8 @@ void Template::SetAccessorProperty(
v8::Local<FunctionTemplate> setter,
v8::PropertyAttribute attribute,
v8::AccessControl access_control) {
// TODO(verwaest): Remove |access_control|.
ASSERT_EQ(v8::DEFAULT, access_control);
i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
ENTER_V8(isolate);
ASSERT(!name.IsEmpty());
@ -844,8 +846,7 @@ void Template::SetAccessorProperty(
name,
getter,
setter,
v8::Integer::New(v8_isolate, attribute),
v8::Integer::New(v8_isolate, access_control)};
v8::Integer::New(v8_isolate, attribute)};
TemplateSet(isolate, this, kSize, data);
}
@ -3446,6 +3447,8 @@ void Object::SetAccessorProperty(Local<String> name,
Handle<Function> setter,
PropertyAttribute attribute,
AccessControl settings) {
// TODO(verwaest): Remove |settings|.
ASSERT_EQ(v8::DEFAULT, settings);
i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
ON_BAILOUT(isolate, "v8::Object::SetAccessorProperty()", return);
ENTER_V8(isolate);
@ -3457,8 +3460,7 @@ void Object::SetAccessorProperty(Local<String> name,
v8::Utils::OpenHandle(*name),
getter_i,
setter_i,
static_cast<PropertyAttributes>(attribute),
settings);
static_cast<PropertyAttributes>(attribute));
}

View File

@ -94,14 +94,14 @@ function ConfigureTemplateInstance(obj, data) {
var attributes = properties[i + 3];
var value = Instantiate(prop_data, name);
%SetProperty(obj, name, value, attributes);
} else if (length == 5) {
} else if (length == 4 || length == 5) {
// TODO(verwaest): The 5th value used to be access_control. Remove once
// the bindings are updated.
var name = properties[i + 1];
var getter = properties[i + 2];
var setter = properties[i + 3];
var attribute = properties[i + 4];
var access_control = properties[i + 5];
%SetAccessorProperty(
obj, name, getter, setter, attribute, access_control);
%SetAccessorProperty(obj, name, getter, setter, attribute);
} else {
throw "Bad properties array";
}

View File

@ -151,7 +151,6 @@ Handle<AccessorPair> Factory::NewAccessorPair() {
Handle<AccessorPair>::cast(NewStruct(ACCESSOR_PAIR_TYPE));
accessors->set_getter(*the_hole_value(), SKIP_WRITE_BARRIER);
accessors->set_setter(*the_hole_value(), SKIP_WRITE_BARRIER);
accessors->set_access_flags(Smi::FromInt(0), SKIP_WRITE_BARRIER);
return accessors;
}

View File

@ -878,7 +878,6 @@ void AccessorPair::AccessorPairVerify() {
CHECK(IsAccessorPair());
VerifyPointer(getter());
VerifyPointer(setter());
VerifySmiField(kAccessFlagsOffset);
}

View File

@ -5181,7 +5181,6 @@ ACCESSORS(Box, value, Object, kValueOffset)
ACCESSORS(AccessorPair, getter, Object, kGetterOffset)
ACCESSORS(AccessorPair, setter, Object, kSetterOffset)
ACCESSORS_TO_SMI(AccessorPair, access_flags, kAccessFlagsOffset)
ACCESSORS(AccessCheckInfo, named_callback, Object, kNamedCallbackOffset)
ACCESSORS(AccessCheckInfo, indexed_callback, Object, kIndexedCallbackOffset)
@ -6587,28 +6586,6 @@ void ExecutableAccessorInfo::clear_setter() {
}
void AccessorPair::set_access_flags(v8::AccessControl access_control) {
int current = access_flags()->value();
current = BooleanBit::set(current,
kAllCanReadBit,
access_control & ALL_CAN_READ);
current = BooleanBit::set(current,
kAllCanWriteBit,
access_control & ALL_CAN_WRITE);
set_access_flags(Smi::FromInt(current));
}
bool AccessorPair::all_can_read() {
return BooleanBit::get(access_flags(), kAllCanReadBit);
}
bool AccessorPair::all_can_write() {
return BooleanBit::get(access_flags(), kAllCanWriteBit);
}
template<typename Derived, typename Shape, typename Key>
void Dictionary<Derived, Shape, Key>::SetEntry(int entry,
Handle<Object> key,

View File

@ -1010,8 +1010,6 @@ void AccessorPair::AccessorPairPrint(FILE* out) {
getter()->ShortPrint(out);
PrintF(out, "\n - setter: ");
setter()->ShortPrint(out);
PrintF(out, "\n - flag: ");
access_flags()->ShortPrint(out);
}

View File

@ -575,8 +575,6 @@ static bool FindAllCanReadHolder(LookupIterator* it) {
Handle<Object> accessors = it->GetAccessors();
if (accessors->IsAccessorInfo()) {
if (AccessorInfo::cast(*accessors)->all_can_read()) return true;
} else if (accessors->IsAccessorPair()) {
if (AccessorPair::cast(*accessors)->all_can_read()) return true;
}
}
}
@ -619,8 +617,6 @@ static bool FindAllCanWriteHolder(LookupResult* result,
Object* callback_obj = result->GetCallbackObject();
if (callback_obj->IsAccessorInfo()) {
if (AccessorInfo::cast(callback_obj)->all_can_write()) return true;
} else if (callback_obj->IsAccessorPair()) {
if (AccessorPair::cast(callback_obj)->all_can_write()) return true;
}
}
if (!check_prototype) break;
@ -6440,8 +6436,7 @@ void JSObject::DefineElementAccessor(Handle<JSObject> object,
uint32_t index,
Handle<Object> getter,
Handle<Object> setter,
PropertyAttributes attributes,
v8::AccessControl access_control) {
PropertyAttributes attributes) {
switch (object->GetElementsKind()) {
case FAST_SMI_ELEMENTS:
case FAST_ELEMENTS:
@ -6498,7 +6493,6 @@ void JSObject::DefineElementAccessor(Handle<JSObject> object,
Isolate* isolate = object->GetIsolate();
Handle<AccessorPair> accessors = isolate->factory()->NewAccessorPair();
accessors->SetComponents(*getter, *setter);
accessors->set_access_flags(access_control);
SetElementCallback(object, index, accessors, attributes);
}
@ -6529,13 +6523,11 @@ void JSObject::DefinePropertyAccessor(Handle<JSObject> object,
Handle<Name> name,
Handle<Object> getter,
Handle<Object> setter,
PropertyAttributes attributes,
v8::AccessControl access_control) {
PropertyAttributes attributes) {
// We could assert that the property is configurable here, but we would need
// to do a lookup, which seems to be a bit of overkill.
bool only_attribute_changes = getter->IsNull() && setter->IsNull();
if (object->HasFastProperties() && !only_attribute_changes &&
access_control == v8::DEFAULT &&
(object->map()->NumberOfOwnDescriptors() <= kMaxNumberOfDescriptors)) {
bool getterOk = getter->IsNull() ||
DefineFastAccessor(object, name, ACCESSOR_GETTER, getter, attributes);
@ -6546,7 +6538,6 @@ void JSObject::DefinePropertyAccessor(Handle<JSObject> object,
Handle<AccessorPair> accessors = CreateAccessorPairFor(object, name);
accessors->SetComponents(*getter, *setter);
accessors->set_access_flags(access_control);
SetPropertyCallback(object, name, accessors, attributes);
}
@ -6647,8 +6638,7 @@ void JSObject::DefineAccessor(Handle<JSObject> object,
Handle<Name> name,
Handle<Object> getter,
Handle<Object> setter,
PropertyAttributes attributes,
v8::AccessControl access_control) {
PropertyAttributes attributes) {
Isolate* isolate = object->GetIsolate();
// Check access rights if needed.
if (object->IsAccessCheckNeeded() &&
@ -6666,8 +6656,7 @@ void JSObject::DefineAccessor(Handle<JSObject> object,
name,
getter,
setter,
attributes,
access_control);
attributes);
return;
}
@ -6704,11 +6693,9 @@ void JSObject::DefineAccessor(Handle<JSObject> object,
}
if (is_element) {
DefineElementAccessor(
object, index, getter, setter, attributes, access_control);
DefineElementAccessor(object, index, getter, setter, attributes);
} else {
DefinePropertyAccessor(
object, name, getter, setter, attributes, access_control);
DefinePropertyAccessor(object, name, getter, setter, attributes);
}
if (is_observed) {
@ -6784,8 +6771,10 @@ bool JSObject::DefineFastAccessor(Handle<JSObject> object,
ASSERT(target->NumberOfOwnDescriptors() ==
object->map()->NumberOfOwnDescriptors());
// This works since descriptors are sorted in order of addition.
ASSERT(object->map()->instance_descriptors()->
GetKey(descriptor_number) == *name);
ASSERT(Name::Equals(
handle(object->map()->instance_descriptors()->GetKey(
descriptor_number)),
name));
return TryAccessorTransition(object, target, descriptor_number,
component, accessor, attributes);
}

View File

@ -2228,8 +2228,7 @@ class JSObject: public JSReceiver {
Handle<Name> name,
Handle<Object> getter,
Handle<Object> setter,
PropertyAttributes attributes,
v8::AccessControl access_control = v8::DEFAULT);
PropertyAttributes attributes);
// Defines an AccessorInfo property on the given object.
MUST_USE_RESULT static MaybeHandle<Object> SetAccessor(
@ -2823,16 +2822,14 @@ class JSObject: public JSReceiver {
uint32_t index,
Handle<Object> getter,
Handle<Object> setter,
PropertyAttributes attributes,
v8::AccessControl access_control);
PropertyAttributes attributes);
static Handle<AccessorPair> CreateAccessorPairFor(Handle<JSObject> object,
Handle<Name> name);
static void DefinePropertyAccessor(Handle<JSObject> object,
Handle<Name> name,
Handle<Object> getter,
Handle<Object> setter,
PropertyAttributes attributes,
v8::AccessControl access_control);
PropertyAttributes attributes);
// Try to define a single accessor paying attention to map transitions.
// Returns false if this was not possible and we have to use the slow case.
@ -10647,17 +10644,10 @@ class ExecutableAccessorInfo: public AccessorInfo {
// * undefined: considered an accessor by the spec, too, strangely enough
// * the hole: an accessor which has not been set
// * a pointer to a map: a transition used to ensure map sharing
// access_flags provides the ability to override access checks on access check
// failure.
class AccessorPair: public Struct {
public:
DECL_ACCESSORS(getter, Object)
DECL_ACCESSORS(setter, Object)
DECL_ACCESSORS(access_flags, Smi)
inline void set_access_flags(v8::AccessControl access_control);
inline bool all_can_read();
inline bool all_can_write();
DECLARE_CAST(AccessorPair)
@ -10694,13 +10684,9 @@ class AccessorPair: public Struct {
static const int kGetterOffset = HeapObject::kHeaderSize;
static const int kSetterOffset = kGetterOffset + kPointerSize;
static const int kAccessFlagsOffset = kSetterOffset + kPointerSize;
static const int kSize = kAccessFlagsOffset + kPointerSize;
static const int kSize = kSetterOffset + kPointerSize;
private:
static const int kAllCanReadBit = 0;
static const int kAllCanWriteBit = 1;
// Strangely enough, in addition to functions and harmony proxies, the spec
// requires us to consider undefined as a kind of accessor, too:
// var obj = {};

View File

@ -1899,14 +1899,6 @@ static bool CheckAccessException(Object* callback,
(access_type == v8::ACCESS_GET && info->all_can_read()) ||
(access_type == v8::ACCESS_SET && info->all_can_write());
}
if (callback->IsAccessorPair()) {
AccessorPair* info = AccessorPair::cast(callback);
return
(access_type == v8::ACCESS_HAS &&
(info->all_can_read() || info->all_can_write())) ||
(access_type == v8::ACCESS_GET && info->all_can_read()) ||
(access_type == v8::ACCESS_SET && info->all_can_write());
}
return false;
}
@ -1959,8 +1951,8 @@ static void CheckPropertyAccess(Handle<JSObject> obj,
}
// Access check callback denied the access, but some properties
// can have a special permissions which override callbacks descision
// (currently see v8::AccessControl).
// can have a special permissions which override callbacks decision
// (see v8::AccessControl).
// API callbacks can have per callback access exceptions.
if (lookup.IsFound() && lookup.type() == INTERCEPTOR) {
lookup.holder()->LookupOwnRealNamedProperty(name, &lookup);
@ -2175,13 +2167,12 @@ static Handle<Object> InstantiateAccessorComponent(Isolate* isolate,
RUNTIME_FUNCTION(Runtime_SetAccessorProperty) {
HandleScope scope(isolate);
ASSERT(args.length() == 6);
ASSERT(args.length() == 5);
CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
CONVERT_ARG_HANDLE_CHECKED(Name, name, 1);
CONVERT_ARG_HANDLE_CHECKED(Object, getter, 2);
CONVERT_ARG_HANDLE_CHECKED(Object, setter, 3);
CONVERT_SMI_ARG_CHECKED(attribute, 4);
CONVERT_SMI_ARG_CHECKED(access_control, 5);
RUNTIME_ASSERT(getter->IsUndefined() || getter->IsFunctionTemplateInfo());
RUNTIME_ASSERT(setter->IsUndefined() || setter->IsFunctionTemplateInfo());
RUNTIME_ASSERT(PropertyDetails::AttributesField::is_valid(
@ -2190,8 +2181,7 @@ RUNTIME_FUNCTION(Runtime_SetAccessorProperty) {
name,
InstantiateAccessorComponent(isolate, getter),
InstantiateAccessorComponent(isolate, setter),
static_cast<PropertyAttributes>(attribute),
static_cast<v8::AccessControl>(access_control));
static_cast<PropertyAttributes>(attribute));
return isolate->heap()->undefined_value();
}

View File

@ -206,7 +206,7 @@ namespace internal {
F(GetTemplateField, 2, 1) \
F(DisableAccessChecks, 1, 1) \
F(EnableAccessChecks, 1, 1) \
F(SetAccessorProperty, 6, 1) \
F(SetAccessorProperty, 5, 1) \
\
/* Dates */ \
F(DateCurrentTime, 0, 1) \

View File

@ -9020,19 +9020,13 @@ static bool IndexedAccessBlocker(Local<v8::Object> global,
}
static int g_echo_value_1 = -1;
static int g_echo_value_2 = -1;
static int g_echo_value = -1;
static void EchoGetter(
Local<String> name,
const v8::PropertyCallbackInfo<v8::Value>& info) {
info.GetReturnValue().Set(v8_num(g_echo_value_1));
}
static void EchoGetter(const v8::FunctionCallbackInfo<v8::Value>& info) {
info.GetReturnValue().Set(v8_num(g_echo_value_2));
info.GetReturnValue().Set(v8_num(g_echo_value));
}
@ -9040,14 +9034,7 @@ static void EchoSetter(Local<String> name,
Local<Value> value,
const v8::PropertyCallbackInfo<void>&) {
if (value->IsNumber())
g_echo_value_1 = value->Int32Value();
}
static void EchoSetter(const v8::FunctionCallbackInfo<v8::Value>& info) {
v8::Handle<v8::Value> value = info[0];
if (value->IsNumber())
g_echo_value_2 = value->Int32Value();
g_echo_value = value->Int32Value();
}
@ -9088,13 +9075,6 @@ TEST(AccessControl) {
v8::AccessControl(v8::ALL_CAN_READ | v8::ALL_CAN_WRITE));
global_template->SetAccessorProperty(
v8_str("accessible_js_prop"),
v8::FunctionTemplate::New(isolate, EchoGetter),
v8::FunctionTemplate::New(isolate, EchoSetter),
v8::None,
v8::AccessControl(v8::ALL_CAN_READ | v8::ALL_CAN_WRITE));
// Add an accessor that is not accessible by cross-domain JS code.
global_template->SetAccessor(v8_str("blocked_prop"),
UnreachableGetter, UnreachableSetter,
@ -9223,45 +9203,26 @@ TEST(AccessControl) {
value = CompileRun("other.accessible_prop = 3");
CHECK(value->IsNumber());
CHECK_EQ(3, value->Int32Value());
CHECK_EQ(3, g_echo_value_1);
// Access accessible js property
value = CompileRun("other.accessible_js_prop = 3");
CHECK(value->IsNumber());
CHECK_EQ(3, value->Int32Value());
CHECK_EQ(3, g_echo_value_2);
CHECK_EQ(3, g_echo_value);
value = CompileRun("other.accessible_prop");
CHECK(value->IsNumber());
CHECK_EQ(3, value->Int32Value());
value = CompileRun("other.accessible_js_prop");
CHECK(value->IsNumber());
CHECK_EQ(3, value->Int32Value());
value = CompileRun(
"Object.getOwnPropertyDescriptor(other, 'accessible_prop').value");
CHECK(value->IsNumber());
CHECK_EQ(3, value->Int32Value());
value = CompileRun(
"Object.getOwnPropertyDescriptor(other, 'accessible_js_prop').get()");
CHECK(value->IsNumber());
CHECK_EQ(3, value->Int32Value());
value = CompileRun("propertyIsEnumerable.call(other, 'accessible_prop')");
CHECK(value->IsTrue());
value = CompileRun("propertyIsEnumerable.call(other, 'accessible_js_prop')");
CHECK(value->IsTrue());
// Enumeration doesn't enumerate accessors from inaccessible objects in
// the prototype chain even if the accessors are in themselves accessible.
value =
CompileRun("(function(){var obj = {'__proto__':other};"
"for (var p in obj)"
" if (p == 'accessible_prop' ||"
" p == 'accessible_js_prop' ||"
" p == 'blocked_js_prop' ||"
" p == 'blocked_js_prop') {"
" return false;"
@ -9336,7 +9297,7 @@ TEST(AccessControlES5) {
// Make sure that we can set the accessible accessors value using normal
// assignment.
CompileRun("other.accessible_prop = 42");
CHECK_EQ(42, g_echo_value_1);
CHECK_EQ(42, g_echo_value);
v8::Handle<Value> value;
CompileRun("Object.defineProperty(other, 'accessible_prop', {value: -1})");

View File

@ -6,5 +6,4 @@ var _name = "name";
var arg2 = undefined;
var arg3 = undefined;
var _attribute = 1;
var _access_control = 1;
%SetAccessorProperty(_object, _name, arg2, arg3, _attribute, _access_control);
%SetAccessorProperty(_object, _name, arg2, arg3, _attribute);

View File

@ -158,8 +158,7 @@ CUSTOM_KNOWN_GOOD_INPUT = {
"NumberToRadixString": [None, "2", None],
"ParseJson": ["\"{}\"", 1],
"RegExpExecMultiple": [None, None, "['a']", "['a']", None],
"SetAccessorProperty": [None, None, "undefined", "undefined", None, None,
None],
"SetAccessorProperty": [None, None, "undefined", "undefined", None, None],
"SetIteratorInitialize": [None, None, "2", None],
"SetDebugEventListener": ["undefined", None, None],
"SetFunctionBreakPoint": [None, 200, None, None],