Deprecate SetAllowCodeGenerationFromStringsCallback.

It has been superseeded by SetModifyCodeGenerationFromStringsCallback.

The new method has been introduced in M77 [1], in current form since M80
[2], default-used by Blink since M80 [3].

[1] https://crrev.com/b9342b7b5ff2e5588eceb503dd52bb1e3fbfb21c
[2] https://crrev.com/6c0825aaa73ca3163f089ca161c1f6e15633f306
[3] https://crrev.com/bfd0621af3f09557e9713d5c76108c7dddaa49a6

Bug: v8:10096
Change-Id: If5475aaff9cfee29b42529cd158372b191d34f32
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1987252
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65717}

include/v8.h

@@ -9203,6 +9203,9 @@ class V8_EXPORT Isolate {
    * Set the callback to invoke to check if code generation from
    * strings should be allowed.
    */
+  V8_DEPRECATED(
+      "Use Isolate::SetModifyCodeGenerationFromStringsCallback instead. "
+      "See http://crbug.com/v8/10096.")
   void SetAllowCodeGenerationFromStringsCallback(
       AllowCodeGenerationFromStringsCallback callback);
   void SetModifyCodeGenerationFromStringsCallback(

src/runtime/runtime-test.cc

@@ -1013,18 +1013,25 @@ RUNTIME_FUNCTION(Runtime_IsAsmWasmCode) {
 }
 
 namespace {
-bool DisallowCodegenFromStringsCallback(v8::Local<v8::Context> context,
-                                        v8::Local<v8::String> source) {
-  return false;
+
+v8::ModifyCodeGenerationFromStringsResult DisallowCodegenFromStringsCallback(
+    v8::Local<v8::Context> context, v8::Local<v8::Value> source) {
+  return {false, {}};
 }
+
+bool DisallowWasmCodegenFromStringsCallback(v8::Local<v8::Context> context,
+                                            v8::Local<v8::String> source) {
+  return false;
 }
 
+}  // namespace
+
 RUNTIME_FUNCTION(Runtime_DisallowCodegenFromStrings) {
   SealHandleScope shs(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_BOOLEAN_ARG_CHECKED(flag, 0);
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
-  v8_isolate->SetAllowCodeGenerationFromStringsCallback(
+  v8_isolate->SetModifyCodeGenerationFromStringsCallback(
       flag ? DisallowCodegenFromStringsCallback : nullptr);
   return ReadOnlyRoots(isolate).undefined_value();
 }
@@ -1035,7 +1042,7 @@ RUNTIME_FUNCTION(Runtime_DisallowWasmCodegen) {
   CONVERT_BOOLEAN_ARG_CHECKED(flag, 0);
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
   v8_isolate->SetAllowWasmCodeGenerationCallback(
-      flag ? DisallowCodegenFromStringsCallback : nullptr);
+      flag ? DisallowWasmCodegenFromStringsCallback : nullptr);
   return ReadOnlyRoots(isolate).undefined_value();
 }
 

test/cctest/test-api.cc

@@ -19309,19 +19309,21 @@ void CheckCodeGenerationDisallowed() {
 
 char first_fourty_bytes[41];
 
-bool CodeGenerationAllowed(Local<Context> context, Local<String> source) {
+v8::ModifyCodeGenerationFromStringsResult CodeGenerationAllowed(
+    Local<Context> context, Local<Value> source) {
   String::Utf8Value str(CcTest::isolate(), source);
   size_t len = std::min(sizeof(first_fourty_bytes) - 1,
                         static_cast<size_t>(str.length()));
   strncpy(first_fourty_bytes, *str, len);
   first_fourty_bytes[len] = 0;
   ApiTestFuzzer::Fuzz();
-  return true;
+  return {true, {}};
 }
 
-bool CodeGenerationDisallowed(Local<Context> context, Local<String> source) {
+v8::ModifyCodeGenerationFromStringsResult CodeGenerationDisallowed(
+    Local<Context> context, Local<Value> source) {
   ApiTestFuzzer::Fuzz();
-  return false;
+  return {false, {}};
 }
 
 v8::ModifyCodeGenerationFromStringsResult ModifyCodeGeneration(
@@ -19373,13 +19375,13 @@ THREADED_TEST(AllowCodeGenFromStrings) {
 
   // Disallow but setting a global callback that will allow the calls.
   context->AllowCodeGenerationFromStrings(false);
-  context->GetIsolate()->SetAllowCodeGenerationFromStringsCallback(
+  context->GetIsolate()->SetModifyCodeGenerationFromStringsCallback(
       &CodeGenerationAllowed);
   CHECK(!context->IsCodeGenerationFromStringsAllowed());
   CheckCodeGenerationAllowed();
 
   // Set a callback that disallows the code generation.
-  context->GetIsolate()->SetAllowCodeGenerationFromStringsCallback(
+  context->GetIsolate()->SetModifyCodeGenerationFromStringsCallback(
       &CodeGenerationDisallowed);
   CHECK(!context->IsCodeGenerationFromStringsAllowed());
   CheckCodeGenerationDisallowed();
@@ -19429,7 +19431,7 @@ TEST(SetErrorMessageForCodeGenFromStrings) {
 
   Local<String> message = v8_str("Message");
   Local<String> expected_message = v8_str("Uncaught EvalError: Message");
-  context->GetIsolate()->SetAllowCodeGenerationFromStringsCallback(
+  context->GetIsolate()->SetModifyCodeGenerationFromStringsCallback(
       &CodeGenerationDisallowed);
   context->AllowCodeGenerationFromStrings(false);
   context->SetErrorMessageForCodeGenerationFromStrings(message);
@@ -19445,7 +19447,7 @@ TEST(CaptureSourceForCodeGenFromStrings) {
   v8::HandleScope scope(context->GetIsolate());
   TryCatch try_catch(context->GetIsolate());
 
-  context->GetIsolate()->SetAllowCodeGenerationFromStringsCallback(
+  context->GetIsolate()->SetModifyCodeGenerationFromStringsCallback(
       &CodeGenerationAllowed);
   context->AllowCodeGenerationFromStrings(false);
   CompileRun("eval('42')");

test/cctest/wasm/test-wasm-serialization.cc

@@ -263,7 +263,7 @@ TEST(BlockWasmCodeGenAtDeserialization) {
   WasmSerializationTest test;
   {
     HandleScope scope(test.current_isolate());
-    test.current_isolate_v8()->SetAllowCodeGenerationFromStringsCallback(False);
+    test.current_isolate_v8()->SetAllowWasmCodeGenerationCallback(False);
     v8::MaybeLocal<v8::WasmModuleObject> nothing = test.Deserialize();
     CHECK(nothing.IsEmpty());
   }

test/mjsunit/wasm/asm-with-wasm-off.js

@@ -4,10 +4,10 @@
 
 // Flags: --noexpose-wasm --validate-asm --allow-natives-syntax
 
-// NOTE: This is in its own file because it calls %DisallowCodegenFromStrings,
-// which messes with the isolate's state.
+// NOTE: This is in its own file because it calls %DisallowWasmCodegen, which
+// messes with the isolate's state.
 (function testAsmWithWasmOff() {
-  %DisallowCodegenFromStrings(true);
+  %DisallowWasmCodegen(true);
   function Module() {
     'use asm';
     function foo() {

test/mjsunit/wasm/disallow-codegen.js

@@ -18,33 +18,17 @@ let buffer = (function CreateBuffer() {
   return builder.toBuffer();
 })();
 
-%DisallowCodegenFromStrings(true);
 %DisallowWasmCodegen(true);
 
-async function SyncTestOk() {
+function SyncTestOk() {
   print('sync module compile (ok)...');
-  %DisallowCodegenFromStrings(false);
   %DisallowWasmCodegen(false);
   let module = new WebAssembly.Module(buffer);
   assertInstanceof(module, WebAssembly.Module);
 }
 
-async function SyncTestFail() {
-  print('sync module compile (fail)...');
-  %DisallowCodegenFromStrings(true);
-  %DisallowWasmCodegen(false);
-  try {
-    let module = new WebAssembly.Module(buffer);
-    assertUnreachable();
-  } catch (e) {
-    print("  " + e);
-    assertInstanceof(e, WebAssembly.CompileError);
-  }
-}
-
-async function SyncTestWasmFail(disallow_codegen) {
+function SyncTestWasmFail() {
   print('sync wasm module compile (fail)...');
-  %DisallowCodegenFromStrings(disallow_codegen);
   %DisallowWasmCodegen(true);
   try {
     let module = new WebAssembly.Module(buffer);
@@ -57,7 +41,6 @@ async function SyncTestWasmFail(disallow_codegen) {
 
 async function AsyncTestOk() {
   print('async module compile (ok)...');
-  %DisallowCodegenFromStrings(false);
   %DisallowWasmCodegen(false);
   let promise = WebAssembly.compile(buffer);
   assertPromiseResult(
@@ -66,7 +49,6 @@ async function AsyncTestOk() {
 
 async function AsyncTestWithInstantiateOk() {
   print('async module instantiate (ok)...');
-  %DisallowCodegenFromStrings(false);
   %DisallowWasmCodegen(false);
   let promise = WebAssembly.instantiate(buffer);
   assertPromiseResult(
@@ -74,35 +56,8 @@ async function AsyncTestWithInstantiateOk() {
       module => assertInstanceof(module.instance, WebAssembly.Instance));
 }
 
-async function AsyncTestFail() {
-  print('async module compile (fail)...');
-  %DisallowCodegenFromStrings(true);
-  %DisallowWasmCodegen(false);
-  try {
-    let m = await WebAssembly.compile(buffer);
-    assertUnreachable();
-  } catch (e) {
-    print("  " + e);
-    assertInstanceof(e, WebAssembly.CompileError);
-  }
-}
-
-async function AsyncTestWithInstantiateFail() {
-  print('async module instantiate (fail)...');
-  %DisallowCodegenFromStrings(true);
-  %DisallowWasmCodegen(false);
-  try {
-    let m = await WebAssembly.instantiate(buffer);
-    assertUnreachable();
-  } catch (e) {
-    print("  " + e);
-    assertInstanceof(e, WebAssembly.CompileError);
-  }
-}
-
-async function AsyncTestWasmFail(disallow_codegen) {
+async function AsyncTestWasmFail() {
   print('async wasm module compile (fail)...');
-  %DisallowCodegenFromStrings(disallow_codegen);
   %DisallowWasmCodegen(true);
   try {
     let m = await WebAssembly.compile(buffer);
@@ -113,9 +68,8 @@ async function AsyncTestWasmFail(disallow_codegen) {
   }
 }
 
-async function AsyncTestWasmWithInstantiateFail(disallow_codegen) {
+async function AsyncTestWasmWithInstantiateFail() {
   print('async wasm module instantiate (fail)...');
-  %DisallowCodegenFromStrings(disallow_codegen);
   %DisallowWasmCodegen(true);
   try {
     let m = await WebAssembly.instantiate(buffer);
@@ -130,20 +84,18 @@ async function StreamingTestOk() {
   print('streaming module compile (ok)...');
   // TODO(titzer): compileStreaming must be supplied by embedder.
   // (and it takes a response, not a buffer)
-  %DisallowCodegenFromStrings(false);
   %DisallowWasmCodegen(false);
   if ("Function" != typeof WebAssembly.compileStreaming) {
     print("  no embedder for streaming compilation");
     return;
   }
   let promise = WebAssembly.compileStreaming(buffer);
-  assertPromiseResult(
+  await assertPromiseResult(
     promise, module => assertInstanceof(module, WebAssembly.Module));
 }
 
 async function StreamingTestFail() {
   print('streaming module compile (fail)...');
-  %DisallowCodegenFromStrings(true);
   %DisallowWasmCodegen(false);
   // TODO(titzer): compileStreaming must be supplied by embedder.
   // (and it takes a response, not a buffer)
@@ -161,9 +113,8 @@ async function StreamingTestFail() {
 }
 
 
-async function StreamingTestWasmFail(disallow_codegen) {
+async function StreamingTestWasmFail() {
   print('streaming wasm module compile (fail)...');
-  %DisallowCodegenFromStrings(disallow_codegen);
   %DisallowWasmCodegen(true);
   // TODO(titzer): compileStreaming must be supplied by embedder.
   // (and it takes a response, not a buffer)
@@ -181,23 +132,14 @@ async function StreamingTestWasmFail(disallow_codegen) {
 }
 
 async function RunAll() {
-  await SyncTestOk();
-  await SyncTestFail();
-  await AsyncTestOk();
-  await AsyncTestWithInstantiateOk();
-  await AsyncTestFail();
-  await AsyncTestWithInstantiateFail();
+  SyncTestOk();
+  AsyncTestOk();
   await StreamingTestOk();
   await StreamingTestFail();
-
-  disallow_codegen = false;
-  for (count = 0; count < 2; ++count) {
-    SyncTestWasmFail(disallow_codegen);
-    AsyncTestWasmFail(disallow_codegen);
-    AsyncTestWasmWithInstantiateFail(disallow_codegen);
-    StreamingTestWasmFail(disallow_codegen)
-    disallow_codegen = true;
-  }
+  SyncTestWasmFail();
+  await AsyncTestWasmFail();
+  await AsyncTestWasmWithInstantiateFail();
+  await StreamingTestWasmFail()
 }
 
 assertPromiseResult(RunAll());