Skip to content

Commit

Permalink
Get import assertions from v8::ModuleRequest and add them to blink::M…
Browse files Browse the repository at this point in the history
…oduleRequest

This is the first in a series of changes that will implement import
assertions integration in Blink and tie the assertions into the existing
JSON and CSS module prototypes.

See the import assertions spec [1], the HTML integration PR [2], and the
I2P [3].

This change retrieves the import assertions from V8 in
ModuleRequest::ModuleRequests and in the ResolveModuleCallback.  The
code to unpack them is moved inside blink::ModuleRequest to avoid code
duplication across those two places.  ModuleRequest is upgraded
from a struct to a class now that its constructors are no longer trivial.

ModuleRequest is passed to ModuleRecordResolver::Resolve in place of the
specifier, where the import assertions will eventually be used alongside
the specifier to resolve the module.

IsolateHolder now passes "type" as a supported assertion via V8
Isolate::CreateParams so that Blink will start receiving module "type"
assertions.

Subsequent changes will add the assertions to the module map key and
require the correct assertion to be present before a JSON or CSS module
can be successfully loaded.

[1] https://tc39.es/proposal-import-assertions
[2] whatwg/html#5883
[3] https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/Xft04J07Oh0/m/RLPMXVEiAAAJ

Bug: 1132413
Change-Id: Id41e9e470d3c4ed1988d8dfa1ef8fa26cd9d23a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2597778
Commit-Queue: Dan Clark <daniec@microsoft.com>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839060}
  • Loading branch information
dandclark authored and Chromium LUCI CQ committed Dec 23, 2020
1 parent 41f6bdf commit b4c95a4
Show file tree
Hide file tree
Showing 13 changed files with 140 additions and 19 deletions.
4 changes: 4 additions & 0 deletions gin/isolate_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ IsolateHolder::IsolateHolder(
params.only_terminate_in_safe_scope = true;
params.embedder_wrapper_type_index = kWrapperInfoIndex;
params.embedder_wrapper_object_index = kEncodedValueIndex;
params.supported_import_assertions = {kSupportedImportAssertions.begin(),
kSupportedImportAssertions.end()};

v8::Isolate::Initialize(isolate_, params);
}
Expand Down Expand Up @@ -118,4 +120,6 @@ void IsolateHolder::EnableIdleTasks(
isolate_data_->EnableIdleTasks(std::move(idle_task_runner));
}

constexpr std::array<const char*, 1> IsolateHolder::kSupportedImportAssertions;

} // namespace gin
8 changes: 8 additions & 0 deletions gin/public/isolate_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef GIN_PUBLIC_ISOLATE_HOLDER_H_
#define GIN_PUBLIC_ISOLATE_HOLDER_H_

#include <array>
#include <memory>

#include "base/macros.h"
Expand Down Expand Up @@ -120,6 +121,13 @@ class GIN_EXPORT IsolateHolder {
AccessMode access_mode_;
IsolateType isolate_type_;

// Equivalent to the list returned by HostGetSupportedImportAssertions:
// https://tc39.es/proposal-import-assertions/#sec-hostgetsupportedimportassertions
// This will be used to indicate to V8 that Blink wants to receive
// module "type" import assertions.
static constexpr std::array<const char*, 1> kSupportedImportAssertions = {
"type"};

DISALLOW_COPY_AND_ASSIGN(IsolateHolder);
};

Expand Down
56 changes: 53 additions & 3 deletions third_party/blink/renderer/bindings/core/v8/module_record.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,15 @@ Vector<ModuleRequest> ModuleRecord::ModuleRequests(
TextPosition position(
OrdinalNumber::FromZeroBasedInt(v8_loc.GetLineNumber()),
OrdinalNumber::FromZeroBasedInt(v8_loc.GetColumnNumber()));
Vector<ImportAssertion> import_assertions =
ModuleRecord::ToBlinkImportAssertions(
script_state->GetContext(), record,
v8_module_request->GetImportAssertions());

requests.emplace_back(ToCoreString(v8_specifier), position);
requests.emplace_back(ToCoreString(v8_specifier), position,
import_assertions);
}

