src: make cross-context MakeCallback() calls work

Check that invoking a callback on a receiver from a different context works. It ran afoul of an `env->context() == isolate->GetCurrentContext()` assertion so retrieve the environment from the callback context and the context to enter from the environment's context() method. We could also have retrieved the environment from the receiver's context and that would have made little practical difference. It just seemed more correct to get it from the callback context because that is the actual execution context. PR-URL: #9221 Reviewed-By: Anna Henningsen <[email protected]>

Author: bnoordhuis

src/node.cc

@@ -1304,45 +1304,48 @@ Local<Value> MakeCallback(Environment* env,
 
 
 Local<Value> MakeCallback(Isolate* isolate,
-Local<Object> recv,
-constchar* method,
-int argc,
-Local<Value> argv[]){
+ Local<Object> recv,
+constchar* method,
+int argc,
+ Local<Value> argv[]){
  EscapableHandleScope handle_scope(isolate);
- Local<Context> context = recv->CreationContext();
- Environment* env = Environment::GetCurrent(context);
- Context::Scope context_scope(context);
+ Local<String> method_string = OneByteString(isolate, method);
 return handle_scope.Escape(
-Local<Value>::New(isolate, MakeCallback(env, recv, method, argc, argv)));
+MakeCallback(isolate, recv, method_string, argc, argv));
 }
 
 
 Local<Value> MakeCallback(Isolate* isolate,
-Local<Object> recv,
-Local<String> symbol,
-int argc,
-Local<Value> argv[]){
+ Local<Object> recv,
+ Local<String> symbol,
+int argc,
+ Local<Value> argv[]){
  EscapableHandleScope handle_scope(isolate);
- Local<Context> context = recv->CreationContext();
-Environment* env = Environment::GetCurrent(context);
-Context::Scope context_scope(context);
-return handle_scope.Escape(
- Local<Value>::New(isolate, MakeCallback(env, recv, symbol, argc, argv)));
+ Local<Value> callback_v = recv->Get(symbol);
+if (callback_v.IsEmpty()) return Local<Value>();
+if (!callback_v->IsFunction()) return Local<Value>();
+Local<Function> callback = callback_v.As<Function>();
+return handle_scope.Escape(MakeCallback(isolate, recv, callback, argc, argv));
 }
 
 
 Local<Value> MakeCallback(Isolate* isolate,
- Local<Object> recv,
- Local<Function> callback,
-int argc,
- Local<Value> argv[]){
+ Local<Object> recv,
+ Local<Function> callback,
+int argc,
+ Local<Value> argv[]){
+// Observe the following two subtleties:
+//
+// 1. The environment is retrieved from the callback function's context.
+// 2. The context to enter is retrieved from the environment.
+//
+// Because of the AssignToContext() call in src/node_contextify.cc,
+// the two contexts need not be the same.
  EscapableHandleScope handle_scope(isolate);
- Local<Context> context = recv->CreationContext();
- Environment* env = Environment::GetCurrent(context);
- Context::Scope context_scope(context);
-return handle_scope.Escape(Local<Value>::New(
- isolate,
-MakeCallback(env, recv.As<Value>(), callback, argc, argv)));
+ Environment* env = Environment::GetCurrent(callback->CreationContext());
+ Context::Scope context_scope(env->context());
+return handle_scope.Escape(
+MakeCallback(env, recv.As<Value>(), callback, argc, argv));
 }
 
 

test/addons/make-callback/test.js

@@ -36,6 +36,10 @@ const recv ={
 assert.strictEqual(42,makeCallback(recv,'one'));
 assert.strictEqual(42,makeCallback(recv,'two',1337));
 
+// Check that callbacks on a receiver from a different context works.
+constforeignObject=vm.runInNewContext('({fortytwo(){return 42} })');
+assert.strictEqual(42,makeCallback(foreignObject,'fortytwo'));
+
 // Check that the callback is made in the context of the receiver.
 consttarget=vm.runInNewContext(`
  (function($Object){