From 0b690a9ce3a10aa68aefbb50398758e52f50a331 Mon Sep 17 00:00:00 2001
From: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Date: Wed, 18 Oct 2017 16:30:34 +0200
Subject: [PATCH] deps: cherry-pick 37a3a15c3 from V8 upstream
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
  https://github.com/nodejs/node/pull/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@{#48683}

PR-URL: https://github.com/nodejs/node/pull/16294
Backport-PR-URL: https://github.com/nodejs/node/pull/16413
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>
---
 deps/v8/src/lookup.h                         |  2 +-
 deps/v8/src/objects.cc                       | 25 +++++++++++---------
 deps/v8/test/cctest/test-api-interceptors.cc |  7 +++---
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/deps/v8/src/lookup.h b/deps/v8/src/lookup.h
index 25c5a6cc3bf0c5..9ea2d77cf6067e 100644
--- a/deps/v8/src/lookup.h
+++ b/deps/v8/src/lookup.h
@@ -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,
diff --git a/deps/v8/src/objects.cc b/deps/v8/src/objects.cc
index c1a2e41bf17003..28c1cd681ffd46 100644
--- a/deps/v8/src/objects.cc
+++ b/deps/v8/src/objects.cc
@@ -6713,17 +6713,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);
 }
 
@@ -6739,6 +6728,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
diff --git a/deps/v8/test/cctest/test-api-interceptors.cc b/deps/v8/test/cctest/test-api-interceptors.cc
index 265698d131fcd8..ca9b18016d9d25 100644
--- a/deps/v8/test/cctest/test-api-interceptors.cc
+++ b/deps/v8/test/cctest/test-api-interceptors.cc
@@ -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 =