[api] Deprecate v8::ScriptCompiler::CompileFunctionInContext

- Introduce v8::ScriptCompiler::CompileFunction
- Deprecate v8::ScriptCompiler::CompileFunctionInContext
- Add v8::Function::GetUnboundScript
- Add v8::Script::GetResourceName

The ScriptOrModule out-parameter is only used by NodeJS since we don't
allow arbitrary objects has host-defined options and they need a way to
keep the options alive.

This CL deprecates the out-parameter and adds helper methods to
address the most common use-cases.

The final fix still requires more fundamental changes on how host-defined
options are handled.

Bug: chromium:1244145
Change-Id: Id29de53521ad626c41391b8300146ee37a1b8a51
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3245117Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Auto-Submit: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77564}

include/v8-function.h

@@ -18,6 +18,7 @@
 namespace v8 {
 
 class Context;
+class UnboundScript;
 
 /**
  * A JavaScript function object (ECMA-262, 15.3).
@@ -58,6 +59,8 @@ class V8_EXPORT Function : public Object {
   void SetName(Local<String> name);
   Local<Value> GetName() const;
 
+  MaybeLocal<UnboundScript> GetUnboundScript() const;
+
   /**
    * Name inferred from variable or property assignment of this function.
    * Used to facilitate debugging and profiling of JavaScript code written

include/v8-script.h

@@ -345,6 +345,12 @@ class V8_EXPORT Script {
    * Returns the corresponding context-unbound script.
    */
   Local<UnboundScript> GetUnboundScript();
+
+  /**
+   * The name that was passed by the embedder as ResourceName to the
+   * ScriptOrigin. This can be either a v8::String or v8::Undefined.
+   */
+  Local<Value> GetResourceName();
 };
 
 enum class ScriptType { kClassic, kModule };
@@ -682,6 +688,7 @@ class V8_EXPORT ScriptCompiler {
    * It is possible to specify multiple context extensions (obj in the above
    * example).
    */
+  V8_DEPRECATE_SOON("Use CompileFunction")
   static V8_WARN_UNUSED_RESULT MaybeLocal<Function> CompileFunctionInContext(
       Local<Context> context, Source* source, size_t arguments_count,
       Local<String> arguments[], size_t context_extension_count,
@@ -689,6 +696,12 @@ class V8_EXPORT ScriptCompiler {
       CompileOptions options = kNoCompileOptions,
       NoCacheReason no_cache_reason = kNoCacheNoReason,
       Local<ScriptOrModule>* script_or_module_out = nullptr);
+  static V8_WARN_UNUSED_RESULT MaybeLocal<Function> CompileFunction(
+      Local<Context> context, Source* source, size_t arguments_count = 0,
+      Local<String> arguments[] = nullptr, size_t context_extension_count = 0,
+      Local<Object> context_extensions[] = nullptr,
+      CompileOptions options = kNoCompileOptions,
+      NoCacheReason no_cache_reason = kNoCacheNoReason);
 
   /**
    * Creates and returns code cache for the specified unbound_script.
@@ -707,7 +720,7 @@ class V8_EXPORT ScriptCompiler {
 
   /**
    * Creates and returns code cache for the specified function that was
-   * previously produced by CompileFunctionInContext.
+   * previously produced by CompileFunction.
    * This will return nullptr if the script cannot be serialized. The
    * CachedData returned by this function should be owned by the caller.
    */
@@ -717,6 +730,13 @@ class V8_EXPORT ScriptCompiler {
   static V8_WARN_UNUSED_RESULT MaybeLocal<UnboundScript> CompileUnboundInternal(
       Isolate* isolate, Source* source, CompileOptions options,
       NoCacheReason no_cache_reason);
+
+  static V8_WARN_UNUSED_RESULT MaybeLocal<Function> CompileFunctionInternal(
+      Local<Context> context, Source* source, size_t arguments_count,
+      Local<String> arguments[], size_t context_extension_count,
+      Local<Object> context_extensions[], CompileOptions options,
+      NoCacheReason no_cache_reason,
+      Local<ScriptOrModule>* script_or_module_out);
 };
 
 ScriptCompiler::Source::Source(Local<String> string, const ScriptOrigin& origin,

src/api/api.cc

@@ -2127,12 +2127,23 @@ Local<PrimitiveArray> ScriptOrModule::GetHostDefinedOptions() {
 }
 
 Local<UnboundScript> Script::GetUnboundScript() {
-  i::Handle<i::Object> obj = Utils::OpenHandle(this);
-  i::SharedFunctionInfo sfi = i::JSFunction::cast(*obj).shared();
+  i::DisallowGarbageCollection no_gc;
+  i::Handle<i::JSFunction> obj = Utils::OpenHandle(this);
+  i::SharedFunctionInfo sfi = (*obj).shared();
   i::Isolate* isolate = sfi.GetIsolate();
   return ToApiHandle<UnboundScript>(i::handle(sfi, isolate));
 }
 
+Local<Value> Script::GetResourceName() {
+  i::DisallowGarbageCollection no_gc;
+  i::Handle<i::JSFunction> func = Utils::OpenHandle(this);
+  i::SharedFunctionInfo sfi = (*func).shared();
+  i::Isolate* isolate = func->GetIsolate();
+  CHECK(sfi.script().IsScript());
+  return ToApiHandle<Value>(
+      i::handle(i::Script::cast(sfi.script()).name(), isolate));
+}
+
 // static
 Local<PrimitiveArray> PrimitiveArray::New(Isolate* v8_isolate, int length) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
@@ -2594,9 +2605,32 @@ bool IsIdentifier(i::Isolate* isolate, i::Handle<i::String> string) {
   }
   return true;
 }
-}  // anonymous namespace
+}  // namespace
+
+// static
+V8_WARN_UNUSED_RESULT MaybeLocal<Function> ScriptCompiler::CompileFunction(
+    Local<Context> context, Source* source, size_t arguments_count,
+    Local<String> arguments[], size_t context_extension_count,
+    Local<Object> context_extensions[], CompileOptions options,
+    NoCacheReason no_cache_reason) {
+  return CompileFunctionInternal(context, source, arguments_count, arguments,
+                                 context_extension_count, context_extensions,
+                                 options, no_cache_reason, nullptr);
+}
 
