[api] Introduce new HostImportModuleDynamicallyCallback

The new callback does no longer use ScriptOrModule but rather gets the
host-defined options and the referrer name as separate arguments.

This brings us one step closer to deprecate ScriptOrModule and putting
the host-defined options in the script context.

- Add v8::Data::IsFixedArray and cast helpers
- Deprecate HostImportModuleDynamicallyWithImportAssertionsCallback soon
- Add Script::Run entry point that explicitly takes host-defined
  options (unused yet)

Bug: chromium:1244145
Change-Id: I08bc92cfb3b79d840e766fb71b8d91d301f4399c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3263893
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77733}

include/v8-callbacks.h

@@ -316,7 +316,7 @@ using SharedArrayBufferConstructorEnabledCallback =
     bool (*)(Local<Context> context);
 
 /**
- * HostImportModuleDynamicallyWithImportAssertionsCallback is called when we
+ * HostImportModuleDynamicallyCallback is called when we
  * require the embedder to load a module. This is used as part of the dynamic
  * import syntax.
  *
@@ -346,6 +346,10 @@ using HostImportModuleDynamicallyWithImportAssertionsCallback =
                             Local<ScriptOrModule> referrer,
                             Local<String> specifier,
                             Local<FixedArray> import_assertions);
+using HostImportModuleDynamicallyCallback = MaybeLocal<Promise> (*)(
+    Local<Context> context, Local<Data> host_defined_options,
+    Local<Value> resource_name, Local<String> specifier,
+    Local<FixedArray> import_assertions);
 
 /**
  * HostInitializeImportMetaObjectCallback is called the first time import.meta

include/v8-data.h

@@ -27,6 +27,11 @@ class V8_EXPORT Data {
    */
   bool IsModule() const;
 
+  /**
+   * Returns tru if this data is a |v8::FixedArray|
+   */
+  bool IsFixedArray() const;
+
   /**
    * Returns true if this data is a |v8::Private|.
    */
@@ -58,6 +63,16 @@ class V8_EXPORT FixedArray : public Data {
  public:
   int Length() const;
   Local<Data> Get(Local<Context> context, int i) const;
+
+  V8_INLINE static FixedArray* Cast(Data* data) {
+#ifdef V8_ENABLE_CHECKS
+    CheckCast(data);
+#endif
+    return reinterpret_cast<FixedArray*>(data);
+  }
+
+ private:
+  static void CheckCast(Data* obj);
 };
 
 }  // namespace v8

include/v8-isolate.h

