Skip to content

Commit

Permalink
deps: cherry-pick 37a3a15c3 from V8 upstream
Browse files Browse the repository at this point in the history
Original commit message:
  [api] Intercept DefineProperty after Descriptor query

  Analog to other interceptors, intercept the DefineProperty
  call only after obtaining the property descriptor.

  This behavior allows us to mirror calls on a sandboxed object
  as it is needed in Node. See for example
  nodejs#13265

  Bug:
  Change-Id: I73b8f8908d13473939b37fb6727858d0bee6bda3
  Reviewed-on: https://chromium-review.googlesource.com/725295
  Reviewed-by: Andreas Haas <ahaas@chromium.org>
  Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
  Cr-Commit-Position: refs/heads/master@{nodejs#48683}

PR-URL: nodejs#16294
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
fhinkel authored and targos committed Dec 6, 2017
1 parent 30274b2 commit 3038d53
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 16 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.0',
'v8_embedder_string': '-node.1',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/lookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class V8_EXPORT_PRIVATE LookupIterator final BASE_EMBEDDED {
kInterceptor = 1 << 0,
kPrototypeChain = 1 << 1,

// Convience combinations of bits.
// Convenience combinations of bits.
OWN_SKIP_INTERCEPTOR = 0,
OWN = kInterceptor,
PROTOTYPE_CHAIN_SKIP_INTERCEPTOR = kPrototypeChain,
Expand Down
25 changes: 14 additions & 11 deletions deps/v8/src/objects.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6828,17 +6828,6 @@ Maybe<bool> JSReceiver::OrdinaryDefineOwnProperty(Isolate* isolate,
it.Next();
}

// Handle interceptor
if (it.state() == LookupIterator::INTERCEPTOR) {
if (it.HolderIsReceiverOrHiddenPrototype()) {
Maybe<bool> result = DefinePropertyWithInterceptorInternal(
&it, it.GetInterceptor(), should_throw, *desc);
if (result.IsNothing() || result.FromJust()) {
return result;
}
}
}

return OrdinaryDefineOwnProperty(&it, desc, should_throw);
}

Expand All @@ -6854,6 +6843,20 @@ Maybe<bool> JSReceiver::OrdinaryDefineOwnProperty(LookupIterator* it,
PropertyDescriptor current;
MAYBE_RETURN(GetOwnPropertyDescriptor(it, &current), Nothing<bool>());

it->Restart();
// Handle interceptor
for (; it->IsFound(); it->Next()) {
if (it->state() == LookupIterator::INTERCEPTOR) {
if (it->HolderIsReceiverOrHiddenPrototype()) {
Maybe<bool> result = DefinePropertyWithInterceptorInternal(
it, it->GetInterceptor(), should_throw, *desc);
if (result.IsNothing() || result.FromJust()) {
return result;
}
}
}
}

// TODO(jkummerow/verwaest): It would be nice if we didn't have to reset
// the iterator every time. Currently, the reasons why we need it are:
// - handle interceptors correctly
Expand Down
7 changes: 4 additions & 3 deletions deps/v8/test/cctest/test-api-interceptors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -716,20 +716,21 @@ bool define_was_called_in_order = false;
void GetterCallbackOrder(Local<Name> property,
const v8::PropertyCallbackInfo<v8::Value>& info) {
get_was_called_in_order = true;
CHECK(define_was_called_in_order);
CHECK(!define_was_called_in_order);
info.GetReturnValue().Set(property);
}

void DefinerCallbackOrder(Local<Name> property,
const v8::PropertyDescriptor& desc,
const v8::PropertyCallbackInfo<v8::Value>& info) {
CHECK(!get_was_called_in_order); // Define called before get.
// Get called before DefineProperty because we query the descriptor first.
CHECK(get_was_called_in_order);
define_was_called_in_order = true;
}

} // namespace

// Check that definer callback is called before getter callback.
// Check that getter callback is called before definer callback.
THREADED_TEST(DefinerCallbackGetAndDefine) {
v8::HandleScope scope(CcTest::isolate());
v8::Local<v8::FunctionTemplate> templ =
Expand Down

0 comments on commit 3038d53

Please sign in to comment.