+// static
 MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
+    Local<Context> context, Source* source, size_t arguments_count,
+    Local<String> arguments[], size_t context_extension_count,
+    Local<Object> context_extensions[], CompileOptions options,
+    NoCacheReason no_cache_reason,
+    Local<ScriptOrModule>* script_or_module_out) {
+  return CompileFunctionInternal(
+      context, source, arguments_count, arguments, context_extension_count,
+      context_extensions, options, no_cache_reason, script_or_module_out);
+}
+
+MaybeLocal<Function> ScriptCompiler::CompileFunctionInternal(
     Local<Context> v8_context, Source* source, size_t arguments_count,
     Local<String> arguments[], size_t context_extension_count,
     Local<Object> context_extensions[], CompileOptions options,
@@ -2605,7 +2639,7 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
   Local<Function> result;
 
   {
-    PREPARE_FOR_EXECUTION(v8_context, ScriptCompiler, CompileFunctionInContext,
+    PREPARE_FOR_EXECUTION(v8_context, ScriptCompiler, CompileFunction,
                           Function);
     TRACE_EVENT_CALL_STATS_SCOPED(isolate, "v8", "V8.ScriptCompiler");
 
@@ -5288,6 +5322,14 @@ int Function::GetScriptColumnNumber() const {
   return kLineOffsetNotFound;
 }
 
+MaybeLocal<UnboundScript> Function::GetUnboundScript() const {
+  i::Handle<i::Object> self = Utils::OpenHandle(this);
+  if (!self->IsJSFunction()) return MaybeLocal<UnboundScript>();
+  i::SharedFunctionInfo sfi = i::JSFunction::cast(*self).shared();
+  i::Isolate* isolate = sfi.GetIsolate();
+  return ToApiHandle<UnboundScript>(i::handle(sfi, isolate));
+}
+
 int Function::ScriptId() const {
   i::JSReceiver self = *Utils::OpenHandle(this);
   if (!self.IsJSFunction()) return v8::UnboundScript::kNoScriptId;

src/debug/debug.cc

@@ -2160,7 +2160,7 @@ namespace {
 debug::Location GetDebugLocation(Handle<Script> script, int source_position) {
   Script::PositionInfo info;
   Script::GetPositionInfo(script, source_position, &info, Script::WITH_OFFSET);
-  // V8 provides ScriptCompiler::CompileFunctionInContext method which takes
+  // V8 provides ScriptCompiler::CompileFunction method which takes
   // expression and compile it as anonymous function like (function() ..
   // expression ..). To produce correct locations for stmts inside of this
   // expression V8 compile this function with negative offset. Instead of stmt

src/logging/runtime-call-stats.h

@@ -245,7 +245,7 @@ class RuntimeCallTimer final {
   V(RegExp_Exec)                                           \
   V(RegExp_New)                                            \
   V(ScriptCompiler_Compile)                                \
-  V(ScriptCompiler_CompileFunctionInContext)               \
+  V(ScriptCompiler_CompileFunction)                        \
   V(ScriptCompiler_CompileUnbound)                         \
   V(Script_Run)                                            \
   V(Set_Add)                                               \

test/cctest/test-debug.cc

@@ -3038,8 +3038,7 @@ TEST(DebugBreakInWrappedScript) {
   {
     v8::ScriptCompiler::Source script_source(v8_str(source));
     v8::Local<v8::Function> fun =
-        v8::ScriptCompiler::CompileFunctionInContext(
-            env.local(), &script_source, 0, nullptr, 0, nullptr)
+        v8::ScriptCompiler::CompileFunction(env.local(), &script_source)
             .ToLocalChecked();
     v8::Local<v8::Value> result =
         fun->Call(env.local(), env->Global(), 0, nullptr).ToLocalChecked();
@@ -5373,8 +5372,7 @@ TEST(TerminateOnResumeAtException) {
 
     v8::ScriptCompiler::Source script_source(v8_str(source));
     v8::Local<v8::Function> foo =
-        v8::ScriptCompiler::CompileFunctionInContext(
-            env.local(), &script_source, 0, nullptr, 0, nullptr)
+        v8::ScriptCompiler::CompileFunction(env.local(), &script_source)
             .ToLocalChecked();
 
     v8::MaybeLocal<v8::Value> val =
@@ -5620,8 +5618,7 @@ TEST(TerminateOnResumeFromOtherThread) {
 
     v8::ScriptCompiler::Source script_source(v8_str(source));
     v8::Local<v8::Function> foo =
-        v8::ScriptCompiler::CompileFunctionInContext(
-            env.local(), &script_source, 0, nullptr, 0, nullptr)
+        v8::ScriptCompiler::CompileFunction(env.local(), &script_source)
             .ToLocalChecked();
 
     v8::MaybeLocal<v8::Value> val =
@@ -5675,8 +5672,7 @@ TEST(TerminateOnResumeAtInterruptFromOtherThread) {
 
     v8::ScriptCompiler::Source script_source(v8_str(source));
     v8::Local<v8::Function> foo =
-        v8::ScriptCompiler::CompileFunctionInContext(
-            env.local(), &script_source, 0, nullptr, 0, nullptr)
+        v8::ScriptCompiler::CompileFunction(env.local(), &script_source)
             .ToLocalChecked();
 
     CHECK(timeout_thread.Start());

test/cctest/test-log.cc

@@ -633,8 +633,8 @@ UNINITIALIZED_TEST(LogInterpretedFramesNativeStackWithSerialization) {
 
       v8::ScriptCompiler::Source script_source(source, origin, cache);
       v8::Local<v8::Function> fun =
-          v8::ScriptCompiler::CompileFunctionInContext(
-              context, &script_source, 1, &arg_str, 0, nullptr, options)
+          v8::ScriptCompiler::CompileFunction(context, &script_source, 1,
+                                              &arg_str, 0, nullptr, options)
               .ToLocalChecked();
       if (has_cache) {
         logger.StopLogging();

test/cctest/test-serialize.cc

@@ -4255,7 +4255,7 @@ TEST(WeakArraySerializationInCodeCache) {
   delete cache;
 }
 
-TEST(CachedCompileFunctionInContext) {
+TEST(CachedCompileFunction) {
   DisableAlwaysOpt();
   LocalContext env;
   Isolate* isolate = CcTest::i_isolate();
@@ -4270,9 +4270,9 @@ TEST(CachedCompileFunctionInContext) {
   {
     v8::ScriptCompiler::Source script_source(source);
     v8::Local<v8::Function> fun =
-        v8::ScriptCompiler::CompileFunctionInContext(
-            env.local(), &script_source, 1, &arg_str, 0, nullptr,
-            v8::ScriptCompiler::kEagerCompile)
+        v8::ScriptCompiler::CompileFunction(env.local(), &script_source, 1,
+                                            &arg_str, 0, nullptr,
+                                            v8::ScriptCompiler::kEagerCompile)
             .ToLocalChecked();
     cache = v8::ScriptCompiler::CreateCodeCacheForFunction(fun);
   }
@@ -4281,7 +4281,7 @@ TEST(CachedCompileFunctionInContext) {
     DisallowCompilation no_compile_expected(isolate);
     v8::ScriptCompiler::Source script_source(source, cache);
     v8::Local<v8::Function> fun =
-        v8::ScriptCompiler::CompileFunctionInContext(
+        v8::ScriptCompiler::CompileFunction(
             env.local(), &script_source, 1, &arg_str, 0, nullptr,
             v8::ScriptCompiler::kConsumeCodeCache)
             .ToLocalChecked();