src: update std::vector<v8::Local<T>> to use v8::LocalVector<T>

A follow up of #57578 to
replace all std::vector<v8::Local<T>> to use v8::LocalVector<T>

PR-URL: #57642
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: Aditi-1400

src/README.md

@@ -151,6 +151,25 @@ is done executing. `Local` handles can only be allocated on the C++ stack.
 Most of the V8 API uses `Local` handles to work with JavaScript values or return
 them from functions.
 
+Additionally, according to [V8 public API documentation][`v8::Local<T>`], local handles
+(`v8::Local<T>`) should **never** be allocated on the heap.
+
+This disallows heap-allocated data structures containing instances of `v8::Local`
+
+For example:
+
+```cpp
+// Don't do this
+std::vector<v8::Local<v8::Value>> v1;
+```
+
+Instead, it is recommended to use `v8::LocalVector<T>` provided by V8
+for such scenarios:
+
+```cpp
+v8::LocalVector<v8::Value> v1(isolate);
+```
+
 Whenever a `Local` handle is created, a `v8::HandleScope` or
 `v8::EscapableHandleScope` object must exist on the stack. The `Local` is then
 added to that scope and deleted along with it.
@@ -1176,6 +1195,7 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
 [`v8.h` in Code Search]: https://cs.chromium.org/chromium/src/v8/include/v8.h
 [`v8.h` in Node.js]: https://github.com/nodejs/node/blob/HEAD/deps/v8/include/v8.h
 [`v8.h` in V8]: https://github.com/v8/v8/blob/HEAD/include/v8.h
+[`v8::Local<T>`]: https://v8.github.io/api/head/classv8_1_1Local.html
 [`vm` module]: https://nodejs.org/api/vm.html
 [binding function]: #binding-functions
 [cleanup hooks]: #cleanup-hooks

src/node_contextify.cc