@@ -628,8 +628,11 @@ class V8_EXPORT Isolate {
    * This specifies the callback called by the upcoming dynamic
    * import() language feature to load modules.
    */
+  V8_DEPRECATE_SOON("Use HostImportModuleDynamicallyCallback")
   void SetHostImportModuleDynamicallyCallback(
       HostImportModuleDynamicallyWithImportAssertionsCallback callback);
+  void SetHostImportModuleDynamicallyCallback(
+      HostImportModuleDynamicallyCallback callback);
 
   /**
    * This specifies the callback called by the upcoming import.meta

include/v8-script.h

@@ -340,6 +340,8 @@ class V8_EXPORT Script {
    * UnboundScript::BindToCurrentContext()).
    */
   V8_WARN_UNUSED_RESULT MaybeLocal<Value> Run(Local<Context> context);
+  V8_WARN_UNUSED_RESULT MaybeLocal<Value> Run(Local<Context> context,
+                                              Local<Data> host_defined_options);
 
   /**
    * Returns the corresponding context-unbound script.

src/api/api.cc

@@ -1033,6 +1033,9 @@ void SealHandleScope::operator delete(void*, size_t) { base::OS::Abort(); }
 void SealHandleScope::operator delete[](void*, size_t) { base::OS::Abort(); }
 
 bool Data::IsModule() const { return Utils::OpenHandle(this)->IsModule(); }
+bool Data::IsFixedArray() const {
+  return Utils::OpenHandle(this)->IsFixedArray();
+}
 
 bool Data::IsValue() const {
   i::DisallowGarbageCollection no_gc;
@@ -2051,6 +2054,11 @@ Local<Value> UnboundScript::GetSourceMappingURL() {
 }
 
 MaybeLocal<Value> Script::Run(Local<Context> context) {
+  return Run(context, Local<Data>());
+}
+
+MaybeLocal<Value> Script::Run(Local<Context> context,
+                              Local<Data> host_defined_options) {
   auto v8_isolate = context->GetIsolate();
   auto isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
   TRACE_EVENT_CALL_STATS_SCOPED(isolate, "v8", "V8.Execute");
@@ -2096,12 +2104,12 @@ MaybeLocal<Value> Script::Run(Local<Context> context) {
   }
 
   i::Handle<i::Object> receiver = isolate->global_proxy();
-  i::Handle<i::FixedArray> host_defined_options(
+  // TODO(cbruni, chromium:1244145): Remove once migrated to the context.
+  i::Handle<i::FixedArray> options(
       i::Script::cast(fun->shared().script()).host_defined_options(), isolate);
   Local<Value> result;
   has_pending_exception = !ToLocal<Value>(
-      i::Execution::CallScript(isolate, fun, receiver, host_defined_options),
-      &result);
+      i::Execution::CallScript(isolate, fun, receiver, options), &result);
 
   if (i::FLAG_script_delay_fraction > 0.0) {
     delta = v8::base::TimeDelta::FromMillisecondsD(
@@ -3890,6 +3898,12 @@ void v8::Private::CheckCast(v8::Data* that) {
       "v8::Private::Cast", "Value is not a Private");
 }
 
+void v8::FixedArray::CheckCast(v8::Data* that) {
+  i::Handle<i::Object> obj = Utils::OpenHandle(that);
+  Utils::ApiCheck(obj->IsFixedArray(), "v8::FixedArray::Cast",
+                  "Value is not a FixedArray");
+}
+
 void v8::ModuleRequest::CheckCast(v8::Data* that) {
   i::Handle<i::Object> obj = Utils::OpenHandle(that);
   Utils::ApiCheck(obj->IsModuleRequest(), "v8::ModuleRequest::Cast",
@@ -8647,6 +8661,12 @@ void Isolate::SetHostImportModuleDynamicallyCallback(
   isolate->SetHostImportModuleDynamicallyCallback(callback);
 }
 
+void Isolate::SetHostImportModuleDynamicallyCallback(
+    HostImportModuleDynamicallyCallback callback) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
+  isolate->SetHostImportModuleDynamicallyCallback(callback);
+}
+
 void Isolate::SetHostInitializeImportMetaObjectCallback(
     HostInitializeImportMetaObjectCallback callback) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);

src/d8/d8.cc

@@ -660,17 +660,17 @@ ScriptOrigin CreateScriptOrigin(Isolate* isolate, Local<String> resource_name,
                       false, false, type == v8::ScriptType::kModule, options);
 }
 
-bool IsValidHostDefinedOptions(Local<Context> context,
-                               Local<PrimitiveArray> options,
-                               Local<ScriptOrModule> script_or_module) {
-  Isolate* isolate = context->GetIsolate();
-  if (options->Length() != kHostDefinedOptionsLength) return false;
+bool IsValidHostDefinedOptions(Local<Context> context, Local<Data> options,
+                               Local<Value> resource_name) {
+  if (!options->IsFixedArray()) return false;
+  Local<FixedArray> array = options.As<FixedArray>();
+  if (array->Length() != kHostDefinedOptionsLength) return false;
   uint32_t magic = 0;
-  if (!options->Get(isolate, 0)->Uint32Value(context).To(&magic)) return false;
+  if (!array->Get(context, 0).As<Value>()->Uint32Value(context).To(&magic)) {
+    return false;
+  }
   if (magic != kHostDefinedOptionsMagicConstant) return false;
-  return options->Get(isolate, 1)
-      .As<String>()
-      ->StrictEquals(script_or_module->GetResourceName());
+  return array->Get(context, 1).As<String>()->StrictEquals(resource_name);
 }
 }  // namespace
 
@@ -1223,8 +1223,9 @@ void Shell::ModuleResolutionFailureCallback(
 }
 
 MaybeLocal<Promise> Shell::HostImportModuleDynamically(
-    Local<Context> context, Local<ScriptOrModule> script_or_module,
-    Local<String> specifier, Local<FixedArray> import_assertions) {
+    Local<Context> context, Local<Data> host_defined_options,
+    Local<Value> resource_name, Local<String> specifier,
+    Local<FixedArray> import_assertions) {
   Isolate* isolate = context->GetIsolate();
 
   MaybeLocal<Promise::Resolver> maybe_resolver =
@@ -1232,18 +1233,16 @@ MaybeLocal<Promise> Shell::HostImportModuleDynamically(
   Local<Promise::Resolver> resolver;
   if (!maybe_resolver.ToLocal(&resolver)) return MaybeLocal<Promise>();
 
-  Local<PrimitiveArray> host_defined_options =
-      script_or_module->GetHostDefinedOptions();
   if (!IsValidHostDefinedOptions(context, host_defined_options,
-                                 script_or_module)) {
+                                 resource_name)) {
     resolver
         ->Reject(context, v8::Exception::TypeError(String::NewFromUtf8Literal(
                               isolate, "Invalid host defined options")))
         .ToChecked();
   } else {
-    DynamicImportData* data = new DynamicImportData(
-        isolate, script_or_module->GetResourceName().As<String>(), specifier,
-        import_assertions, resolver);
+    DynamicImportData* data =
+        new DynamicImportData(isolate, resource_name.As<String>(), specifier,
+                              import_assertions, resolver);
     PerIsolateData::Get(isolate)->AddDynamicImportData(data);
     isolate->EnqueueMicrotask(Shell::DoHostImportModuleDynamically, data);
   }

src/d8/d8.h

@@ -607,8 +607,9 @@ class Shell : public i::AllStatic {
   static void MakeDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void RemoveDirectory(const v8::FunctionCallbackInfo<v8::Value>& args);
   static MaybeLocal<Promise> HostImportModuleDynamically(
-      Local<Context> context, Local<ScriptOrModule> script_or_module,
-      Local<String> specifier, Local<FixedArray> import_assertions);
+      Local<Context> context, Local<Data> host_defined_options,
+      Local<Value> resource_name, Local<String> specifier,
+      Local<FixedArray> import_assertions);
 
   static void ModuleResolutionSuccessCallback(
       const v8::FunctionCallbackInfo<v8::Value>& info);

src/execution/isolate.cc

@@ -4371,7 +4371,8 @@ MaybeHandle<JSPromise> Isolate::RunHostImportModuleDynamicallyCallback(
   v8::Local<v8::Context> api_context =
       v8::Utils::ToLocal(Handle<Context>::cast(native_context()));
   if (host_import_module_dynamically_with_import_assertions_callback_ ==
-      nullptr) {
+          nullptr &&
+      host_import_module_dynamically_callback_ == nullptr) {
     Handle<Object> exception =
         factory()->NewError(error_function(), MessageTemplate::kUnsupported);
     return NewRejectedPromise(this, api_context, exception);
@@ -4394,18 +4395,31 @@ MaybeHandle<JSPromise> Isolate::RunHostImportModuleDynamicallyCallback(
     clear_pending_exception();
     return NewRejectedPromise(this, api_context, exception);
   }
-  // TODO(cbruni, v8:12302): Avoid creating tempory ScriptOrModule objects.
-  auto script_or_module = i::Handle<i::ScriptOrModule>::cast(
-      this->factory()->NewStruct(i::SCRIPT_OR_MODULE_TYPE));
-  script_or_module->set_resource_name(referrer->name());
-  script_or_module->set_host_defined_options(referrer->host_defined_options());
-  ASSIGN_RETURN_ON_SCHEDULED_EXCEPTION_VALUE(
-      this, promise,
-      host_import_module_dynamically_with_import_assertions_callback_(
-          api_context, v8::Utils::ToLocal(script_or_module),
-          v8::Utils::ToLocal(specifier_str),
-          ToApiHandle<v8::FixedArray>(import_assertions_array)),
-      MaybeHandle<JSPromise>());
+  if (host_import_module_dynamically_callback_) {
+    ASSIGN_RETURN_ON_SCHEDULED_EXCEPTION_VALUE(
+        this, promise,
+        host_import_module_dynamically_callback_(
+            api_context,
+            v8::Utils::ToLocal(handle(referrer->host_defined_options(), this)),
+            v8::Utils::ToLocal(handle(referrer->name(), this)),
+            v8::Utils::ToLocal(specifier_str),
+            ToApiHandle<v8::FixedArray>(import_assertions_array)),
+        MaybeHandle<JSPromise>());
+  } else {
+    // TODO(cbruni, v8:12302): Avoid creating tempory ScriptOrModule objects.
+    auto script_or_module = i::Handle<i::ScriptOrModule>::cast(
+        this->factory()->NewStruct(i::SCRIPT_OR_MODULE_TYPE));
+    script_or_module->set_resource_name(referrer->name());
+    script_or_module->set_host_defined_options(
+        referrer->host_defined_options());
+    ASSIGN_RETURN_ON_SCHEDULED_EXCEPTION_VALUE(
+        this, promise,
+        host_import_module_dynamically_with_import_assertions_callback_(
+            api_context, v8::Utils::ToLocal(script_or_module),
+            v8::Utils::ToLocal(specifier_str),
+            ToApiHandle<v8::FixedArray>(import_assertions_array)),
+        MaybeHandle<JSPromise>());
+  }
   return v8::Utils::OpenHandle(*promise);
 }
 
@@ -4496,8 +4510,15 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(
 
 void Isolate::ClearKeptObjects() { heap()->ClearKeptObjects(); }
 
+void Isolate::SetHostImportModuleDynamicallyCallback(
+    HostImportModuleDynamicallyCallback callback) {
+  DCHECK_NULL(host_import_module_dynamically_with_import_assertions_callback_);
+  host_import_module_dynamically_callback_ = callback;
+}
+
 void Isolate::SetHostImportModuleDynamicallyCallback(
     HostImportModuleDynamicallyWithImportAssertionsCallback callback) {
+  DCHECK_NULL(host_import_module_dynamically_callback_);
   host_import_module_dynamically_with_import_assertions_callback_ = callback;
 }
 

src/execution/isolate.h

@@ -1693,6 +1693,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   void SetHostImportModuleDynamicallyCallback(
       HostImportModuleDynamicallyWithImportAssertionsCallback callback);
+  void SetHostImportModuleDynamicallyCallback(
+      HostImportModuleDynamicallyCallback callback);
   MaybeHandle<JSPromise> RunHostImportModuleDynamicallyCallback(
       Handle<Script> referrer, Handle<Object> specifier,
       MaybeHandle<Object> maybe_import_assertions_argument);
@@ -2063,6 +2065,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   v8::Isolate::AtomicsWaitCallback atomics_wait_callback_ = nullptr;
   void* atomics_wait_callback_data_ = nullptr;
   PromiseHook promise_hook_ = nullptr;
+  HostImportModuleDynamicallyCallback host_import_module_dynamically_callback_ =
+      nullptr;
   HostImportModuleDynamicallyWithImportAssertionsCallback
       host_import_module_dynamically_with_import_assertions_callback_ = nullptr;
   std::atomic<debug::CoverageMode> code_coverage_mode_{

test/cctest/test-api.cc

@@ -26380,13 +26380,13 @@ TEST(CorrectEnteredContext) {
 const int kCustomHostDefinedOptionsLengthForTesting = 7;
 
 v8::MaybeLocal<v8::Promise> HostImportModuleDynamicallyCallbackResolve(
-    Local<Context> context, Local<v8::ScriptOrModule> referrer,
-    Local<String> specifier, Local<FixedArray> import_assertions) {
-  CHECK(!referrer.IsEmpty());
-  String::Utf8Value referrer_utf8(
-      context->GetIsolate(), Local<String>::Cast(referrer->GetResourceName()));
+    Local<v8::Context> context, Local<v8::Data> host_defined_options,
+    Local<v8::Value> resource_name, Local<v8::String> specifier,
+    Local<v8::FixedArray> import_assertions) {
+  String::Utf8Value referrer_utf8(context->GetIsolate(),
+                                  resource_name.As<String>());
   CHECK_EQ(0, strcmp("www.google.com", *referrer_utf8));
-  CHECK_EQ(referrer->GetHostDefinedOptions()->Length(),
+  CHECK_EQ(host_defined_options.As<v8::FixedArray>()->Length(),
            kCustomHostDefinedOptionsLengthForTesting);
   CHECK(!specifier.IsEmpty());
   String::Utf8Value specifier_utf8(context->GetIsolate(), specifier);
@@ -26428,13 +26428,13 @@ TEST(DynamicImport) {
 
 v8::MaybeLocal<v8::Promise>
 HostImportModuleDynamicallyWithAssertionsCallbackResolve(
-    Local<Context> context, Local<v8::ScriptOrModule> referrer,
-    Local<String> specifier, Local<v8::FixedArray> import_assertions) {
-  CHECK(!referrer.IsEmpty());
-  String::Utf8Value referrer_utf8(
-      context->GetIsolate(), Local<String>::Cast(referrer->GetResourceName()));
+    Local<v8::Context> context, Local<v8::Data> host_defined_options,
+    Local<v8::Value> resource_name, Local<v8::String> specifier,
+    Local<v8::FixedArray> import_assertions) {
+  String::Utf8Value referrer_utf8(context->GetIsolate(),
+                                  resource_name.As<String>());
   CHECK_EQ(0, strcmp("www.google.com", *referrer_utf8));
-  CHECK_EQ(referrer->GetHostDefinedOptions()->Length(),
+  CHECK_EQ(host_defined_options.As<v8::FixedArray>()->Length(),
            kCustomHostDefinedOptionsLengthForTesting);
 
   CHECK(!specifier.IsEmpty());

test/cctest/test-modules.cc

@@ -937,12 +937,12 @@ void DoHostImportModuleDynamically(void* import_data) {
 }
 
 v8::MaybeLocal<v8::Promise> HostImportModuleDynamicallyCallbackResolve(
-    Local<Context> context, Local<v8::ScriptOrModule> referrer,
-    Local<String> specifier, Local<FixedArray> import_assertions) {
+    Local<Context> context, Local<Data> host_defined_options,
+    Local<Value> resource_name, Local<String> specifier,
+    Local<FixedArray> import_assertions) {
   Isolate* isolate = context->GetIsolate();
   Local<v8::Promise::Resolver> resolver =
       v8::Promise::Resolver::New(context).ToLocalChecked();
-
   DynamicImportData* data =
       new DynamicImportData(isolate, resolver, context, true);
   isolate->EnqueueMicrotask(DoHostImportModuleDynamically, data);
@@ -950,12 +950,12 @@ v8::MaybeLocal<v8::Promise> HostImportModuleDynamicallyCallbackResolve(
 }
 
 v8::MaybeLocal<v8::Promise> HostImportModuleDynamicallyCallbackReject(
-    Local<Context> context, Local<v8::ScriptOrModule> referrer,
-    Local<String> specifier, Local<FixedArray> import_assertions) {
+    Local<Context> context, Local<Data> host_defined_options,
+    Local<Value> resource_name, Local<String> specifier,
+    Local<FixedArray> import_assertions) {
   Isolate* isolate = context->GetIsolate();
   Local<v8::Promise::Resolver> resolver =
       v8::Promise::Resolver::New(context).ToLocalChecked();
-
   DynamicImportData* data =
       new DynamicImportData(isolate, resolver, context, false);
   isolate->EnqueueMicrotask(DoHostImportModuleDynamically, data);