Plumb accessing context through to access control callbacks

BUG=none
LOG=n
R=verwaest@chromium.org

Review URL: https://codereview.chromium.org/1410883006

Cr-Commit-Position: refs/heads/master@{#31495}

include/v8.h

@@ -4251,6 +4251,14 @@ enum AccessType {
 };
 
 
+/**
+ * Returns true if the given context should be allowed to access the given
+ * object.
+ */
+typedef bool (*AccessCheckCallback)(Local<Context> accessing_context,
+                                    Local<Object> accessed_object);
+
+
 /**
  * Returns true if cross-context access should be allowed to the named
  * property with the given key on the host object.
@@ -4659,16 +4667,20 @@ class V8_EXPORT ObjectTemplate : public Template {
   void MarkAsUndetectable();
 
   /**
-   * Sets access check callbacks on the object template and enables
-   * access checks.
+   * Sets access check callback on the object template and enables access
+   * checks.
    *
    * When accessing properties on instances of this object template,
    * the access check callback will be called to determine whether or
    * not to allow cross-context access to the properties.
    */
-  void SetAccessCheckCallbacks(NamedSecurityCallback named_handler,
-                               IndexedSecurityCallback indexed_handler,
-                               Local<Value> data = Local<Value>());
+  void SetAccessCheckCallback(AccessCheckCallback callback);
+
+  V8_DEPRECATE_SOON(
+      "Use SetAccessCheckCallback instead",
+      void SetAccessCheckCallbacks(NamedSecurityCallback named_handler,
+                                   IndexedSecurityCallback indexed_handler,
+                                   Local<Value> data = Local<Value>()));
 
   /**
    * Gets the number of internal fields for objects generated from

src/api.cc

@@ -1452,6 +1452,29 @@ void ObjectTemplate::MarkAsUndetectable() {
 }
 
 
+void ObjectTemplate::SetAccessCheckCallback(AccessCheckCallback callback) {
+  i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
+  ENTER_V8(isolate);
+  i::HandleScope scope(isolate);
+  auto cons = EnsureConstructor(isolate, this);
+  EnsureNotInstantiated(cons, "v8::ObjectTemplate::SetAccessCheckCallback");
+
+  i::Handle<i::Struct> struct_info =
+      isolate->factory()->NewStruct(i::ACCESS_CHECK_INFO_TYPE);
+  i::Handle<i::AccessCheckInfo> info =
+      i::Handle<i::AccessCheckInfo>::cast(struct_info);
+
+  SET_FIELD_WRAPPED(info, set_callback, callback);
+  SET_FIELD_WRAPPED(info, set_named_callback, nullptr);
+  SET_FIELD_WRAPPED(info, set_indexed_callback, nullptr);
+
+  info->set_data(*isolate->factory()->undefined_value());
+
+  cons->set_access_check_info(*info);
+  cons->set_needs_access_check(true);
+}
+
+
 void ObjectTemplate::SetAccessCheckCallbacks(
     NamedSecurityCallback named_callback,
     IndexedSecurityCallback indexed_callback, Local<Value> data) {
@@ -1466,6 +1489,7 @@ void ObjectTemplate::SetAccessCheckCallbacks(
   i::Handle<i::AccessCheckInfo> info =
       i::Handle<i::AccessCheckInfo>::cast(struct_info);
 
+  SET_FIELD_WRAPPED(info, set_callback, nullptr);
   SET_FIELD_WRAPPED(info, set_named_callback, named_callback);
   SET_FIELD_WRAPPED(info, set_indexed_callback, indexed_callback);
 

src/isolate.cc

@@ -812,27 +812,33 @@ bool Isolate::MayAccess(Handle<Context> accessing_context,
 
   HandleScope scope(this);
   Handle<Object> data;
-  v8::NamedSecurityCallback callback;
+  v8::AccessCheckCallback callback = nullptr;
+  v8::NamedSecurityCallback named_callback = nullptr;
   { DisallowHeapAllocation no_gc;
     AccessCheckInfo* access_check_info = GetAccessCheckInfo(this, receiver);
     if (!access_check_info) return false;
-    Object* fun_obj = access_check_info->named_callback();
-    callback = v8::ToCData<v8::NamedSecurityCallback>(fun_obj);
-    if (!callback) return false;
-    data = handle(access_check_info->data(), this);
+    Object* fun_obj = access_check_info->callback();
+    callback = v8::ToCData<v8::AccessCheckCallback>(fun_obj);
+    if (!callback) {
+      fun_obj = access_check_info->named_callback();
+      named_callback = v8::ToCData<v8::NamedSecurityCallback>(fun_obj);
+      if (!named_callback) return false;
+      data = handle(access_check_info->data(), this);
+    }
   }
 
   LOG(this, ApiSecurityCheck());
 
   {
-    SaveContext save(this);
-    set_context(accessing_context->native_context());
-
     // Leaving JavaScript.
     VMState<EXTERNAL> state(this);
+    if (callback) {
+      return callback(v8::Utils::ToLocal(accessing_context),
+                      v8::Utils::ToLocal(receiver));
+    }
     Handle<Object> key = factory()->undefined_value();
-    return callback(v8::Utils::ToLocal(receiver), v8::Utils::ToLocal(key),
-                    v8::ACCESS_HAS, v8::Utils::ToLocal(data));
+    return named_callback(v8::Utils::ToLocal(receiver), v8::Utils::ToLocal(key),
+                          v8::ACCESS_HAS, v8::Utils::ToLocal(data));
   }
 }
 

src/objects-debug.cc

@@ -929,6 +929,7 @@ void AccessCheckInfo::AccessCheckInfoVerify() {
   CHECK(IsAccessCheckInfo());
   VerifyPointer(named_callback());
   VerifyPointer(indexed_callback());
+  VerifyPointer(callback());
   VerifyPointer(data());
 }
 

src/objects-inl.h

@@ -5659,6 +5659,7 @@ ACCESSORS(AccessorPair, setter, Object, kSetterOffset)
 
 ACCESSORS(AccessCheckInfo, named_callback, Object, kNamedCallbackOffset)
 ACCESSORS(AccessCheckInfo, indexed_callback, Object, kIndexedCallbackOffset)
+ACCESSORS(AccessCheckInfo, callback, Object, kCallbackOffset)
 ACCESSORS(AccessCheckInfo, data, Object, kDataOffset)
 
 ACCESSORS(InterceptorInfo, getter, Object, kGetterOffset)

src/objects-printer.cc

@@ -1015,6 +1015,7 @@ void AccessCheckInfo::AccessCheckInfoPrint(std::ostream& os) {  // NOLINT
   HeapObject::PrintHeader(os, "AccessCheckInfo");
   os << "\n - named_callback: " << Brief(named_callback());
   os << "\n - indexed_callback: " << Brief(indexed_callback());
+  os << "\n - callback: " << Brief(callback());
   os << "\n - data: " << Brief(data());
   os << "\n";
 }

src/objects.h

@@ -10317,6 +10317,7 @@ class AccessCheckInfo: public Struct {
  public:
   DECL_ACCESSORS(named_callback, Object)
   DECL_ACCESSORS(indexed_callback, Object)
+  DECL_ACCESSORS(callback, Object)
   DECL_ACCESSORS(data, Object)
 
   DECLARE_CAST(AccessCheckInfo)
@@ -10327,7 +10328,8 @@ class AccessCheckInfo: public Struct {
 
   static const int kNamedCallbackOffset   = HeapObject::kHeaderSize;
   static const int kIndexedCallbackOffset = kNamedCallbackOffset + kPointerSize;
-  static const int kDataOffset = kIndexedCallbackOffset + kPointerSize;
+  static const int kCallbackOffset = kIndexedCallbackOffset + kPointerSize;
+  static const int kDataOffset = kCallbackOffset + kPointerSize;
   static const int kSize = kDataOffset + kPointerSize;
 
  private:

test/cctest/test-accessors.cc

@@ -610,8 +610,8 @@ THREADED_TEST(Regress433458) {
 static bool security_check_value = false;
 
 
-static bool SecurityTestCallback(Local<v8::Object> global, Local<Value> name,
-                                 v8::AccessType type, Local<Value> data) {
+static bool SecurityTestCallback(Local<v8::Context> accessing_context,
+                                 Local<v8::Object> accessed_object) {
   return security_check_value;
 }
 
@@ -627,7 +627,7 @@ TEST(PrototypeGetterAccessCheck) {
   fun_templ->InstanceTemplate()->SetAccessorProperty(v8_str("foo"),
                                                      getter_templ);
   auto obj_templ = v8::ObjectTemplate::New(isolate);
-  obj_templ->SetAccessCheckCallbacks(SecurityTestCallback, nullptr);
+  obj_templ->SetAccessCheckCallback(SecurityTestCallback);
   env->Global()->Set(v8_str("Fun"), fun_templ->GetFunction());
   env->Global()->Set(v8_str("obj"), obj_templ->NewInstance());
   env->Global()->Set(v8_str("obj2"), obj_templ->NewInstance());

test/cctest/test-api-interceptors.cc

@@ -1660,8 +1660,8 @@ THREADED_TEST(IndexedInterceptorWithNoSetter) {
 }
 
 
-static bool AccessAlwaysBlocked(Local<v8::Object> global, Local<Value> name,
-                                v8::AccessType type, Local<Value> data) {
+static bool AccessAlwaysBlocked(Local<v8::Context> accessing_context,
+                                Local<v8::Object> accessed_object) {
   return false;
 }
 
@@ -1673,7 +1673,7 @@ THREADED_TEST(IndexedInterceptorWithAccessorCheck) {
   templ->SetHandler(
       v8::IndexedPropertyHandlerConfiguration(IdentityIndexedPropertyGetter));
 
-  templ->SetAccessCheckCallbacks(AccessAlwaysBlocked, nullptr);
+  templ->SetAccessCheckCallback(AccessAlwaysBlocked);
 
   LocalContext context;
   Local<v8::Object> obj = templ->NewInstance();
@@ -2907,12 +2907,13 @@ struct AccessCheckData {
   bool result;
 };
 
+AccessCheckData* g_access_check_data = nullptr;
 
-bool SimpleAccessChecker(Local<v8::Object> global, Local<Value> name,
-                         v8::AccessType type, Local<Value> data) {
-  auto access_check_data = GetWrappedObject<AccessCheckData>(data);
-  access_check_data->count++;
-  return access_check_data->result;
+
+bool SimpleAccessChecker(Local<v8::Context> accessing_context,
+                         Local<v8::Object> access_object) {
+  g_access_check_data->count++;
+  return g_access_check_data->result;
 }
 
 
@@ -2944,7 +2945,7 @@ void ShouldIndexedInterceptor(uint32_t,
 }  // namespace
 
 
-THREADED_TEST(NamedAllCanReadInterceptor) {
+TEST(NamedAllCanReadInterceptor) {
   auto isolate = CcTest::isolate();
   v8::HandleScope handle_scope(isolate);
   LocalContext context;
@@ -2953,6 +2954,8 @@ THREADED_TEST(NamedAllCanReadInterceptor) {
   access_check_data.result = true;
   access_check_data.count = 0;
 
+  g_access_check_data = &access_check_data;
+
   ShouldInterceptData intercept_data_0;
   intercept_data_0.value = 239;
   intercept_data_0.should_intercept = true;
@@ -2980,9 +2983,7 @@ THREADED_TEST(NamedAllCanReadInterceptor) {
   }
 
   auto checked = v8::ObjectTemplate::New(isolate);
-  checked->SetAccessCheckCallbacks(
-      SimpleAccessChecker, nullptr,
-      BuildWrappedObject<AccessCheckData>(isolate, &access_check_data));
+  checked->SetAccessCheckCallback(SimpleAccessChecker);
 
   context->Global()->Set(v8_str("intercepted_0"), intercepted_0->NewInstance());
   context->Global()->Set(v8_str("intercepted_1"), intercepted_1->NewInstance());
@@ -3017,10 +3018,11 @@ THREADED_TEST(NamedAllCanReadInterceptor) {
     CHECK(try_catch.HasCaught());
   }
   CHECK_EQ(9, access_check_data.count);
+  g_access_check_data = nullptr;
 }
 
 
-THREADED_TEST(IndexedAllCanReadInterceptor) {
+TEST(IndexedAllCanReadInterceptor) {
   auto isolate = CcTest::isolate();
   v8::HandleScope handle_scope(isolate);
   LocalContext context;
@@ -3029,6 +3031,8 @@ THREADED_TEST(IndexedAllCanReadInterceptor) {
   access_check_data.result = true;
   access_check_data.count = 0;
 
+  g_access_check_data = &access_check_data;
+
   ShouldInterceptData intercept_data_0;
   intercept_data_0.value = 239;
   intercept_data_0.should_intercept = true;
@@ -3056,9 +3060,7 @@ THREADED_TEST(IndexedAllCanReadInterceptor) {
   }
 
   auto checked = v8::ObjectTemplate::New(isolate);
-  checked->SetAccessCheckCallbacks(
-      SimpleAccessChecker, nullptr,
-      BuildWrappedObject<AccessCheckData>(isolate, &access_check_data));
+  checked->SetAccessCheckCallback(SimpleAccessChecker);
 
   context->Global()->Set(v8_str("intercepted_0"), intercepted_0->NewInstance());
   context->Global()->Set(v8_str("intercepted_1"), intercepted_1->NewInstance());
@@ -3094,6 +3096,8 @@ THREADED_TEST(IndexedAllCanReadInterceptor) {
     CHECK(try_catch.HasCaught());
   }
   CHECK_EQ(9, access_check_data.count);
+
+  g_access_check_data = nullptr;
 }
 
 

test/cctest/test-object-observe.cc

@@ -988,10 +988,8 @@ TEST(UseCountObjectGetNotifier) {
 }
 
 
-static bool NamedAccessCheckAlwaysAllow(Local<v8::Object> global,
-                                        Local<v8::Value> name,
-                                        v8::AccessType type,
-                                        Local<Value> data) {
+static bool NamedAccessCheckAlwaysAllow(Local<v8::Context> accessing_context,
+                                        Local<v8::Object> accessed_object) {
   return true;
 }
 
@@ -1002,7 +1000,7 @@ TEST(DisallowObserveAccessCheckedObject) {
   LocalContext env;
   v8::Local<v8::ObjectTemplate> object_template =
       v8::ObjectTemplate::New(isolate);
-  object_template->SetAccessCheckCallbacks(NamedAccessCheckAlwaysAllow, NULL);
+  object_template->SetAccessCheckCallback(NamedAccessCheckAlwaysAllow);
   Local<Object> new_instance =
       object_template->NewInstance(
                          v8::Isolate::GetCurrent()->GetCurrentContext())
@@ -1023,7 +1021,7 @@ TEST(DisallowGetNotifierAccessCheckedObject) {
   LocalContext env;
   v8::Local<v8::ObjectTemplate> object_template =
       v8::ObjectTemplate::New(isolate);
-  object_template->SetAccessCheckCallbacks(NamedAccessCheckAlwaysAllow, NULL);
+  object_template->SetAccessCheckCallback(NamedAccessCheckAlwaysAllow);
   Local<Object> new_instance =
       object_template->NewInstance(
                          v8::Isolate::GetCurrent()->GetCurrentContext())