Revision: 21918
Author: [email protected]
Date: Mon Jun 23 09:02:16 2014 UTC
Log: Remove AccessControl from AccessorPairs, as it's an invalid
usecase of AllCan*
BUG=
[email protected]
Review URL: https://codereview.chromium.org/332863003
http://code.google.com/p/v8/source/detail?r=21918
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/apinatives.js
/branches/bleeding_edge/src/factory.cc
/branches/bleeding_edge/src/objects-debug.cc
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects-printer.cc
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/runtime.h
/branches/bleeding_edge/test/cctest/test-api.cc
/branches/bleeding_edge/test/mjsunit/runtime-gen/setaccessorproperty.js
/branches/bleeding_edge/tools/generate-runtime-tests.py
=======================================
--- /branches/bleeding_edge/src/api.cc Fri Jun 20 08:40:11 2014 UTC
+++ /branches/bleeding_edge/src/api.cc Mon Jun 23 09:02:16 2014 UTC
@@ -833,6 +833,8 @@
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 @@
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 @@
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 @@
v8::Utils::OpenHandle(*name),
getter_i,
setter_i,
- static_cast<PropertyAttributes>(attribute),
- settings);
+ static_cast<PropertyAttributes>(attribute));
}
=======================================
--- /branches/bleeding_edge/src/apinatives.js Mon May 12 08:43:01 2014 UTC
+++ /branches/bleeding_edge/src/apinatives.js Mon Jun 23 09:02:16 2014 UTC
@@ -94,14 +94,14 @@
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";
}
=======================================
--- /branches/bleeding_edge/src/factory.cc Wed Jun 18 13:26:02 2014 UTC
+++ /branches/bleeding_edge/src/factory.cc Mon Jun 23 09:02:16 2014 UTC
@@ -151,7 +151,6 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/objects-debug.cc Fri Jun 20 08:40:11 2014
UTC
+++ /branches/bleeding_edge/src/objects-debug.cc Mon Jun 23 09:02:16 2014
UTC
@@ -878,7 +878,6 @@
CHECK(IsAccessorPair());
VerifyPointer(getter());
VerifyPointer(setter());
- VerifySmiField(kAccessFlagsOffset);
}
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Mon Jun 23 08:51:13 2014 UTC
+++ /branches/bleeding_edge/src/objects-inl.h Mon Jun 23 09:02:16 2014 UTC
@@ -5181,7 +5181,6 @@
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)
@@ -6585,28 +6584,6 @@
void ExecutableAccessorInfo::clear_setter() {
set_setter(GetIsolate()->heap()->undefined_value(), SKIP_WRITE_BARRIER);
}
-
-
-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>
=======================================
--- /branches/bleeding_edge/src/objects-printer.cc Fri Jun 20 08:40:11 2014
UTC
+++ /branches/bleeding_edge/src/objects-printer.cc Mon Jun 23 09:02:16 2014
UTC
@@ -1010,8 +1010,6 @@
getter()->ShortPrint(out);
PrintF(out, "\n - setter: ");
setter()->ShortPrint(out);
- PrintF(out, "\n - flag: ");
- access_flags()->ShortPrint(out);
}
=======================================
--- /branches/bleeding_edge/src/objects.cc Fri Jun 20 08:40:11 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc Mon Jun 23 09:02:16 2014 UTC
@@ -575,8 +575,6 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
name,
getter,
setter,
- attributes,
- access_control);
+ attributes);
return;
}
@@ -6704,11 +6693,9 @@
}
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 @@
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);
}
=======================================
--- /branches/bleeding_edge/src/objects.h Mon Jun 23 08:51:13 2014 UTC
+++ /branches/bleeding_edge/src/objects.h Mon Jun 23 09:02:16 2014 UTC
@@ -2228,8 +2228,7 @@
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 @@
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 @@
// * 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 @@
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 = {};
=======================================
--- /branches/bleeding_edge/src/runtime.cc Mon Jun 23 08:53:27 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc Mon Jun 23 09:02:16 2014 UTC
@@ -1899,14 +1899,6 @@
(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 @@
}
// 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 @@
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 @@
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();
}
=======================================
--- /branches/bleeding_edge/src/runtime.h Mon Jun 23 07:10:25 2014 UTC
+++ /branches/bleeding_edge/src/runtime.h Mon Jun 23 09:02:16 2014 UTC
@@ -206,7 +206,7 @@
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) \
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Mon Jun 23 08:53:27
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-api.cc Mon Jun 23 09:02:16
2014 UTC
@@ -9020,19 +9020,13 @@
}
-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 @@
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 @@
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,
@@ -9222,46 +9202,27 @@
// Access accessible property
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 @@
// 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})");
=======================================
--- /branches/bleeding_edge/test/mjsunit/runtime-gen/setaccessorproperty.js
Thu May 8 13:11:59 2014 UTC
+++ /branches/bleeding_edge/test/mjsunit/runtime-gen/setaccessorproperty.js
Mon Jun 23 09:02:16 2014 UTC
@@ -6,5 +6,4 @@
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);
=======================================
--- /branches/bleeding_edge/tools/generate-runtime-tests.py Fri Jun 13
12:19:04 2014 UTC
+++ /branches/bleeding_edge/tools/generate-runtime-tests.py Mon Jun 23
09:02:16 2014 UTC
@@ -158,8 +158,7 @@
"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],
--
--
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/d/optout.
