Commit dae07e7a authored by Daniel Clark's avatar Daniel Clark Committed by Commit Bot

[modules][api] Remove import assertions sorting for HostImportModuleDynamically callback

Hosts are not supposed to rely on the ordering of import assertions list
received from V8. Thus, as a simplification, remove the sorting of the
import assertions passed to the HostImportModuleDynamically callback.

Update the corresponding test so that it doesn't require any particular
ordering of assertions.

Import asssertions for static imports will continue to be sorted. These
need to have a consistent ordering for purposes of deduplication in
SourceTextModuleDescriptor::module_requests_, so removing sorting of
these wouldn't simplify much.

Bug: v8:10958
Change-Id: I2cb07c4e68f24fa45152bf3f4321938bf94d84ba
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2653170Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Reviewed-by: 's avatarMarja Hölttä <marja@chromium.org>
Reviewed-by: 's avatarMythri Alle <mythria@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#72445}
parent 13a7cc1e
...@@ -4262,15 +4262,11 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument( ...@@ -4262,15 +4262,11 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(
GetKeysConversion::kConvertToString) GetKeysConversion::kConvertToString)
.ToHandleChecked(); .ToHandleChecked();
// Assertions passed to the host must be sorted by the code point order of the // The assertions will be passed to the host in the form: [key1,
// key of each entry. // value1, key2, value2, ...].
auto stringCompare = [this](Handle<String> lhs, Handle<String> rhs) { constexpr size_t kAssertionEntrySizeForDynamicImport = 2;
return String::Compare(this, lhs, rhs) == ComparisonResult::kLessThan; import_assertions_array = factory()->NewFixedArray(static_cast<int>(
}; assertion_keys->length() * kAssertionEntrySizeForDynamicImport));
std::map<Handle<String>, Handle<String>, decltype(stringCompare)>
sorted_assertions(stringCompare);
// Collect the assertions in the sorted map.
for (int i = 0; i < assertion_keys->length(); i++) { for (int i = 0; i < assertion_keys->length(); i++) {
Handle<String> assertion_key(String::cast(assertion_keys->get(i)), this); Handle<String> assertion_key(String::cast(assertion_keys->get(i)), this);
Handle<Object> assertion_value; Handle<Object> assertion_value;
...@@ -4288,25 +4284,10 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument( ...@@ -4288,25 +4284,10 @@ MaybeHandle<FixedArray> Isolate::GetImportAssertionsFromArgument(
return MaybeHandle<FixedArray>(); return MaybeHandle<FixedArray>();
} }
auto insertion_result = sorted_assertions.insert( import_assertions_array->set((i * kAssertionEntrySizeForDynamicImport),
std::make_pair(assertion_key, Handle<String>::cast(assertion_value))); *assertion_key);
import_assertions_array->set((i * kAssertionEntrySizeForDynamicImport) + 1,
// Duplicate keys are not expected here. *assertion_value);
CHECK(insertion_result.second);
}
// Move the assertions from the sorted map to the FixedArray that will be
// passed to the host. They will be stored in the array in the form: [key1,
// value1, key2, value2, ...].
constexpr size_t kAssertionEntrySizeForDynamicImport = 2;
import_assertions_array = factory()->NewFixedArray(static_cast<int>(
sorted_assertions.size() * kAssertionEntrySizeForDynamicImport));
int import_assertions_array_index = 0;
for (const auto& pair : sorted_assertions) {
import_assertions_array->set(import_assertions_array_index, *(pair.first));
import_assertions_array->set(import_assertions_array_index + 1,
*(pair.second));
import_assertions_array_index += kAssertionEntrySizeForDynamicImport;
} }
return import_assertions_array; return import_assertions_array;
......
...@@ -26167,30 +26167,32 @@ HostImportModuleDynamicallyWithAssertionsCallbackResolve( ...@@ -26167,30 +26167,32 @@ HostImportModuleDynamicallyWithAssertionsCallbackResolve(
String::Utf8Value specifier_utf8(context->GetIsolate(), specifier); String::Utf8Value specifier_utf8(context->GetIsolate(), specifier);
CHECK_EQ(0, strcmp("index.js", *specifier_utf8)); CHECK_EQ(0, strcmp("index.js", *specifier_utf8));
// clang-format off
const char* expected_assertions[][2] = {
{"a", "z"},
{"aa", "x"},
{"b", "w"},
{"c", "y"}
};
// clang-format on
CHECK_EQ(8, import_assertions->Length()); CHECK_EQ(8, import_assertions->Length());
constexpr size_t kAssertionEntrySizeForDynamicImport = 2; constexpr int kAssertionEntrySizeForDynamicImport = 2;
for (int i = 0; i < static_cast<int>(arraysize(expected_assertions)); ++i) { for (int i = 0;
i < import_assertions->Length() / kAssertionEntrySizeForDynamicImport;
++i) {
Local<String> assertion_key = Local<String> assertion_key =
import_assertions import_assertions
->Get(context, (i * kAssertionEntrySizeForDynamicImport)) ->Get(context, (i * kAssertionEntrySizeForDynamicImport))
.As<Value>() .As<Value>()
.As<String>(); .As<String>();
CHECK(v8_str(expected_assertions[i][0])->StrictEquals(assertion_key));
Local<String> assertion_value = Local<String> assertion_value =
import_assertions import_assertions
->Get(context, (i * kAssertionEntrySizeForDynamicImport) + 1) ->Get(context, (i * kAssertionEntrySizeForDynamicImport) + 1)
.As<Value>() .As<Value>()
.As<String>(); .As<String>();
CHECK(v8_str(expected_assertions[i][1])->StrictEquals(assertion_value)); if (v8_str("a")->StrictEquals(assertion_key)) {
CHECK(v8_str("z")->StrictEquals(assertion_value));
} else if (v8_str("aa")->StrictEquals(assertion_key)) {
CHECK(v8_str("x")->StrictEquals(assertion_value));
} else if (v8_str("b")->StrictEquals(assertion_key)) {
CHECK(v8_str("w")->StrictEquals(assertion_value));
} else if (v8_str("c")->StrictEquals(assertion_key)) {
CHECK(v8_str("y")->StrictEquals(assertion_value));
} else {
UNREACHABLE();
}
} }
Local<v8::Promise::Resolver> resolver = Local<v8::Promise::Resolver> resolver =
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment