Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 867586c

Browse filesBrowse files
targostargos
committed
deps: V8: cherry-pick 93b1a74cbc9b
Original commit message: Reland "[api] allow v8::Data as internal field" This is a reland of commit 0aa622e12893e9921c01a34ce9507b544e599c4a The original patch tried to run a test that calls exit() in the fatal error handler in parallel, which would not work. This marked the test with TEST() to avoid running it in parallel. Original change's description: > [api] allow v8::Data as internal field > > Previously only v8::Value can be stored as internal fields. > In some cases, however, it's necessary for the embedder to > tie the lifetime of a v8::Data with the lifetime of a > JS object, and that v8::Data may not be a v8::Value, as > it can be something returned from the V8 API. One way to > keep the v8::Data alive may be to use a v8::Persistent<v8::Data> > but that can easily lead to leaks. > > This patch changes v8::Object::GetInternalField() and > v8::Object::SetInernalField() to accept v8::Data instead of just > v8::Value, so that v8::Data can kept alive by a JS object in > a way that the GC can be aware of to address this problem. > This is a breaking change for embedders > using v8::Object::GetInternalField() as it changes the return > type. Since most v8::Value subtypes only support direct casts > from v8::Value but not v8::Data, calls like > > object->GetInternalField(index).As<v8::External>() > > needs to be updated to cast the value to v8::Value first: > > object->GetInternalField(index).As<v8::Value>().As<v8::External>() > > Bug: v8:14120 > Change-Id: I731c958d1756b9d5ee4a3e78813416cd60d1b7ca > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4707972 > Reviewed-by: Michael Lippautz <mlippautz@chromium.org> > Commit-Queue: Joyee Cheung <joyee@igalia.com> > Cr-Commit-Position: refs/heads/main@{#89718} Bug: v8:14120 Change-Id: I3e45d09b5c300d5eefc73e380ef21ac2bd61760c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4834471 Commit-Queue: Joyee Cheung <joyee@igalia.com> Reviewed-by: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/main@{#89824} Refs: v8/v8@93b1a74 PR-URL: #49639 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 4ad3479 commit 867586c
Copy full SHA for 867586c

File tree

Expand file treeCollapse file tree

8 files changed

+104
-35
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+104
-35
lines changed
Open diff view settings
Collapse file

‎common.gypi‎

Copy file name to clipboardExpand all lines: common.gypi
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.7',
39+
'v8_embedder_string': '-node.8',
4040

4141
##### V8 defaults for Node.js #####
4242

Collapse file

‎deps/v8/include/v8-object.h‎

Copy file name to clipboardExpand all lines: deps/v8/include/v8-object.h
+16-7Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -483,11 +483,20 @@ class V8_EXPORT Object : public Value {
483483
return object.template value<Object>()->InternalFieldCount();
484484
}
485485

486-
/** Gets the value from an internal field. */
487-
V8_INLINE Local<Value> GetInternalField(int index);
486+
/**
487+
* Gets the data from an internal field.
488+
* To cast the return value into v8::Value subtypes, it needs to be
489+
* casted to a v8::Value first. For example, to cast it into v8::External:
490+
*
491+
* object->GetInternalField(index).As<v8::Value>().As<v8::External>();
492+
*
493+
* The embedder should make sure that the internal field being retrieved
494+
* using this method has already been set with SetInternalField() before.
495+
**/
496+
V8_INLINE Local<Data> GetInternalField(int index);
488497

489-
/** Sets the value in an internal field. */
490-
void SetInternalField(int index, Local<Value> value);
498+
/** Sets the data in an internal field. */
499+
void SetInternalField(int index, Local<Data> data);
491500

492501
/**
493502
* Gets a 2-byte-aligned native pointer from an internal field. This field
@@ -725,13 +734,13 @@ class V8_EXPORT Object : public Value {
725734
private:
726735
Object();
727736
static void CheckCast(Value* obj);
728-
Local<Value> SlowGetInternalField(int index);
737+
Local<Data> SlowGetInternalField(int index);
729738
void* SlowGetAlignedPointerFromInternalField(int index);
730739
};
731740

732741
// --- Implementation ---
733742

734-
Local<Value> Object::GetInternalField(int index) {
743+
Local<Data> Object::GetInternalField(int index) {
735744
#ifndef V8_ENABLE_CHECKS
736745
using A = internal::Address;
737746
using I = internal::Internals;
@@ -750,7 +759,7 @@ Local<Value> Object::GetInternalField(int index) {
750759

751760
auto isolate = reinterpret_cast<v8::Isolate*>(
752761
internal::IsolateFromNeverReadOnlySpaceObject(obj));
753-
return Local<Value>::New(isolate, value);
762+
return Local<Data>::New(isolate, value);
754763
}
755764
#endif
756765
return SlowGetInternalField(index);
Collapse file

‎deps/v8/samples/process.cc‎

Copy file name to clipboardExpand all lines: deps/v8/samples/process.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ Local<Object> JsHttpRequestProcessor::WrapMap(map<string, string>* obj) {
386386
// Utility function that extracts the C++ map pointer from a wrapper
387387
// object.
388388
map<string, string>* JsHttpRequestProcessor::UnwrapMap(Local<Object> obj) {
389-
Local<External> field = obj->GetInternalField(0).As<External>();
389+
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
390390
void* ptr = field->Value();
391391
return static_cast<map<string, string>*>(ptr);
392392
}
@@ -502,7 +502,7 @@ Local<Object> JsHttpRequestProcessor::WrapRequest(HttpRequest* request) {
502502
* wrapper object.
503503
*/
504504
HttpRequest* JsHttpRequestProcessor::UnwrapRequest(Local<Object> obj) {
505-
Local<External> field = obj->GetInternalField(0).As<External>();
505+
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
506506
void* ptr = field->Value();
507507
return static_cast<HttpRequest*>(ptr);
508508
}
Collapse file

‎deps/v8/src/api/api.cc‎

Copy file name to clipboardExpand all lines: deps/v8/src/api/api.cc
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6246,16 +6246,16 @@ static bool InternalFieldOK(i::Handle<i::JSReceiver> obj, int index,
62466246
location, "Internal field out of bounds");
62476247
}
62486248