@@ -1165,7 +1165,7 @@ Maybe<void> StoreCodeCacheResult(
 MaybeLocal<Function> CompileFunction(Local<Context> context,
                                      Local<String> filename,
                                      Local<String> content,
-                                     std::vector<Local<String>>* parameters) {
+                                     LocalVector<String>* parameters) {
   ScriptOrigin script_origin(filename, 0, 0, true);
   ScriptCompiler::Source script_source(content, script_origin);
 
@@ -1474,7 +1474,7 @@ void ContextifyContext::CompileFunction(
   Context::Scope scope(parsing_context);
 
   // Read context extensions from buffer
-  std::vector<Local<Object>> context_extensions;
+  LocalVector<Object> context_extensions(isolate);
   if (!context_extensions_buf.IsEmpty()) {
     for (uint32_t n = 0; n < context_extensions_buf->Length(); n++) {
       Local<Value> val;
@@ -1485,7 +1485,7 @@ void ContextifyContext::CompileFunction(
   }
 
   // Read params from params buffer
-  std::vector<Local<String>> params;
+  LocalVector<String> params(isolate);
   if (!params_buf.IsEmpty()) {
     for (uint32_t n = 0; n < params_buf->Length(); n++) {
       Local<Value> val;
@@ -1517,22 +1517,24 @@ void ContextifyContext::CompileFunction(
   args.GetReturnValue().Set(result);
 }
 
-static std::vector<Local<String>> GetCJSParameters(IsolateData* data) {
-  return {
-      data->exports_string(),
-      data->require_string(),
-      data->module_string(),
-      data->__filename_string(),
-      data->__dirname_string(),
-  };
+static LocalVector<String> GetCJSParameters(IsolateData* data) {
+  LocalVector<String> result(data->isolate(),
+                             {
+                                 data->exports_string(),
+                                 data->require_string(),
+                                 data->module_string(),
+                                 data->__filename_string(),
+                                 data->__dirname_string(),
+                             });
+  return result;
 }
 
 Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
     Environment* env,
     Local<Context> parsing_context,
     ScriptCompiler::Source* source,
-    std::vector<Local<String>> params,
-    std::vector<Local<Object>> context_extensions,
+    LocalVector<String> params,
+    LocalVector<Object> context_extensions,
     ScriptCompiler::CompileOptions options,
     bool produce_cached_data,
     Local<Symbol> id_symbol,
@@ -1668,7 +1670,7 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(
     options = ScriptCompiler::kConsumeCodeCache;
   }
 
-  std::vector<Local<String>> params;
+  LocalVector<String> params(isolate);
   if (is_cjs_scope) {
     params = GetCJSParameters(env->isolate_data());
   }

src/node_contextify.h

@@ -92,8 +92,8 @@ class ContextifyContext : public BaseObject {
       Environment* env,
       v8::Local<v8::Context> parsing_context,
       v8::ScriptCompiler::Source* source,
-      std::vector<v8::Local<v8::String>> params,
-      std::vector<v8::Local<v8::Object>> context_extensions,
+      v8::LocalVector<v8::String> params,
+      v8::LocalVector<v8::Object> context_extensions,
       v8::ScriptCompiler::CompileOptions options,
       bool produce_cached_data,
       v8::Local<v8::Symbol> id_symbol,
@@ -189,7 +189,7 @@ v8::MaybeLocal<v8::Function> CompileFunction(
     v8::Local<v8::Context> context,
     v8::Local<v8::String> filename,
     v8::Local<v8::String> content,
-    std::vector<v8::Local<v8::String>>* parameters);
+    v8::LocalVector<v8::String>* parameters);
 
 }  // namespace contextify
 }  // namespace node

src/node_sea.cc

@@ -36,6 +36,7 @@ using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
+using v8::LocalVector;
 using v8::MaybeLocal;
 using v8::NewStringType;
 using v8::Object;
@@ -450,13 +451,15 @@ std::optional<std::string> GenerateCodeCache(std::string_view main_path,
     return std::nullopt;
   }
 
-  std::vector<Local<String>> parameters = {
-      FIXED_ONE_BYTE_STRING(isolate, "exports"),
-      FIXED_ONE_BYTE_STRING(isolate, "require"),
-      FIXED_ONE_BYTE_STRING(isolate, "module"),
-      FIXED_ONE_BYTE_STRING(isolate, "__filename"),
-      FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
-  };
+  LocalVector<String> parameters(
+      isolate,
+      {
+          FIXED_ONE_BYTE_STRING(isolate, "exports"),
+          FIXED_ONE_BYTE_STRING(isolate, "require"),
+          FIXED_ONE_BYTE_STRING(isolate, "module"),
+          FIXED_ONE_BYTE_STRING(isolate, "__filename"),
+          FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
+      });
 
   // TODO(RaisinTen): Using the V8 code cache prevents us from using `import()`
   // in the SEA code. Support it.

src/node_snapshotable.cc

@@ -41,6 +41,7 @@ using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
+using v8::LocalVector;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::SnapshotCreator;
@@ -1479,11 +1480,13 @@ void CompileSerializeMain(const FunctionCallbackInfo<Value>& args) {
   Local<Context> context = isolate->GetCurrentContext();
   // TODO(joyeecheung): do we need all of these? Maybe we would want a less
   // internal version of them.
-  std::vector<Local<String>> parameters = {
-      FIXED_ONE_BYTE_STRING(isolate, "require"),
-      FIXED_ONE_BYTE_STRING(isolate, "__filename"),
-      FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
-  };
+  LocalVector<String> parameters(
+      isolate,
+      {
+          FIXED_ONE_BYTE_STRING(isolate, "require"),
+          FIXED_ONE_BYTE_STRING(isolate, "__filename"),
+          FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
+      });
   Local<Function> fn;
   if (contextify::CompileFunction(context, filename, source, &parameters)
           .ToLocal(&fn)) {