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

Unified Diff: src/objects.cc

Issue 2397603003: [runtime] Let native setters have a return value. (Closed)
Patch Set: Ouch. Created 4 years, 2 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 | « src/api-arguments.h ('k') | test/cctest/heap/test-alloc.cc » ('j') | 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 9e8b06c6fc467e8ca7668d039df96b29a9f9c4eb..d299dc86065cb91a945a2d92e1d5e5b5215f8036 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1417,12 +1417,20 @@ Maybe<bool> Object::SetPropertyWithAccessor(LookupIterator* it,
return Nothing<bool>();
}
- v8::AccessorNameSetterCallback call_fun =
- v8::ToCData<v8::AccessorNameSetterCallback>(info->setter());
- // TODO(verwaest): We should not get here anymore once all AccessorInfos are
- // marked as special_data_property. They cannot both be writable and not
- // have a setter.
- if (call_fun == nullptr) return Just(true);
+ // The actual type of call_fun is either v8::AccessorNameSetterCallback or
+ // i::Accesors::AccessorNameBooleanSetterCallback, depending on whether the
+ // AccessorInfo was created by the API or internally (see accessors.cc).
+ // Here we handle both cases using GenericNamedPropertySetterCallback and
+ // its Call method.
+ GenericNamedPropertySetterCallback call_fun =
+ v8::ToCData<GenericNamedPropertySetterCallback>(info->setter());
+
+ if (call_fun == nullptr) {
+ // TODO(verwaest): We should not get here anymore once all AccessorInfos
+ // are marked as special_data_property. They cannot both be writable and
+ // not have a setter.
+ return Just(true);
+ }
if (info->is_sloppy() && !receiver->IsJSReceiver()) {
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
@@ -1432,9 +1440,15 @@ Maybe<bool> Object::SetPropertyWithAccessor(LookupIterator* it,
PropertyCallbackArguments args(isolate, info->data(), *receiver, *holder,
should_throw);
- args.Call(call_fun, name, value);
+ Handle<Object> result = args.Call(call_fun, name, value);
+ // In the case of AccessorNameSetterCallback, we know that the result value
+ // cannot have been set, so the result of Call will be null. In the case of
+ // AccessorNameBooleanSetterCallback, the result will either be null
+ // (signalling an exception) or a boolean Oddball.
RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate, Nothing<bool>());
- return Just(true);
+ if (result.is_null()) return Just(true);
+ DCHECK(result->BooleanValue() || should_throw == DONT_THROW);
+ return Just(result->BooleanValue());
}
// Regular accessor.
@@ -5781,17 +5795,10 @@ Maybe<bool> JSObject::DefineOwnPropertyIgnoreAttributes(
it->TransitionToAccessorPair(accessors, attributes);
}
- Maybe<bool> result =
- JSObject::SetPropertyWithAccessor(it, value, should_throw);
-
- if (current_attributes == attributes || result.IsNothing()) {
- return result;
- }
-
- } else {
- it->ReconfigureDataProperty(value, attributes);
+ return JSObject::SetPropertyWithAccessor(it, value, should_throw);
}
+ it->ReconfigureDataProperty(value, attributes);
return Just(true);
}
case LookupIterator::INTEGER_INDEXED_EXOTIC:
« no previous file with comments | « src/api-arguments.h ('k') | test/cctest/heap/test-alloc.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698