Commit bc1eb7b4 authored by Antonio Sartori's avatar Antonio Sartori Committed by V8 LUCI CQ

[api] Add API callback setter for the SAB origin trial

This change makes it possible to enable SharedArrayBuffer per Context,
controlling whether it should be enabled or not with a callback. The
previous implementation of the reverse origin trial for
SharedArrayBuffer was broken, since the feature could only be enabled
globally per process, and only if the feature flag is set early enough
in the v8 initialization. This does not play well with how origin
trials work.

The implementation is similar to the callbacks that already exist for
the origin trials for WebAssembly simd and exceptions.

SharedArrayBuffer is still controlled by the flag
harmony_sharedarraybuffer. If that flag is disabled, then
SharedArrayBuffer is disabled unconditionally. On top of that, this CL
introduces a new flag for enabling SharedArrayBuffer per context. If
that flag is set, a callback is used to determine whether
SharedArrayBuffer should be enabled.


Note that this only controls whether the SharedArrayBuffer constructor
should be exposed on the global object or not. It is always possible
to construct a SharedArrayBuffer using

  new WebAssembly.Memory({
    shared:true, initial:0, maximum:0 }).buffer.constructor;


There are few things which I do not like of this approach, but I did
not have better ideas:

1. The complex logic of dobule flag + callback. However, this seemed
the best way to me to not break embedders which rely on that flag
being enabled by default.

2. The fact that what actually matters is just whether the callback
returns `true` once. It would be good to check that the callback gives
a consistent return value, or to provide a better API that cannot be
missunderstood.