6249-
Local<Value> v8::Object::SlowGetInternalField(int index) {
6249+
Local<Data> v8::Object::SlowGetInternalField(int index) {
62506250
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
62516251
const char* location = "v8::Object::GetInternalField()";
62526252
if (!InternalFieldOK(obj, index, location)) return Local<Value>();
62536253
i::Handle<i::Object> value(i::JSObject::cast(*obj)->GetEmbedderField(index),
62546254
obj->GetIsolate());
6255-
return Utils::ToLocal(value);
6255+
return ToApiHandle<Data>(value);
62566256
}
62576257

6258-
void v8::Object::SetInternalField(int index, v8::Local<Value> value) {
6258+
void v8::Object::SetInternalField(int index, v8::Local<Data> value) {
62596259
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
62606260
const char* location = "v8::Object::SetInternalField()";
62616261
if (!InternalFieldOK(obj, index, location)) return;
Collapse file

‎deps/v8/test/cctest/test-api.cc‎

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-api.cc
+57-9Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2884,6 +2884,45 @@ THREADED_TEST(FunctionPrototype) {
28842884
CHECK_EQ(v8_run_int32value(script), 321);
28852885
}
28862886

2887+
bool internal_field_check_called = false;
2888+
void OnInternalFieldCheck(const char* location, const char* message) {
2889+
internal_field_check_called = true;
2890+
exit(strcmp(location, "v8::Value::Cast") +
2891+
strcmp(message, "Data is not a Value"));
2892+
}
2893+
2894+
// The fatal error handler would call exit() so this should not be run in
2895+
// parallel.
2896+
TEST(InternalDataFields) {
2897+
LocalContext env;
2898+
v8::Isolate* isolate = env->GetIsolate();
2899+
v8::HandleScope scope(isolate);
2900+
2901+
Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
2902+
Local<v8::ObjectTemplate> instance_templ = templ->InstanceTemplate();
2903+
instance_templ->SetInternalFieldCount(1);
2904+
Local<v8::Object> obj = templ->GetFunction(env.local())
2905+
.ToLocalChecked()
2906+
->NewInstance(env.local())
2907+
.ToLocalChecked();
2908+
CHECK_EQ(1, obj->InternalFieldCount());
2909+
Local<v8::Data> data = obj->GetInternalField(0);
2910+
CHECK(data->IsValue() && data.As<v8::Value>()->IsUndefined());
2911+
Local<v8::Private> sym = v8::Private::New(isolate, v8_str("Foo"));
2912+
obj->SetInternalField(0, sym);
2913+
Local<v8::Data> field = obj->GetInternalField(0);
2914+
CHECK(!field->IsValue());
2915+
CHECK(field->IsPrivate());
2916+
CHECK_EQ(sym, field);
2917+
2918+
#ifdef V8_ENABLE_CHECKS
2919+
isolate->SetFatalErrorHandler(OnInternalFieldCheck);
2920+
USE(obj->GetInternalField(0).As<v8::Value>());
2921+
// If it's never called this would fail.
2922+
CHECK(internal_field_check_called);
2923+
#endif
2924+
}
2925+
28872926
THREADED_TEST(InternalFields) {
28882927
LocalContext env;
28892928
v8::Isolate* isolate = env->GetIsolate();
@@ -2897,9 +2936,12 @@ THREADED_TEST(InternalFields) {
28972936
->NewInstance(env.local())
28982937
.ToLocalChecked();
28992938
CHECK_EQ(1, obj->InternalFieldCount());
2900-
CHECK(obj->GetInternalField(0)->IsUndefined());
2939+
CHECK(obj->GetInternalField(0).As<v8::Value>()->IsUndefined());
29012940
obj->SetInternalField(0, v8_num(17));
2902-
CHECK_EQ(17, obj->GetInternalField(0)->Int32Value(env.local()).FromJust());
2941+
CHECK_EQ(17, obj->GetInternalField(0)
2942+
.As<v8::Value>()
2943+
->Int32Value(env.local())
2944+
.FromJust());
29032945
}
29042946

29052947
TEST(InternalFieldsSubclassing) {
@@ -2925,14 +2967,16 @@ TEST(InternalFieldsSubclassing) {
29252967
CHECK_EQ(0, i_obj->map()->GetInObjectProperties());
29262968
// Check writing and reading internal fields.
29272969
for (int j = 0; j < nof_embedder_fields; j++) {
2928-
CHECK(obj->GetInternalField(j)->IsUndefined());
2970+
CHECK(obj->GetInternalField(j).As<v8::Value>()->IsUndefined());
29292971
int value = 17 + j;
29302972
obj->SetInternalField(j, v8_num(value));
29312973
}
29322974
for (int j = 0; j < nof_embedder_fields; j++) {
29332975
int value = 17 + j;
2934-
CHECK_EQ(value,
2935-
obj->GetInternalField(j)->Int32Value(env.local()).FromJust());
2976+
CHECK_EQ(value, obj->GetInternalField(j)
2977+
.As<v8::Value>()
2978+
->Int32Value(env.local())
2979+
.FromJust());
29362980
}
29372981
CHECK(env->Global()
29382982
->Set(env.local(), v8_str("BaseClass"), constructor)
@@ -3032,9 +3076,12 @@ THREADED_TEST(GlobalObjectInternalFields) {
30323076
v8::Local<v8::Object> global_proxy = env->Global();
30333077
v8::Local<v8::Object> global = global_proxy->GetPrototype().As<v8::Object>();
30343078
CHECK_EQ(1, global->InternalFieldCount());
3035-
CHECK(global->GetInternalField(0)->IsUndefined());
3079+
CHECK(global->GetInternalField(0).As<v8::Value>()->IsUndefined());
30363080
global->SetInternalField(0, v8_num(17));
3037-
CHECK_EQ(17, global->GetInternalField(0)->Int32Value(env.local()).FromJust());
3081+
CHECK_EQ(17, global->GetInternalField(0)
3082+
.As<v8::Value>()
3083+
->Int32Value(env.local())
3084+
.FromJust());
30383085
}
30393086

30403087

@@ -7789,7 +7836,7 @@ void InternalFieldCallback(bool global_gc) {
77897836
.ToLocalChecked();
77907837
handle.Reset(isolate, obj);
77917838
CHECK_EQ(2, obj->InternalFieldCount());
7792-
CHECK(obj->GetInternalField(0)->IsUndefined());
7839+
CHECK(obj->GetInternalField(0).As<v8::Value>()->IsUndefined());
77937840
t1 = new Trivial(42);
77947841
t2 = new Trivial2(103, 9);
77957842

@@ -29699,7 +29746,8 @@ class HiddenDataDelegate : public v8::Context::DeepFreezeDelegate {
2969929746
std::vector<v8::Local<v8::Object>>& children_out) override {
2970029747
int fields = obj->InternalFieldCount();
2970129748
for (int idx = 0; idx < fields; idx++) {
29702-
v8::Local<v8::Value> child_value = obj->GetInternalField(idx);
29749+
v8::Local<v8::Value> child_value =
29750+
obj->GetInternalField(idx).As<v8::Value>();
2970329751
if (child_value->IsExternal()) {
2970429752
if (!FreezeExternal(v8::Local<v8::External>::Cast(child_value),
2970529753
children_out)) {
Collapse file

‎deps/v8/test/cctest/test-api.h‎

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-api.h
+5-3Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,11 @@ template <typename T>
4646
static void CheckInternalFieldsAreZero(v8::Local<T> value) {
4747
CHECK_EQ(T::kInternalFieldCount, value->InternalFieldCount());
4848
for (int i = 0; i < value->InternalFieldCount(); i++) {
49-
CHECK_EQ(0, value->GetInternalField(i)
50-
->Int32Value(CcTest::isolate()->GetCurrentContext())
51-
.FromJust());
49+
v8::Local<v8::Value> field =
50+
value->GetInternalField(i).template As<v8::Value>();
51+
CHECK_EQ(
52+
0,
53+
field->Int32Value(CcTest::isolate()->GetCurrentContext()).FromJust());
5254
}
5355
}
5456

Collapse file

‎deps/v8/test/cctest/test-serialize.cc‎

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-serialize.cc
+12-8Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3667,23 +3667,27 @@ UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
36673667
.ToLocalChecked()
36683668
->ToObject(context)
36693669
.ToLocalChecked();
3670-
v8::Local<v8::Object> b =
3671-
a->GetInternalField(0)->ToObject(context).ToLocalChecked();
3672-
v8::Local<v8::Object> c =
3673-
b->GetInternalField(0)->ToObject(context).ToLocalChecked();
3670+
v8::Local<v8::Object> b = a->GetInternalField(0)
3671+
.As<v8::Value>()
3672+
->ToObject(context)
3673+
.ToLocalChecked();
3674+
v8::Local<v8::Object> c = b->GetInternalField(0)
3675+
.As<v8::Value>()
3676+
->ToObject(context)
3677+
.ToLocalChecked();
36743678

36753679
InternalFieldData* a1 = reinterpret_cast<InternalFieldData*>(
36763680
a->GetAlignedPointerFromInternalField(1));
3677-
v8::Local<v8::Value> a2 = a->GetInternalField(2);
3681+
v8::Local<v8::Value> a2 = a->GetInternalField(2).As<v8::Value>();
36783682

36793683
InternalFieldData* b1 = reinterpret_cast<InternalFieldData*>(
36803684
b->GetAlignedPointerFromInternalField(1));
3681-
v8::Local<v8::Value> b2 = b->GetInternalField(2);
3685+
v8::Local<v8::Value> b2 = b->GetInternalField(2).As<v8::Value>();
36823686

3683-
v8::Local<v8::Value> c0 = c->GetInternalField(0);
3687+
v8::Local<v8::Value> c0 = c->GetInternalField(0).As<v8::Value>();
36843688
InternalFieldData* c1 = reinterpret_cast<InternalFieldData*>(
36853689
c->GetAlignedPointerFromInternalField(1));
3686-
v8::Local<v8::Value> c2 = c->GetInternalField(2);
3690+
v8::Local<v8::Value> c2 = c->GetInternalField(2).As<v8::Value>();
36873691

36883692
CHECK(c0->IsUndefined());
36893693

Collapse file

‎deps/v8/test/unittests/objects/value-serializer-unittest.cc‎

Copy file name to clipboardExpand all lines: deps/v8/test/unittests/objects/value-serializer-unittest.cc
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,15 @@ class ValueSerializerTest : public TestWithIsolate {
6363
StringFromUtf8("value"),
6464
[](Local<String> property, const PropertyCallbackInfo<Value>& info) {
6565
CHECK(i::ValidateCallbackInfo(info));
66-
info.GetReturnValue().Set(info.Holder()->GetInternalField(0));
66+
info.GetReturnValue().Set(
67+
info.Holder()->GetInternalField(0).As<v8::Value>());
6768
});
6869
function_template->InstanceTemplate()->SetAccessor(
6970
StringFromUtf8("value2"),
7071
[](Local<String> property, const PropertyCallbackInfo<Value>& info) {
7172
CHECK(i::ValidateCallbackInfo(info));
72-
info.GetReturnValue().Set(info.Holder()->GetInternalField(1));
73+
info.GetReturnValue().Set(
74+
info.Holder()->GetInternalField(1).As<v8::Value>());
7375
});
7476
for (Local<Context> context :
7577
{serialization_context_, deserialization_context_}) {
@@ -2884,6 +2886,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint32) {
28842886
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
28852887
uint32_t value = 0;
28862888
EXPECT_TRUE(object->GetInternalField(0)
2889+
.As<v8::Value>()
28872890
->Uint32Value(serialization_context())
28882891
.To(&value));
28892892
WriteExampleHostObjectTag();
@@ -2915,9 +2918,11 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint64) {
29152918
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
29162919
uint32_t value = 0, value2 = 0;
29172920
EXPECT_TRUE(object->GetInternalField(0)
2921+
.As<v8::Value>()
29182922
->Uint32Value(serialization_context())
29192923
.To(&value));
29202924
EXPECT_TRUE(object->GetInternalField(1)
2925+
.As<v8::Value>()
29212926
->Uint32Value(serialization_context())
29222927
.To(&value2));
29232928
WriteExampleHostObjectTag();
@@ -2955,6 +2960,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripDouble) {
29552960
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
29562961
double value = 0;
29572962
EXPECT_TRUE(object->GetInternalField(0)
2963+
.As<v8::Value>()
29582964
->NumberValue(serialization_context())
29592965
.To(&value));
29602966
WriteExampleHostObjectTag();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.