Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(187)

Unified Diff: src/objects.cc

Issue 1177103003: Use LookupIterator for elements in the observed part of DefineAccessor (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Extend LookupIterator for simplification and bugfixing Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 400be3edaebb01269d94481d7b0e7c59c2043e3e..48becb50dc5567a1aa047a2ba5d68505f5c3ef51 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -6371,21 +6371,6 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
Handle<Object> setter,
PropertyAttributes attributes) {
Isolate* isolate = object->GetIsolate();
- // Check access rights if needed.
- if (object->IsAccessCheckNeeded() && !isolate->MayAccess(object)) {
- isolate->ReportFailedAccessCheck(object);
- RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
- return isolate->factory()->undefined_value();
- }
-
- if (object->IsJSGlobalProxy()) {
- PrototypeIterator iter(isolate, object);
- if (iter.IsAtEnd()) return isolate->factory()->undefined_value();
- DCHECK(PrototypeIterator::GetCurrent(iter)->IsJSGlobalObject());
- DefineAccessor(Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter)),
- name, getter, setter, attributes);
- return isolate->factory()->undefined_value();
- }
// Make sure that the top context does not change when doing callbacks or
// interceptor calls.
@@ -6395,39 +6380,36 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));
uint32_t index = 0;
- bool is_element = name->AsArrayIndex(&index);
+ LookupIterator::Configuration c = LookupIterator::HIDDEN_SKIP_INTERCEPTOR;
+ LookupIterator it = name->AsArrayIndex(&index)
+ ? LookupIterator(isolate, object, index, c)
+ : LookupIterator(object, name, c);
+
+ if (it.state() == LookupIterator::ACCESS_CHECK) {
+ if (!it.HasAccess()) {
+ isolate->ReportFailedAccessCheck(it.GetHolder<JSObject>());
+ RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
+ return isolate->factory()->undefined_value();
+ }
+ it.Next();
+ }
Handle<Object> old_value = isolate->factory()->the_hole_value();
bool is_observed = object->map()->is_observed() &&
!isolate->IsInternallyUsedPropertyName(name);
bool preexists = false;
if (is_observed) {
- if (is_element) {
- Maybe<bool> maybe = HasOwnElement(object, index);
- // Workaround for a GCC 4.4.3 bug which leads to "‘preexists’ may be used
- // uninitialized in this function".
- if (!maybe.IsJust()) {
- DCHECK(false);
- return isolate->factory()->undefined_value();
- }
- preexists = maybe.FromJust();
- if (preexists && GetOwnElementAccessorPair(object, index).is_null()) {
- old_value =
- Object::GetElement(isolate, object, index).ToHandleChecked();
- }
- } else {
- LookupIterator it(object, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);
- CHECK(GetPropertyAttributes(&it).IsJust());
- preexists = it.IsFound();
- if (preexists && (it.state() == LookupIterator::DATA ||
- it.GetAccessors()->IsAccessorInfo())) {
- old_value = GetProperty(&it).ToHandleChecked();
- }
+ CHECK(GetPropertyAttributes(&it).IsJust());
+ preexists = it.IsFound();
+ if (preexists && (it.state() == LookupIterator::DATA ||
+ it.GetAccessors()->IsAccessorInfo())) {
+ old_value = GetProperty(&it).ToHandleChecked();
}
}
- if (is_element) {
- DefineElementAccessor(object, index, getter, setter, attributes);
+ if (it.IsElement()) {
+ DefineElementAccessor(it.GetStoreTarget(), index, getter, setter,
+ attributes);
} else {
DCHECK(getter->IsSpecFunction() || getter->IsUndefined() ||
getter->IsNull());
@@ -6435,11 +6417,6 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
setter->IsNull());
// At least one of the accessors needs to be a new value.
DCHECK(!getter->IsNull() || !setter->IsNull());
- LookupIterator it(object, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
- if (it.state() == LookupIterator::ACCESS_CHECK) {
- // We already did an access check before. We do have access.
- it.Next();
- }
if (!getter->IsNull()) {
it.TransitionToAccessorProperty(ACCESSOR_GETTER, getter, attributes);
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698