Bug: chromium:923807,chromium:1071424,chromium:1138860
Change-Id: Ibe3776fad4d3bff5dda9066967e4b20328014266
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2867473Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74378}
parent f486a343
......@@ -7590,6 +7590,10 @@ using WasmSimdEnabledCallback = bool (*)(Local<Context> context);
// --- Callback for checking if WebAssembly exceptions are enabled ---
using WasmExceptionsEnabledCallback = bool (*)(Local<Context> context);
// --- Callback for checking if the SharedArrayBuffer constructor is enabled ---
using SharedArrayBufferConstructorEnabledCallback =
bool (*)(Local<Context> context);
// --- Garbage Collection Callbacks ---
/**
......@@ -9539,6 +9543,9 @@ class V8_EXPORT Isolate {
void SetWasmExceptionsEnabledCallback(WasmExceptionsEnabledCallback callback);
void SetSharedArrayBufferConstructorEnabledCallback(
SharedArrayBufferConstructorEnabledCallback callback);
/**
* This function can be called by the embedder to signal V8 that the dynamic
* enabling of features has finished. V8 can now set up dynamically added
......
......@@ -5930,6 +5930,7 @@ void V8::GetSharedMemoryStatistics(SharedMemoryStatistics* statistics) {
void V8::SetIsCrossOriginIsolated() {
i::FLAG_harmony_sharedarraybuffer = true;
i::FLAG_enable_sharedarraybuffer_per_context = false;
#if V8_ENABLE_WEBASSEMBLY
i::FLAG_experimental_wasm_threads = true;
#endif // V8_ENABLE_WEBASSEMBLY
......@@ -8996,13 +8997,18 @@ CALLBACK_SETTER(WasmSimdEnabledCallback, WasmSimdEnabledCallback,
CALLBACK_SETTER(WasmExceptionsEnabledCallback, WasmExceptionsEnabledCallback,
wasm_exceptions_enabled_callback)
CALLBACK_SETTER(SharedArrayBufferConstructorEnabledCallback,
SharedArrayBufferConstructorEnabledCallback,
sharedarraybuffer_constructor_enabled_callback)
void Isolate::InstallConditionalFeatures(Local<Context> context) {
v8::HandleScope handle_scope(this);
v8::Context::Scope context_scope(context);
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
isolate->InstallConditionalFeatures(Utils::OpenHandle(*context));
#if V8_ENABLE_WEBASSEMBLY
if (i::FLAG_expose_wasm) {
v8::HandleScope handle_scope(this);
v8::Context::Scope context_scope(context);
i::WasmJs::InstallConditionalFeatures(reinterpret_cast<i::Isolate*>(this),
Utils::OpenHandle(*context));
i::WasmJs::InstallConditionalFeatures(isolate, Utils::OpenHandle(*context));
}
#endif // V8_ENABLE_WEBASSEMBLY
}
......
......@@ -2577,6 +2577,29 @@ void Isolate::SetAbortOnUncaughtExceptionCallback(
abort_on_uncaught_exception_callback_ = callback;
}
void Isolate::InstallConditionalFeatures(Handle<Context> context) {
Handle<JSGlobalObject> global = handle(context->global_object(), this);
Handle<String> sab_name = factory()->SharedArrayBuffer_string();
if (!JSReceiver::HasProperty(global, sab_name).FromMaybe(true)) {
if (IsSharedArrayBufferConstructorEnabled(context)) {
JSObject::AddProperty(this, global, factory()->SharedArrayBuffer_string(),
shared_array_buffer_fun(), DONT_ENUM);
}
}
}
bool Isolate::IsSharedArrayBufferConstructorEnabled(Handle<Context> context) {
if (!FLAG_harmony_sharedarraybuffer) return false;
if (!FLAG_enable_sharedarraybuffer_per_context) return true;
if (sharedarraybuffer_constructor_enabled_callback()) {
v8::Local<v8::Context> api_context = v8::Utils::ToLocal(context);
return sharedarraybuffer_constructor_enabled_callback()(api_context);
}
return false;
}
bool Isolate::IsWasmSimdEnabled(Handle<Context> context) {
#if V8_ENABLE_WEBASSEMBLY
if (wasm_simd_enabled_callback()) {
......
......@@ -434,6 +434,8 @@ using DebugObjectCache = std::vector<Handle<HeapObject>>;
V(AllowWasmCodeGenerationCallback, allow_wasm_code_gen_callback, nullptr) \
V(ExtensionCallback, wasm_module_callback, &NoExtension) \
V(ExtensionCallback, wasm_instance_callback, &NoExtension) \
V(SharedArrayBufferConstructorEnabledCallback, \
sharedarraybuffer_constructor_enabled_callback, nullptr) \
V(WasmStreamingCallback, wasm_streaming_callback, nullptr) \
V(WasmLoadSourceMapCallback, wasm_load_source_map_callback, nullptr) \
V(WasmSimdEnabledCallback, wasm_simd_enabled_callback, nullptr) \
......@@ -685,6 +687,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
inline void set_pending_exception(Object exception_obj);
inline void clear_pending_exception();
void InstallConditionalFeatures(Handle<Context> context);
bool IsSharedArrayBufferConstructorEnabled(Handle<Context> context);
bool IsWasmSimdEnabled(Handle<Context> context);
bool AreWasmExceptionsEnabled(Handle<Context> context);
......
......@@ -338,6 +338,13 @@ HARMONY_SHIPPING(FLAG_SHIPPING_FEATURES)
DEFINE_BOOL(builtin_subclassing, true,
"subclassing support in built-in methods")
// If the following flag is set to `true`, the SharedArrayBuffer constructor is
// enabled per context depending on the callback set via
// `SetSharedArrayBufferConstructorEnabledCallback`. If no callback is set, the
// SharedArrayBuffer constructor is disabled.
DEFINE_BOOL(enable_sharedarraybuffer_per_context, false,
"enable the SharedArrayBuffer constructor per context")
#ifdef V8_INTL_SUPPORT
DEFINE_BOOL(icu_timezone_data, true, "get information about timezones from ICU")
#endif
......
......@@ -4405,7 +4405,10 @@ EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_intl_dateformat_day_period)
#undef EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE
void Genesis::InitializeGlobal_harmony_sharedarraybuffer() {
if (!FLAG_harmony_sharedarraybuffer) return;
if (!FLAG_harmony_sharedarraybuffer ||
FLAG_enable_sharedarraybuffer_per_context) {
return;
}
Handle<JSGlobalObject> global(native_context()->global_object(), isolate());
......
......@@ -29071,3 +29071,52 @@ THREADED_TEST(MicrotaskQueueOfContext) {
microtask_queue.get());
CHECK_EQ(context->GetMicrotaskQueue(), microtask_queue.get());
}
namespace {
bool sab_constructor_enabled_value = false;
bool MockSabConstructorEnabledCallback(v8::Local<v8::Context>) {
return sab_constructor_enabled_value;
}
} // namespace
TEST(TestSetSabConstructorEnabledCallback) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
v8::HandleScope scope(isolate);
v8::Local<v8::Context> context = v8::Context::New(CcTest::isolate());
i::Handle<i::Context> i_context = v8::Utils::OpenHandle(*context);
// {Isolate::IsSharedArrayBufferConstructorEnabled} calls the callback set by
// the embedder if such a callback exists. Otherwise it returns
// {FLAG_harmony_sharedarraybuffer}. First we test that the flag is returned
// correctly if no callback is set. Then we test that the flag is ignored if
// the callback is set.
i::FLAG_harmony_sharedarraybuffer = false;
i::FLAG_enable_sharedarraybuffer_per_context = false;
CHECK(!i_isolate->IsSharedArrayBufferConstructorEnabled(i_context));
i::FLAG_harmony_sharedarraybuffer = false;
i::FLAG_enable_sharedarraybuffer_per_context = false;
CHECK(!i_isolate->IsSharedArrayBufferConstructorEnabled(i_context));
i::FLAG_harmony_sharedarraybuffer = true;
i::FLAG_enable_sharedarraybuffer_per_context = false;
CHECK(i_isolate->IsSharedArrayBufferConstructorEnabled(i_context));
i::FLAG_harmony_sharedarraybuffer = true;
i::FLAG_enable_sharedarraybuffer_per_context = true;
CHECK(!i_isolate->IsSharedArrayBufferConstructorEnabled(i_context));
isolate->SetSharedArrayBufferConstructorEnabledCallback(
MockSabConstructorEnabledCallback);
sab_constructor_enabled_value = false;
CHECK(!i_isolate->IsSharedArrayBufferConstructorEnabled(i_context));
sab_constructor_enabled_value = true;
CHECK(i_isolate->IsSharedArrayBufferConstructorEnabled(i_context));
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment