Fix receiver checks for v8::Function on a remote context receiver.

v8 allows the embedder to specify a global template to use when
creating a new context. However, v8 does not use the supplied
template directly when creating the global proxy: it creates a
unique template for each global proxy. However, this is problematic
for remote contexts: functions cannot use strict receiver checks
with the remote context, as the global template will never match
the global proxy.

To fix this, remote contexts now also include a remote global
object in the prototype chain that is instantiated with the global
template. This mirrors the way the global proxy is configured for a
full v8 context, and allows strict receiver checks to work.

BUG=527190

Review-Url: https://codereview.chromium.org/2677653002
Cr-Commit-Position: refs/heads/master@{#43361}

src/api.cc

@@ -4504,33 +4504,6 @@ bool v8::Object::SetPrototype(Local<Value> value) {
   return SetPrototype(context, value).FromMaybe(false);
 }
 
-static bool HasInstanceInGlobalProxy(i::JSGlobalProxy* global_proxy,
-                                     i::FunctionTemplateInfo* target_template) {
-  auto* constructor_object = global_proxy->map()->GetConstructor();
-  if (!constructor_object->IsJSFunction()) return false;
-
-  auto* constructor = i::JSFunction::cast(constructor_object);
-  if (!constructor->shared()->function_data()->IsFunctionTemplateInfo())
-    return false;
-
-  auto* proxy_constructor_template =
-      i::FunctionTemplateInfo::cast(constructor->shared()->function_data());
-  if (!proxy_constructor_template->prototype_template()->IsObjectTemplateInfo())
-    return false;
-
-  auto* global_template = i::ObjectTemplateInfo::cast(
-      proxy_constructor_template->prototype_template());
-  // Iterate through the chain of inheriting function templates to
-  // see if the required one occurs.
-  for (i::Object* type = global_template->constructor();
-       type->IsFunctionTemplateInfo();
-       type = i::FunctionTemplateInfo::cast(type)->parent_template()) {
-    if (type == target_template) return true;
-  }
-  // Didn't find the required type in the inheritance chain.
-  return false;
-}
-
 Local<Object> v8::Object::FindInstanceInPrototypeChain(
     v8::Local<FunctionTemplate> tmpl) {
   auto self = Utils::OpenHandle(this);
@@ -4539,16 +4512,7 @@ Local<Object> v8::Object::FindInstanceInPrototypeChain(
   auto tmpl_info = *Utils::OpenHandle(*tmpl);
   while (!tmpl_info->IsTemplateFor(iter.GetCurrent<i::JSObject>())) {
     iter.Advance();
-    if (iter.IsAtEnd()) {
-      // Normally, a standard prototype walk is sufficient; however, global
-      // proxies aren't directly constructed with the supplied template.
-      // Normally, this is not a problem, because the prototype chain includes
-      // the global object; however, a remote context has no global object.
-      if (self->IsJSGlobalProxy() &&
-          HasInstanceInGlobalProxy(i::JSGlobalProxy::cast(*self), tmpl_info))
-        return Utils::ToLocal(self);
-      return Local<Object>();
-    }
+    if (iter.IsAtEnd()) return Local<Object>();
     if (!iter.GetCurrent()->IsJSObject()) return Local<Object>();
   }
   // IsTemplateFor() ensures that iter.GetCurrent() can't be a Proxy here.
@@ -6615,12 +6579,13 @@ bool FunctionTemplate::HasInstance(v8::Local<v8::Value> value) {
     return true;
   }
   if (obj->IsJSGlobalProxy()) {
-    auto* global_proxy = i::JSGlobalProxy::cast(*obj);
-    // For global proxies, check the constructor's prototype instead. Remote
-    // global proxies have no global object to perform instance checks on, but
-    // the constructor's prototype's constructor corresponds to the original
-    // template used to create the context.
-    return HasInstanceInGlobalProxy(global_proxy, *self);
+    // If it's a global proxy, then test with the global object. Note that the
+    // inner global object may not necessarily be a JSGlobalObject.
+    i::PrototypeIterator iter(i::JSObject::cast(*obj)->map());
+    // The global proxy should always have a prototype, as it is a bug to call
+    // this on a detached JSGlobalProxy.
+    DCHECK(!iter.IsAtEnd());
+    return self->IsTemplateFor(iter.GetCurrent<i::JSObject>());
   }
   return false;
 }

src/bootstrapper.cc

@@ -4927,11 +4927,19 @@ Genesis::Genesis(Isolate* isolate,
     global_proxy = factory()->NewUninitializedJSGlobalProxy(proxy_size);
   }
 
-  // CreateNewGlobals.
+  // Create a remote object as the global object.
   Handle<ObjectTemplateInfo> global_proxy_data =
-      v8::Utils::OpenHandle(*global_proxy_template);
+      Utils::OpenHandle(*global_proxy_template);
   Handle<FunctionTemplateInfo> global_constructor(
       FunctionTemplateInfo::cast(global_proxy_data->constructor()));
+
+  Handle<ObjectTemplateInfo> global_object_template(
+      ObjectTemplateInfo::cast(global_constructor->prototype_template()));
+  Handle<JSObject> global_object =
+      ApiNatives::InstantiateRemoteObject(
+          global_object_template).ToHandleChecked();
+
+  // (Re)initialize the global proxy object.
   Handle<SharedFunctionInfo> shared =
       FunctionTemplateInfo::GetOrCreateSharedFunctionInfo(isolate,
                                                           global_constructor);
@@ -4955,11 +4963,14 @@ Genesis::Genesis(Isolate* isolate,
   global_proxy_function->shared()->set_instance_class_name(*global_name);
   factory()->ReinitializeJSGlobalProxy(global_proxy, global_proxy_function);
 
-  // GlobalProxy.
+  // A remote global proxy has no native context.
   global_proxy->set_native_context(heap()->null_value());
 
-  // DetachGlobal.
-  JSObject::ForceSetPrototype(global_proxy, factory()->null_value());
+  // Configure the hidden prototype chain of the global proxy.
+  JSObject::ForceSetPrototype(global_proxy, global_object);
+  // TODO(dcheng): This is a hack. Why does this need to be manually called
+  // here? Line 4812 should have taken care of it?
+  global_proxy->map()->set_has_hidden_prototype(true);
 
   global_proxy_ = global_proxy;
 }

test/cctest/test-access-checks.cc

@@ -102,6 +102,23 @@ void IndexedEnumerator(const v8::PropertyCallbackInfo<v8::Array>& info) {
   info.GetReturnValue().Set(names);
 }
 
+void MethodGetter(v8::Local<v8::Name> property,
+                  const v8::PropertyCallbackInfo<v8::Value>& info) {
+  v8::Isolate* isolate = info.GetIsolate();
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+
+  v8::Local<v8::External> data = info.Data().As<v8::External>();
+  v8::Local<v8::FunctionTemplate>& function_template =
+      *reinterpret_cast<v8::Local<v8::FunctionTemplate>*>(data->Value());
+
+  info.GetReturnValue().Set(
+      function_template->GetFunction(context).ToLocalChecked());
+}
+
+void MethodCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
+  info.GetReturnValue().Set(8);
+}
+
 void NamedGetterThrowsException(
     v8::Local<v8::Name> property,
     const v8::PropertyCallbackInfo<v8::Value>& info) {
@@ -288,6 +305,44 @@ TEST(AccessCheckWithInterceptor) {
   CheckCrossContextAccess(isolate, context1, context0->Global());
 }
 
+TEST(CallFunctionWithRemoteContextReceiver) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::FunctionTemplate> global_template =
+      v8::FunctionTemplate::New(isolate);
+
+  v8::Local<v8::Signature> signature =
+      v8::Signature::New(isolate, global_template);
+  v8::Local<v8::FunctionTemplate> function_template = v8::FunctionTemplate::New(
+      isolate, MethodCallback, v8::External::New(isolate, &function_template),
+      signature);
+
+  global_template->InstanceTemplate()->SetAccessCheckCallbackAndHandler(
+      AccessCheck, v8::NamedPropertyHandlerConfiguration(
+                       MethodGetter, nullptr, nullptr, nullptr, nullptr,
+                       v8::External::New(isolate, &function_template)),
+      v8::IndexedPropertyHandlerConfiguration());
+
+  v8::Local<v8::Object> accessed_object =
+      v8::Context::NewRemoteContext(isolate,
+                                    global_template->InstanceTemplate())
+          .ToLocalChecked();
+  v8::Local<v8::Context> accessing_context =
+      v8::Context::New(isolate, nullptr, global_template->InstanceTemplate());
+
+  v8::HandleScope handle_scope(isolate);
+  accessing_context->Global()
+      ->Set(accessing_context, v8_str("other"), accessed_object)
+      .FromJust();
+  v8::Context::Scope context_scope(accessing_context);
+
+  {
+    v8::TryCatch try_catch(isolate);
+    ExpectInt32("this.other.method()", 8);
+    CHECK(!try_catch.HasCaught());
+  }
+}
+
 TEST(AccessCheckWithExceptionThrowingInterceptor) {
   v8::Isolate* isolate = CcTest::isolate();
   isolate->SetFailedAccessCheckCallbackFunction([](v8::Local<v8::Object> target,

test/unittests/api/remote-object-unittest.cc

@@ -67,11 +67,6 @@ TEST_F(RemoteObjectTest, RemoteContextInstanceChecks) {
           .ToLocalChecked();
   EXPECT_TRUE(parent_template->HasInstance(remote_context));
   EXPECT_TRUE(constructor_template->HasInstance(remote_context));
-
-  EXPECT_EQ(remote_context,
-            remote_context->FindInstanceInPrototypeChain(parent_template));
-  EXPECT_EQ(remote_context,
-            remote_context->FindInstanceInPrototypeChain(constructor_template));
 }
 
 }  // namespace v8