Commit 4ce88f56 authored by Nico Hartmann's avatar Nico Hartmann Committed by V8 LUCI CQ

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

This reverts commit bc1eb7b4.

Reason for revert: https://ci.chromium.org/ui/p/chromium/builders/try/android-pie-arm64-rel/369203/overview

Original change's description:
> [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/+/2867473
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#74378}

Bug: chromium:923807
Bug: chromium:1071424
Bug: chromium:1138860
Change-Id: Iec678dee130db891c2096e47bc072a5d77ae9476
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2874403
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: 's avatarLutz Vahl <vahl@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74404}
parent 9c5623c7
......@@ -7590,10 +7590,6 @@ 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 ---
/**
......@@ -9543,9 +9539,6 @@ 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,7 +5930,6 @@ 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
......@@ -8997,18 +8996,13 @@ 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) {
i::WasmJs::InstallConditionalFeatures(isolate, Utils::OpenHandle(*context));
v8::HandleScope handle_scope(this);
v8::Context::Scope context_scope(context);
i::WasmJs::InstallConditionalFeatures(reinterpret_cast<i::Isolate*>(this),
Utils::OpenHandle(*context));
}
#endif // V8_ENABLE_WEBASSEMBLY
}
......
......@@ -2577,29 +2577,6 @@ 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,8 +434,6 @@ 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) \
......@@ -687,10 +685,6 @@ 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);
......
......@@ -340,13 +340,6 @@ 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,10 +4405,7 @@ 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 ||
FLAG_enable_sharedarraybuffer_per_context) {
return;
}
if (!FLAG_harmony_sharedarraybuffer) return;
Handle<JSGlobalObject> global(native_context()->global_object(), isolate());
......
......@@ -29071,52 +29071,3 @@ 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