Skip to content

Commit 73ee794

Browse files
verwaestCommit bot
verwaest
authored and
Commit bot
committed
Support subclassing API functions.
When instantiating a subclassed API function, the instance cache is avoided. There is currently no direct API yet to instantiate a Template while passing in a new.target. It probably makes sense to extend ObjectTemplate::NewInstance to accept a new.target, in line with Reflect.construct. BUG=v8:3330, v8:5001 Review-Url: https://codereview.chromium.org/1972613002 Cr-Commit-Position: refs/heads/master@{#36179}
1 parent 1721543 commit 73ee794

File tree

5 files changed

+167
-58
lines changed

5 files changed

+167
-58
lines changed

‎src/api-natives.cc‎

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace {
1717

1818
MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
1919
Handle<ObjectTemplateInfo> data,
20+
Handle<JSReceiver> new_target,
2021
bool is_hidden_prototype);
2122

2223
MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
@@ -31,7 +32,7 @@ MaybeHandle<Object> Instantiate(Isolate* isolate, Handle<Object> data,
3132
Handle<FunctionTemplateInfo>::cast(data), name);
3233
} else if (data->IsObjectTemplateInfo()) {
3334
return InstantiateObject(isolate, Handle<ObjectTemplateInfo>::cast(data),
34-
false);
35+
Handle<JSReceiver>(), false);
3536
} else {
3637
return data;
3738
}
@@ -288,11 +289,25 @@ void UncacheTemplateInstantiation(Isolate* isolate, uint32_t serial_number) {
288289

289290
MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
290291
Handle<ObjectTemplateInfo> info,
292+
Handle<JSReceiver> new_target,
291293
bool is_hidden_prototype) {
292-
// Fast path.
293-
Handle<JSObject> result;
294+
Handle<JSFunction> constructor;
294295
uint32_t serial_number =
295296
static_cast<uint32_t>(Smi::cast(info->serial_number())->value());
297+
if (!new_target.is_null()) {
298+
if (new_target->IsJSFunction() &&
299+
JSFunction::cast(*new_target)->shared()->function_data() ==
300+
info->constructor() &&
301+
JSFunction::cast(*new_target)->context()->native_context() ==
302+
isolate->context()->native_context()) {
303+
constructor = Handle<JSFunction>::cast(new_target);
304+
} else {
305+
// Disable caching for subclass instantiation.
306+
serial_number = 0;
307+
}
308+
}
309+
// Fast path.
310+
Handle<JSObject> result;
296311
if (serial_number) {
297312
// Probe cache.
298313
auto cache = isolate->template_instantiations_cache();
@@ -305,20 +320,27 @@ MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
305320
}
306321
// Enter a new scope. Recursion could otherwise create a lot of handles.
307322
HandleScope scope(isolate);
308-
auto constructor = handle(info->constructor(), isolate);
309-
Handle<JSFunction> cons;
310-
if (constructor->IsUndefined()) {
311-
cons = isolate->object_function();
312-
} else {
313-
auto cons_templ = Handle<FunctionTemplateInfo>::cast(constructor);
314-
ASSIGN_RETURN_ON_EXCEPTION(
315-
isolate, cons, InstantiateFunction(isolate, cons_templ), JSFunction);
323+
324+
if (constructor.is_null()) {
325+
Handle<Object> cons(info->constructor(), isolate);
326+
if (cons->IsUndefined()) {
327+
constructor = isolate->object_function();
328+
} else {
329+
auto cons_templ = Handle<FunctionTemplateInfo>::cast(cons);
330+
ASSIGN_RETURN_ON_EXCEPTION(isolate, constructor,
331+
InstantiateFunction(isolate, cons_templ),
332+
JSObject);
333+
}
334+
335+
if (new_target.is_null()) new_target = constructor;
316336
}
317-
auto object = isolate->factory()->NewJSObject(cons);
337+
338+
Handle<JSObject> object;
339+
ASSIGN_RETURN_ON_EXCEPTION(isolate, object,
340+
JSObject::New(constructor, new_target), JSObject);
318341
ASSIGN_RETURN_ON_EXCEPTION(
319342
isolate, result,
320-
ConfigureInstance(isolate, object, info, is_hidden_prototype),
321-
JSFunction);
343+
ConfigureInstance(isolate, object, info, is_hidden_prototype), JSObject);
322344
// TODO(dcarney): is this necessary?
323345
JSObject::MigrateSlowToFast(result, 0, "ApiNatives::InstantiateObject");
324346

@@ -356,7 +378,7 @@ MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
356378
isolate, prototype,
357379
InstantiateObject(isolate,
358380
Handle<ObjectTemplateInfo>::cast(prototype_templ),
359-
data->hidden_prototype()),
381+
Handle<JSReceiver>(), data->hidden_prototype()),
360382
JSFunction);
361383
}
362384
auto parent = handle(data->parent_template(), isolate);
@@ -448,12 +470,11 @@ MaybeHandle<JSFunction> ApiNatives::InstantiateFunction(
448470
return ::v8::internal::InstantiateFunction(isolate, data);
449471
}
450472

451-
452473
MaybeHandle<JSObject> ApiNatives::InstantiateObject(
453-
Handle<ObjectTemplateInfo> data) {
474+
Handle<ObjectTemplateInfo> data, Handle<JSReceiver> new_target) {
454475
Isolate* isolate = data->GetIsolate();
455476
InvokeScope invoke_scope(isolate);
456-
return ::v8::internal::InstantiateObject(isolate, data, false);
477+
return ::v8::internal::InstantiateObject(isolate, data, new_target, false);
457478
}
458479

459480

‎src/api-natives.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ class ApiNatives {
2323
Handle<FunctionTemplateInfo> data);
2424

2525
MUST_USE_RESULT static MaybeHandle<JSObject> InstantiateObject(
26-
Handle<ObjectTemplateInfo> data);
26+
Handle<ObjectTemplateInfo> data,
27+
Handle<JSReceiver> new_target = Handle<JSReceiver>());
2728

2829
enum ApiInstanceType {
2930
JavaScriptObjectType,

‎src/builtins.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4571,9 +4571,11 @@ MUST_USE_RESULT MaybeHandle<Object> HandleApiCallHelper(
45714571
}
45724572
Handle<ObjectTemplateInfo> instance_template(
45734573
ObjectTemplateInfo::cast(fun_data->instance_template()), isolate);
4574-
ASSIGN_RETURN_ON_EXCEPTION(isolate, receiver,
4575-
ApiNatives::InstantiateObject(instance_template),
4576-
Object);
4574+
ASSIGN_RETURN_ON_EXCEPTION(
4575+
isolate, receiver,
4576+
ApiNatives::InstantiateObject(instance_template,
4577+
Handle<JSReceiver>::cast(new_target)),
4578+
Object);
45774579
args[0] = *receiver;
45784580
DCHECK_EQ(*receiver, *args.receiver());
45794581
} else {

‎src/objects.cc‎

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11863,50 +11863,52 @@ namespace {
1186311863

1186411864
bool CanSubclassHaveInobjectProperties(InstanceType instance_type) {
1186511865
switch (instance_type) {
11866-
case JS_OBJECT_TYPE:
11867-
case JS_CONTEXT_EXTENSION_OBJECT_TYPE:
11868-
case JS_GENERATOR_OBJECT_TYPE:
11869-
case JS_MODULE_TYPE:
11870-
case JS_VALUE_TYPE:
11871-
case JS_DATE_TYPE:
11872-
case JS_ARRAY_TYPE:
11873-
case JS_MESSAGE_OBJECT_TYPE:
11866+
case JS_API_OBJECT_TYPE:
1187411867
case JS_ARRAY_BUFFER_TYPE:
11875-
case JS_TYPED_ARRAY_TYPE:
11868+
case JS_ARRAY_TYPE:
11869+
case JS_CONTEXT_EXTENSION_OBJECT_TYPE:
1187611870
case JS_DATA_VIEW_TYPE:
11877-
case JS_SET_TYPE:
11871+
case JS_DATE_TYPE:
11872+
case JS_FUNCTION_TYPE:
11873+
case JS_GENERATOR_OBJECT_TYPE:
11874+
case JS_MAP_ITERATOR_TYPE:
1187811875
case JS_MAP_TYPE:
11876+
case JS_MESSAGE_OBJECT_TYPE:
11877+
case JS_MODULE_TYPE:
11878+
case JS_OBJECT_TYPE:
11879+
case JS_PROMISE_TYPE:
11880+
case JS_REGEXP_TYPE:
1187911881
case JS_SET_ITERATOR_TYPE:
11880-
case JS_MAP_ITERATOR_TYPE:
11882+
case JS_SET_TYPE:
11883+
case JS_SPECIAL_API_OBJECT_TYPE:
11884+
case JS_TYPED_ARRAY_TYPE:
11885+
case JS_VALUE_TYPE:
1188111886
case JS_WEAK_MAP_TYPE:
1188211887
case JS_WEAK_SET_TYPE:
11883-
case JS_PROMISE_TYPE:
11884-
case JS_REGEXP_TYPE:
11885-
case JS_FUNCTION_TYPE:
1188611888
return true;
1188711889

11888-
case JS_BOUND_FUNCTION_TYPE:
11889-
case JS_PROXY_TYPE:
11890-
case JS_GLOBAL_PROXY_TYPE:
11891-
case JS_GLOBAL_OBJECT_TYPE:
11890+
case BYTECODE_ARRAY_TYPE:
11891+
case BYTE_ARRAY_TYPE:
11892+
case CELL_TYPE:
11893+
case CODE_TYPE:
11894+
case FILLER_TYPE:
1189211895
case FIXED_ARRAY_TYPE:
1189311896
case FIXED_DOUBLE_ARRAY_TYPE:
11894-
case ODDBALL_TYPE:
1189511897
case FOREIGN_TYPE:
11896-
case MAP_TYPE:
11897-
case CODE_TYPE:
11898-
case CELL_TYPE:
11899-
case PROPERTY_CELL_TYPE:
11900-
case WEAK_CELL_TYPE:
11901-
case SYMBOL_TYPE:
11902-
case BYTECODE_ARRAY_TYPE:
11898+
case FREE_SPACE_TYPE:
1190311899
case HEAP_NUMBER_TYPE:
11900+
case JS_BOUND_FUNCTION_TYPE:
11901+
case JS_GLOBAL_OBJECT_TYPE:
11902+
case JS_GLOBAL_PROXY_TYPE:
11903+
case JS_PROXY_TYPE:
11904+
case MAP_TYPE:
1190411905
case MUTABLE_HEAP_NUMBER_TYPE:
11905-
case SIMD128_VALUE_TYPE:
11906-
case FILLER_TYPE:
11907-
case BYTE_ARRAY_TYPE:
11908-
case FREE_SPACE_TYPE:
11906+
case ODDBALL_TYPE:
11907+
case PROPERTY_CELL_TYPE:
1190911908
case SHARED_FUNCTION_INFO_TYPE:
11909+
case SIMD128_VALUE_TYPE:
11910+
case SYMBOL_TYPE:
11911+
case WEAK_CELL_TYPE:
1191011912

1191111913
#define TYPED_ARRAY_CASE(Type, type, TYPE, ctype, size) \
1191211914
case FIXED_##TYPE##_ARRAY_TYPE:

‎test/cctest/test-api.cc‎

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2142,6 +2142,95 @@ THREADED_TEST(TestObjectTemplateInheritedWithPrototype2) {
21422142
Constructor_GetFunction_New);
21432143
}
21442144

2145+
THREADED_TEST(TestObjectTemplateClassInheritance) {
2146+
LocalContext env;
2147+
v8::Isolate* isolate = CcTest::isolate();
2148+
v8::HandleScope scope(isolate);
2149+
2150+
Local<v8::FunctionTemplate> fun_A = v8::FunctionTemplate::New(isolate);
2151+
fun_A->SetClassName(v8_str("A"));
2152+
2153+
Local<ObjectTemplate> templ_A = fun_A->InstanceTemplate();
2154+
templ_A->SetNativeDataProperty(v8_str("nirk"), GetNirk);
2155+
templ_A->SetNativeDataProperty(v8_str("rino"), GetRino);
2156+
2157+
Local<v8::FunctionTemplate> fun_B = v8::FunctionTemplate::New(isolate);
2158+
v8::Local<v8::String> class_name = v8_str("B");
2159+
fun_B->SetClassName(class_name);
2160+
fun_B->Inherit(fun_A);
2161+
2162+
v8::Local<v8::String> subclass_name = v8_str("C");
2163+
v8::Local<v8::Object> b_proto;
2164+
v8::Local<v8::Object> c_proto;
2165+
// Perform several iterations to make sure the cache doesn't break
2166+
// subclassing.
2167+
for (int i = 0; i < 3; i++) {
2168+
Local<v8::Function> function_B =
2169+
fun_B->GetFunction(env.local()).ToLocalChecked();
2170+
if (i == 0) {
2171+
CHECK(env->Global()->Set(env.local(), class_name, function_B).FromJust());
2172+
CompileRun("class C extends B {}");
2173+
b_proto =
2174+
CompileRun("B.prototype")->ToObject(env.local()).ToLocalChecked();
2175+
c_proto =
2176+
CompileRun("C.prototype")->ToObject(env.local()).ToLocalChecked();
2177+
CHECK(b_proto->Equals(env.local(), c_proto->GetPrototype()).FromJust());
2178+
}
2179+
Local<v8::Object> instance =
2180+
CompileRun("new C()")->ToObject(env.local()).ToLocalChecked();
2181+
CHECK(c_proto->Equals(env.local(), instance->GetPrototype()).FromJust());
2182+
2183+
CHECK(subclass_name->StrictEquals(instance->GetConstructorName()));
2184+
CHECK(env->Global()->Set(env.local(), v8_str("o"), instance).FromJust());
2185+
2186+
CHECK_EQ(900, CompileRun("o.nirk")->IntegerValue(env.local()).FromJust());
2187+
CHECK_EQ(560, CompileRun("o.rino")->IntegerValue(env.local()).FromJust());
2188+
}
2189+
}
2190+
2191+
static void NamedPropertyGetterWhichReturns42(
2192+
Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
2193+
info.GetReturnValue().Set(v8_num(42));
2194+
}
2195+
2196+
THREADED_TEST(TestObjectTemplateReflectConstruct) {
2197+
LocalContext env;
2198+
v8::Isolate* isolate = CcTest::isolate();
2199+
v8::HandleScope scope(isolate);
2200+
2201+
Local<v8::FunctionTemplate> fun_B = v8::FunctionTemplate::New(isolate);
2202+
fun_B->InstanceTemplate()->SetHandler(
2203+
v8::NamedPropertyHandlerConfiguration(NamedPropertyGetterWhichReturns42));
2204+
v8::Local<v8::String> class_name = v8_str("B");
2205+
fun_B->SetClassName(class_name);
2206+
2207+
v8::Local<v8::String> subclass_name = v8_str("C");
2208+
v8::Local<v8::Object> b_proto;
2209+
v8::Local<v8::Object> c_proto;
2210+
// Perform several iterations to make sure the cache doesn't break
2211+
// subclassing.
2212+
for (int i = 0; i < 3; i++) {
2213+
Local<v8::Function> function_B =
2214+
fun_B->GetFunction(env.local()).ToLocalChecked();
2215+
if (i == 0) {
2216+
CHECK(env->Global()->Set(env.local(), class_name, function_B).FromJust());
2217+
CompileRun("function C() {}");
2218+
c_proto =
2219+
CompileRun("C.prototype")->ToObject(env.local()).ToLocalChecked();
2220+
}
2221+
Local<v8::Object> instance = CompileRun("Reflect.construct(B, [], C)")
2222+
->ToObject(env.local())
2223+
.ToLocalChecked();
2224+
CHECK(c_proto->Equals(env.local(), instance->GetPrototype()).FromJust());
2225+
2226+
CHECK(subclass_name->StrictEquals(instance->GetConstructorName()));
2227+
CHECK(env->Global()->Set(env.local(), v8_str("o"), instance).FromJust());
2228+
2229+
CHECK_EQ(42, CompileRun("o.nirk")->IntegerValue(env.local()).FromJust());
2230+
CHECK_EQ(42, CompileRun("o.rino")->IntegerValue(env.local()).FromJust());
2231+
}
2232+
}
2233+
21452234
static void GetFlabby(const v8::FunctionCallbackInfo<v8::Value>& args) {
21462235
ApiTestFuzzer::Fuzz();
21472236
args.GetReturnValue().Set(v8_num(17.2));
@@ -18730,12 +18819,6 @@ TEST(SetterOnConstructorPrototype) {
1873018819
}
1873118820

1873218821

18733-
static void NamedPropertyGetterWhichReturns42(
18734-
Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
18735-
info.GetReturnValue().Set(v8_num(42));
18736-
}
18737-
18738-
1873918822
static void NamedPropertySetterWhichSetsYOnThisTo23(
1874018823
Local<Name> name, Local<Value> value,
1874118824
const v8::PropertyCallbackInfo<v8::Value>& info) {

0 commit comments

Comments
 (0)