Rollforward [compiler,api] Pass non-strings to the modifying callback when unconditional codegen is on.

Original change reviewed in https://chromium-review.googlesource.com/c/v8/v8/+/1917147.

Added an expect fail/pass for the tests that caused a revert in https://chromium-review.googlesource.com/c/chromium/src/+/2184229.

This reverts commit dd1b1de11f.

Bug: chromium:1024786
Change-Id: I7db6faa4c17c232a0fafd389fc4a26e8116852c7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2183910
Auto-Submit: Stefano Sanfilippo <ssanfilippo@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67637}

src/codegen/compiler.cc

@@ -1845,7 +1845,6 @@ bool CodeGenerationFromStringsAllowed(Isolate* isolate, Handle<Context> context,
 // (via v8::Isolate::SetModifyCodeGenerationFromStringsCallback)
 bool ModifyCodeGenerationFromStrings(Isolate* isolate, Handle<Context> context,
                                      Handle<i::Object>* source) {
-  DCHECK(context->allow_code_gen_from_strings().IsFalse(isolate));
   DCHECK(isolate->modify_code_gen_callback());
   DCHECK(source);
 
@@ -1886,10 +1885,8 @@ std::pair<MaybeHandle<String>, bool> Compiler::ValidateDynamicCompilationSource(
   // allow_code_gen_from_strings can be many things, so we'll always check
   // against the 'false' literal, so that e.g. undefined and 'true' are treated
   // the same.
-  if (!context->allow_code_gen_from_strings().IsFalse(isolate)) {
+  if (!context->allow_code_gen_from_strings().IsFalse(isolate) &&
-    if (!original_source->IsString()) {
+      original_source->IsString()) {
-      return {MaybeHandle<String>(), true};
-    }
     return {Handle<String>::cast(original_source), false};
   }
 

test/cctest/test-api.cc

@@ -19351,6 +19351,52 @@ TEST(ModifyCodeGenFromStrings) {
   try_catch.Reset();
 }
 
+v8::ModifyCodeGenerationFromStringsResult RejectStringsIncrementNumbers(
+    Local<Context> context, Local<Value> source) {
+  if (source->IsString()) {
+    return {false, v8::MaybeLocal<String>()};
+  }
+
+  Local<v8::Number> number;
+  if (!source->ToNumber(context).ToLocal(&number)) {
+    return {true, v8::MaybeLocal<String>()};
+  }
+
+  Local<v8::String> incremented =
+      String::NewFromUtf8(context->GetIsolate(),
+                          std::to_string(number->Value() + 1).c_str(),
+                          v8::NewStringType::kNormal)
+          .ToLocalChecked();
+
+  return {true, incremented};
+}
+
+TEST(AllowFromStringsOrModifyCodegen) {
+  LocalContext context;
+  v8::HandleScope scope(context->GetIsolate());
+  context->GetIsolate()->SetModifyCodeGenerationFromStringsCallback(
+      &RejectStringsIncrementNumbers);
+
+  context->AllowCodeGenerationFromStrings(false);
+
+  TryCatch try_catch(CcTest::isolate());
+  Local<Value> result = CompileRun("eval('40+2')");
+  CHECK(result.IsEmpty());
+  CHECK(try_catch.HasCaught());
+  try_catch.Reset();
+
+  result = CompileRun("eval(42)");
+  CHECK_EQ(43, result->Int32Value(context.local()).FromJust());
+
+  context->AllowCodeGenerationFromStrings(true);
+
+  result = CompileRun("eval('40+2')");
+  CHECK_EQ(42, result->Int32Value(context.local()).FromJust());
+
+  result = CompileRun("eval(42)");
+  CHECK_EQ(43, result->Int32Value(context.local()).FromJust());
+}
+
 TEST(SetErrorMessageForCodeGenFromStrings) {
   LocalContext context;
   v8::HandleScope scope(context->GetIsolate());