return requests;
}

Expand All @@ -166,14 +172,58 @@ v8::MaybeLocal<v8::Module> ModuleRecord::ResolveModuleCallback(
Modulator* modulator = Modulator::From(ScriptState::From(context));
DCHECK(modulator);

ModuleRequest module_request(ToCoreStringWithNullCheck(specifier),
TextPosition(),
ModuleRecord::ToBlinkImportAssertions(
context, referrer, import_assertions));

ExceptionState exception_state(isolate, ExceptionState::kExecutionContext,
"ModuleRecord", "resolveModuleCallback");
v8::Local<v8::Module> resolved =
modulator->GetModuleRecordResolver()->Resolve(
ToCoreStringWithNullCheck(specifier), referrer, exception_state);
modulator->GetModuleRecordResolver()->Resolve(module_request, referrer,
exception_state);
DCHECK(!resolved.IsEmpty());
DCHECK(!exception_state.HadException());

return resolved;
}

Vector<ImportAssertion> ModuleRecord::ToBlinkImportAssertions(
v8::Local<v8::Context> context,
v8::Local<v8::Module> record,
v8::Local<v8::FixedArray> v8_import_assertions) {
// The list of import assertions obtained from V8 for a ModuleRequest is given
// in the form: [key1, value1, source_offset1, key2, value2, source_offset2,
// ...].
constexpr int kV8AssertionEntrySize = 3;

Vector<ImportAssertion> import_assertions;
int number_of_import_assertions =
v8_import_assertions->Length() / kV8AssertionEntrySize;
import_assertions.ReserveInitialCapacity(number_of_import_assertions);
for (int i = 0; i < number_of_import_assertions; ++i) {
v8::Local<v8::String> v8_assertion_key =
v8_import_assertions->Get(context, i * kV8AssertionEntrySize)
.As<v8::String>();
v8::Local<v8::String> v8_assertion_value =
v8_import_assertions->Get(context, (i * kV8AssertionEntrySize) + 1)
.As<v8::String>();
int32_t v8_assertion_source_offset =
v8_import_assertions->Get(context, (i * kV8AssertionEntrySize) + 2)
.As<v8::Int32>()
->Value();
v8::Location v8_assertion_loc =
record->SourceOffsetToLocation(v8_assertion_source_offset);
TextPosition assertion_position(
OrdinalNumber::FromZeroBasedInt(v8_assertion_loc.GetLineNumber()),
OrdinalNumber::FromZeroBasedInt(v8_assertion_loc.GetColumnNumber()));

import_assertions.emplace_back(ToCoreString(v8_assertion_key),
ToCoreString(v8_assertion_value),
assertion_position);
}

return import_assertions;
}

} // namespace blink
5 changes: 5 additions & 0 deletions third_party/blink/renderer/bindings/core/v8/module_record.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ class CORE_EXPORT ModuleRecord final {
static v8::Local<v8::Value> V8Namespace(v8::Local<v8::Module> record);

private:
static Vector<ImportAssertion> ToBlinkImportAssertions(
v8::Local<v8::Context> context,
v8::Local<v8::Module> record,
v8::Local<v8::FixedArray> v8_import_assertions);

static v8::MaybeLocal<v8::Module> ResolveModuleCallback(
v8::Local<v8::Context>,
v8::Local<v8::String> specifier,
Expand Down
34 changes: 32 additions & 2 deletions third_party/blink/renderer/bindings/core/v8/module_record_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ class TestModuleRecordResolver final : public ModuleRecordResolver {
return nullptr;
}

v8::Local<v8::Module> Resolve(const String& specifier,
v8::Local<v8::Module> Resolve(const ModuleRequest& module_request,
v8::Local<v8::Module> module,
ExceptionState&) override {
specifiers_.push_back(specifier);
specifiers_.push_back(module_request.specifier);
return module_records_.TakeFirst()->NewLocal(isolate_);
}

Expand Down Expand Up @@ -139,7 +139,37 @@ TEST_P(ModuleRecordTest, moduleRequests) {
auto requests = ModuleRecord::ModuleRequests(scope.GetScriptState(), module);
EXPECT_EQ(2u, requests.size());
EXPECT_EQ("a", requests[0].specifier);
EXPECT_EQ(0u, requests[0].import_assertions.size());
EXPECT_EQ("b", requests[1].specifier);
EXPECT_EQ(0u, requests[1].import_assertions.size());
}

TEST_P(ModuleRecordTest, moduleRequestsWithImportAssertions) {
V8TestingScope scope;
v8::V8::SetFlagsFromString("--harmony-import-assertions");
const KURL js_url("https://example.com/foo.js");
v8::Local<v8::Module> module = ModuleTestBase::CompileModule(
scope.GetIsolate(),
"import 'a' assert { };"
"import 'b' assert { type: 'x'};"
"import 'c' assert { foo: 'y', type: 'z' };",
js_url);
ASSERT_FALSE(module.IsEmpty());

auto requests = ModuleRecord::ModuleRequests(scope.GetScriptState(), module);
EXPECT_EQ(3u, requests.size());
EXPECT_EQ("a", requests[0].specifier);
EXPECT_EQ(0u, requests[0].import_assertions.size());

EXPECT_EQ("b", requests[1].specifier);
EXPECT_EQ(1u, requests[1].import_assertions.size());
EXPECT_EQ("type", requests[1].import_assertions[0].key);
EXPECT_EQ("x", requests[1].import_assertions[0].value);

EXPECT_EQ("c", requests[2].specifier);
EXPECT_EQ(1u, requests[2].import_assertions.size());
EXPECT_EQ("type", requests[2].import_assertions[0].key);
EXPECT_EQ("z", requests[2].import_assertions[0].value);
}

TEST_P(ModuleRecordTest, instantiateNoDeps) {
Expand Down
25 changes: 23 additions & 2 deletions third_party/blink/renderer/bindings/core/v8/module_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,32 @@

namespace blink {

// An import assertion key/value pair per spec:
// https://tc39.es/proposal-import-assertions/
struct ImportAssertion {
String key;
String value;
TextPosition position;
ImportAssertion(const String& key,
const String& value,
const TextPosition& position)
: key(key), value(value), position(position) {}
};

// An instance of a ModuleRequest record:
// https://tc39.es/proposal-import-assertions/#sec-modulerequest-record
// Represents a module script's request to import a module given a specifier and
// list of import assertions.
struct ModuleRequest {
String specifier;
TextPosition position;
ModuleRequest(const String& specifier, const TextPosition& position)
: specifier(specifier), position(position) {}
Vector<ImportAssertion> import_assertions;
ModuleRequest(const String& specifier,
const TextPosition& position,
const Vector<ImportAssertion>& import_assertions)
: specifier(specifier),
position(position),
import_assertions(import_assertions) {}
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ class ModuleScriptLoaderTestModulator final : public DummyModulator {
void SetModuleRequests(const Vector<String>& requests) {
requests_.clear();
for (const String& request : requests) {
requests_.emplace_back(request, TextPosition::MinimumPosition());
requests_.emplace_back(request, TextPosition::MinimumPosition(),
Vector<ImportAssertion>());
}
}
Vector<ModuleRequest> ModuleRequestsFromModuleRecord(
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/script/module_map_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class TestModuleRecordResolver final : public ModuleRecordResolver {
return nullptr;
}

v8::Local<v8::Module> Resolve(const String& specifier,
v8::Local<v8::Module> Resolve(const ModuleRequest& module_request,
v8::Local<v8::Module> referrer,
ExceptionState&) override {
NOTREACHED();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class CORE_EXPORT ModuleRecordResolver
// Implements "Runtime Semantics: HostResolveImportedModule"
// https://tc39.github.io/ecma262/#sec-hostresolveimportedmodule
// This returns a null ModuleRecord when an exception is thrown.
virtual v8::Local<v8::Module> Resolve(const String& specifier,
virtual v8::Local<v8::Module> Resolve(const ModuleRequest& module_request,
v8::Local<v8::Module> referrer,
ExceptionState&) = 0;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ const ModuleScript* ModuleRecordResolverImpl::GetModuleScriptFromModuleRecord(
// <specdef
// href="https://html.spec.whatwg.org/C/#hostresolveimportedmodule(referencingscriptormodule,-specifier)">
v8::Local<v8::Module> ModuleRecordResolverImpl::Resolve(
const String& specifier,
const ModuleRequest& module_request,
v8::Local<v8::Module> referrer,
ExceptionState& exception_state) {
v8::Isolate* isolate = modulator_->GetScriptState()->GetIsolate();
DVLOG(1) << "ModuleRecordResolverImpl::resolve(specifier=\"" << specifier
<< ", referrer.hash="
DVLOG(1) << "ModuleRecordResolverImpl::resolve(specifier=\""
<< module_request.specifier << ", referrer.hash="
<< BoxedV8ModuleHash::GetHash(
MakeGarbageCollected<BoxedV8Module>(isolate, referrer))
<< ")";
Expand All @@ -91,7 +91,7 @@ v8::Local<v8::Module> ModuleRecordResolverImpl::Resolve(
// <spec step="3.3">Set base URL to referencing script's base URL.</spec>
// <spec step="5">Let url be the result of resolving a module specifier given
// base URL and specifier.</spec>
KURL url = referrer_module->ResolveModuleSpecifier(specifier);
KURL url = referrer_module->ResolveModuleSpecifier(module_request.specifier);

// <spec step="6">Assert: url is never failure, because resolving a module
// specifier must have been previously successful with these same two
Expand All @@ -100,6 +100,8 @@ v8::Local<v8::Module> ModuleRecordResolverImpl::Resolve(

// <spec step="7">Let resolved module script be moduleMap[url]. (This entry
// must exist for us to have gotten to this point.)</spec>
// TODO(crbug.com/1132413): Use import assertions along with URL to get
// resolved module script.
ModuleScript* module_script = modulator_->GetFetchedModuleScript(url);

// <spec step="8">Assert: resolved module script is a module script (i.e., is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class CORE_EXPORT ModuleRecordResolverImpl final

// Implements "Runtime Semantics: HostResolveImportedModule" per HTML spec.
// https://html.spec.whatwg.org/C/#hostresolveimportedmodule(referencingscriptormodule,-specifier))
v8::Local<v8::Module> Resolve(const String& specifier,
v8::Local<v8::Module> Resolve(const ModuleRequest& module_request,
v8::Local<v8::Module> referrer,
ExceptionState&) final;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ TEST_P(ModuleRecordResolverImplTest, RegisterResolveSuccess) {
CreateTargetModuleScript(modulator_, scope);
Modulator()->SetModuleScript(target_module_script);

v8::Local<v8::Module> resolved =
resolver->Resolve("./target.js", referrer_module_script->V8Module(),
scope.GetExceptionState());
v8::Local<v8::Module> resolved = resolver->Resolve(
ModuleRequest("./target.js", TextPosition(), Vector<ImportAssertion>()),
referrer_module_script->V8Module(), scope.GetExceptionState());
EXPECT_FALSE(scope.GetExceptionState().HadException());
EXPECT_EQ(resolved, target_module_script->V8Module());
EXPECT_EQ(1, modulator_->GetFetchedModuleScriptCalled());
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/testing/dummy_modulator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class EmptyModuleRecordResolver final : public ModuleRecordResolver {
return nullptr;
}

v8::Local<v8::Module> Resolve(const String& specifier,
v8::Local<v8::Module> Resolve(const ModuleRequest& module_request,
v8::Local<v8::Module> referrer,
ExceptionState&) override {
NOTREACHED();
Expand Down

0 comments on commit b4c95a4

Please sign in to comment.