Skip to content

Commit 5e139e9

Browse files
isheludkoV8 LUCI CQ
isheludko
authored and
V8 LUCI CQ
committed
[api] Introduce v8::Object::GetPrototypeV2()/SetPrototypeV2()
... as a replacement for soon to be deprecated v8::Object::GetPrototype() and v8::Object::SetPrototype(). The new versions will act just like getting/setting of "__proto__" property, i.e. they will never return or accept JSGlobalObject. This is a first step towards never exposing so-called hidden prototype (i.e. JSGlobalObject) via the V8 Api. This will make the JSGlobalObject a V8 internal thing and make the V8 Api closer to JavaScript spec. Bug: chromium:333672197 Change-Id: Ic850d6a041f443392586b868eccde637ded801d3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5447696 Commit-Queue: Igor Sheludko <[email protected]> Reviewed-by: Leszek Swirski <[email protected]> Cr-Commit-Position: refs/heads/main@{#93325}
1 parent 43dc92e commit 5e139e9

3 files changed

Lines changed: 72 additions & 12 deletions

File tree

‎include/v8-object.h‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,16 +433,41 @@ class V8_EXPORT Object : public Value {
433433
* be skipped by __proto__ and it does not consult the security
434434
* handler.
435435
*/
436+
// V8_DEPRECATE_SOON(
437+
// "V8 will stop providing access to hidden prototype (i.e. "
438+
// "JSGlobalObject). Use GetPrototypeV2() instead. "
439+
// "See http://crbug.com/333672197.")
436440
Local<Value> GetPrototype();
437441

442+
/**
443+
* Get the prototype object (same as getting __proto__ property). This does
444+
* not consult the security handler.
445+
* TODO(333672197): rename back to GetPrototype() once the old version goes
446+
* through the deprecation process and is removed.
447+
*/
448+
Local<Value> GetPrototypeV2();
449+
438450
/**
439451
* Set the prototype object. This does not skip objects marked to
440452
* be skipped by __proto__ and it does not consult the security
441453
* handler.
442454
*/
455+
// V8_DEPRECATE_SOON(
456+
// "V8 will stop providing access to hidden prototype (i.e. "
457+
// "JSGlobalObject). Use SetPrototypeV2() instead. "
458+
// "See http://crbug.com/333672197.")
443459
V8_WARN_UNUSED_RESULT Maybe<bool> SetPrototype(Local<Context> context,
444460
Local<Value> prototype);
445461

462+
/**
463+
* Set the prototype object (same as setting __proto__ property). This does
464+
* does not consult the security handler.
465+
* TODO(333672197): rename back to SetPrototype() once the old version goes
466+
* through the deprecation process and is removed.
467+
*/
468+
V8_WARN_UNUSED_RESULT Maybe<bool> SetPrototypeV2(Local<Context> context,
469+
Local<Value> prototype);
470+
446471
/**
447472
* Finds an instance of the given function template in the prototype
448473
* chain.

‎src/api/api.cc‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4737,31 +4737,65 @@ Local<Value> v8::Object::GetPrototype() {
47374737
return Utils::ToLocal(i::PrototypeIterator::GetCurrent(iter));
47384738
}
47394739

4740-
Maybe<bool> v8::Object::SetPrototype(Local<Context> context,
4741-
Local<Value> value) {
4742-
auto i_isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
4740+
Local<Value> v8::Object::GetPrototypeV2() {
47434741
auto self = Utils::OpenHandle(this);
4742+
auto i_isolate = self->GetIsolate();
4743+
i::PrototypeIterator iter(i_isolate, self);
4744+
if (i::IsJSGlobalProxy(*self)) {
4745+
// Skip hidden prototype (i.e. JSGlobalObject).
4746+
iter.Advance();
4747+
}
4748+
DCHECK(!i::IsJSGlobalObject(*i::PrototypeIterator::GetCurrent(iter)));
4749+
return Utils::ToLocal(i::PrototypeIterator::GetCurrent(iter));
4750+
}
4751+
4752+
namespace {
4753+
4754+
Maybe<bool> SetPrototypeImpl(v8::Object* this_, Local<Context> context,
4755+
Local<Value> value, bool from_javascript) {
4756+
auto i_isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
4757+
auto self = Utils::OpenHandle(this_);
47444758
auto value_obj = Utils::OpenHandle(*value);
4759+
// TODO(333672197): turn this to DCHECK once it's no longer possible
4760+
// to get JSGlobalObject via API.
4761+
CHECK_IMPLIES(from_javascript, !i::IsJSGlobalObject(*value_obj));
47454762
if (i::IsJSProxy(*self)) {
47464763
ENTER_V8(i_isolate, context, Object, SetPrototype, i::HandleScope);
47474764
// We do not allow exceptions thrown while setting the prototype
47484765
// to propagate outside.
47494766
TryCatch try_catch(reinterpret_cast<v8::Isolate*>(i_isolate));
47504767
auto result =
47514768
i::JSProxy::SetPrototype(i_isolate, i::Handle<i::JSProxy>::cast(self),
4752-
value_obj, false, i::kThrowOnError);
4769+
value_obj, from_javascript, i::kThrowOnError);
47534770
has_exception = result.IsNothing();
47544771
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
47554772
} else {
47564773
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
4774+
// TODO(333672197): turn this to DCHECK once it's no longer possible
4775+
// to get JSGlobalObject via API.
4776+
CHECK_IMPLIES(from_javascript, !i::IsJSGlobalObject(*self));
47574777
auto result =
47584778
i::JSObject::SetPrototype(i_isolate, i::Handle<i::JSObject>::cast(self),
4759-
value_obj, false, i::kDontThrow);
4779+
value_obj, from_javascript, i::kDontThrow);
47604780
if (!result.FromJust()) return Nothing<bool>();
47614781
}
47624782
return Just(true);
47634783
}
47644784

4785+
} // namespace
4786+
4787+
Maybe<bool> v8::Object::SetPrototype(Local<Context> context,
4788+
Local<Value> value) {
4789+
static constexpr bool from_javascript = false;
4790+
return SetPrototypeImpl(this, context, value, from_javascript);
4791+
}
4792+
4793+
Maybe<bool> v8::Object::SetPrototypeV2(Local<Context> context,
4794+
Local<Value> value) {
4795+
static constexpr bool from_javascript = true;
4796+
return SetPrototypeImpl(this, context, value, from_javascript);
4797+
}
4798+
47654799
Local<Object> v8::Object::FindInstanceInPrototypeChain(
47664800
v8::Local<FunctionTemplate> tmpl) {
47674801
auto self = Utils::OpenDirectHandle(this);

‎test/unittests/objects/global-object-unittest.cc‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,10 @@ TEST_F(GlobalObjectTest, StrictUndeclaredGlobalVariable) {
4141
Local<String> var_name = NewString("x");
4242
TryCatch try_catch(isolate());
4343
Local<Object> proto = Object::New(isolate());
44-
Local<Object> global = context()->Global()->GetPrototype().As<Object>();
44+
Local<Object> global = context()->Global();
4545
proto->Set(context(), var_name, Number::New(isolate(), 100)).FromJust();
46-
global->SetPrototype(context(), proto).FromJust();
46+
global->SetPrototypeV2(context(), proto).FromJust();
47+
CHECK_EQ(global->GetPrototypeV2(), proto);
4748
CHECK(TryRunJS("\"use strict\"; x = 42;").IsEmpty());
4849
CHECK(try_catch.HasCaught());
4950
String::Utf8Value exception(isolate(), try_catch.Exception());
@@ -108,11 +109,11 @@ TEST_F(GlobalObjectTest, KeysGlobalObject_SetPrototype) {
108109
env2->SetSecurityToken(token);
109110

110111
// Create a reference to env2 global from env1 global.
111-
env1->Global()
112-
->GetPrototype()
113-
.As<Object>()
114-
->SetPrototype(env1, env2->Global()->GetPrototype())
115-
.FromJust();
112+
env1->Global()->SetPrototypeV2(env1, env2->Global()).FromJust();
113+
CHECK_EQ(env1->Global()->GetPrototypeV2(), env2->Global());
114+
CHECK_EQ(env1->Global()->GetPrototype().As<Object>()->GetPrototype(),
115+
env2->Global());
116+
116117
// Set some global variables in global2
117118
env2->Global()->Set(env2, NewString("a"), NewString("a")).FromJust();
118119
env2->Global()->Set(env2, NewString("42"), NewString("42")).FromJust();

0 commit comments

Comments
 (0)