From ec49f96b9874e4e0006805872601ffdbd792fda7 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Mon, 3 Jul 2017 08:01:04 +0000 Subject: [PATCH] [ES6 modules] Update #internal-module-script-graph-fetching procedure This CL implements the changes introduced in whatwg html spec PR: https://github.com/whatwg/html/pull/2674 Summary: - "final result" is now simply the "module script", instead of previous "descendant module script" which was used to report where error occurred. - "#fetch-the-descendants-of-a-module-script" assumes (actually asserts) that all module specifiers are valid, as now it is guaranteed that their failure is handled at ModuleScript::Create. - ModuleTreeLinker::NotifyOneDescendantFinished now waits for all descendants fetch to complete, even in error cases. -- This matches spec procedure precisely, but we can optimize here later while keeping the behavior. - ModuleScriptLinker now only triggers instantiate on module tree root. (iff ModuleGraphLevel == kTopLevelModuleFetch) -- V8 is now responsible for instantiating the descendant tree + keeping their instantiation status as module script's record's [[Status]] (accessible via ModuleScript::RecordStatus() in Blink) -- UninstantiatedInclusiveDescendants() is removed, as it is no longer used. With this change, Blink now conforms to all module script WPTs! Bug: 594639, 727299, https://github.com/whatwg/html/pull/2674 Change-Id: I7354e00820dd222f30b4a4eba1d3cf8c1b319798 Reviewed-on: https://chromium-review.googlesource.com/540960 Reviewed-by: Hiroki Nakagawa Reviewed-by: Kinuko Yasuda Commit-Queue: Kouhei Ueno Cr-Commit-Position: refs/heads/master@{#483957} --- .../WebKit/LayoutTests/TestExpectations | 8 - .../loader/modulescript/ModuleTreeLinker.cpp | 388 ++++++------------ .../loader/modulescript/ModuleTreeLinker.h | 18 +- .../modulescript/ModuleTreeLinkerTest.cpp | 52 ++- 4 files changed, 167 insertions(+), 299 deletions(-) diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 4d756d2ff704b..e747213f74ebf 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations @@ -1757,14 +1757,6 @@ crbug.com/705125 virtual/mojo-loading/http/tests/security/suborigins/crossorigin # This test fails with the stable release mode. crbug.com/694958 virtual/stable/http/tests/navigation/same-and-different-back.html [ Skip ] -# Failing because of module-related implementation/test issues. -crbug.com/594639 external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling.html [ Failure ] -crbug.com/594639 external/wpt/html/semantics/scripting-1/the-script-element/module/evaluation-error-1.html [ Failure ] -crbug.com/594639 external/wpt/html/semantics/scripting-1/the-script-element/module/evaluation-error-2.html [ Failure ] -crbug.com/594639 external/wpt/html/semantics/scripting-1/the-script-element/module/evaluation-error-3.html [ Failure ] -crbug.com/594639 external/wpt/html/semantics/scripting-1/the-script-element/module/evaluation-error-4.html [ Failure ] -crbug.com/594639 external/wpt/html/semantics/scripting-1/the-script-element/module/slow-cycle.html [ Failure ] - # This test has a failure console message with specific performance # numbers so a consistent baseline cannot be added. This test could be # imported if the test passed or if the results for testharness tests diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp index 1662a7a2570d9..72112475c21b1 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp @@ -27,9 +27,9 @@ ModuleTreeLinker* ModuleTreeLinker::Fetch( AncestorList ancestor_list_with_url = ancestor_list; ancestor_list_with_url.insert(request.Url()); - ModuleTreeLinker* fetcher = - new ModuleTreeLinker(ancestor_list_with_url, modulator, registry, client); - fetcher->FetchSelf(request, level); + ModuleTreeLinker* fetcher = new ModuleTreeLinker( + ancestor_list_with_url, level, modulator, registry, client); + fetcher->FetchSelf(request); return fetcher; } @@ -45,8 +45,9 @@ ModuleTreeLinker* ModuleTreeLinker::FetchDescendantsForInlineScript( DCHECK(module_script); // 4. "Fetch the descendants of script (using an empty ancestor list)." - ModuleTreeLinker* fetcher = - new ModuleTreeLinker(empty_ancestor_list, modulator, registry, client); + ModuleTreeLinker* fetcher = new ModuleTreeLinker( + empty_ancestor_list, ModuleGraphLevel::kTopLevelModuleFetch, modulator, + registry, client); fetcher->module_script_ = module_script; fetcher->AdvanceState(State::kFetchingSelf); @@ -65,6 +66,7 @@ ModuleTreeLinker* ModuleTreeLinker::FetchDescendantsForInlineScript( } ModuleTreeLinker::ModuleTreeLinker(const AncestorList& ancestor_list_with_url, + ModuleGraphLevel level, Modulator* modulator, ModuleTreeLinkerRegistry* registry, ModuleTreeClient* client) @@ -72,8 +74,8 @@ ModuleTreeLinker::ModuleTreeLinker(const AncestorList& ancestor_list_with_url, registry_(registry), client_(client), ancestor_list_with_url_(ancestor_list_with_url), - module_script_(this, nullptr), - descendants_module_script_(this, nullptr) { + level_(level), + module_script_(this, nullptr) { CHECK(modulator); CHECK(registry); CHECK(client); @@ -84,14 +86,12 @@ DEFINE_TRACE(ModuleTreeLinker) { visitor->Trace(registry_); visitor->Trace(client_); visitor->Trace(module_script_); - visitor->Trace(descendants_module_script_); visitor->Trace(dependency_clients_); SingleModuleClient::Trace(visitor); } DEFINE_TRACE_WRAPPERS(ModuleTreeLinker) { visitor->TraceWrappers(module_script_); - visitor->TraceWrappers(descendants_module_script_); } #if DCHECK_IS_ON() @@ -145,22 +145,25 @@ void ModuleTreeLinker::AdvanceState(State new_state) { state_ = new_state; if (state_ == State::kFinished) { - RESOURCE_LOADING_DVLOG(1) - << "ModuleTreeLinker[" << this << "] finished with final result " - << descendants_module_script_; + if (module_script_) { + RESOURCE_LOADING_DVLOG(1) + << "ModuleTreeLinker[" << this << "] finished with final result " + << *module_script_; + } else { + RESOURCE_LOADING_DVLOG(1) + << "ModuleTreeLinker[" << this << "] finished with nullptr."; + } registry_->ReleaseFinishedFetcher(this); // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure - // Step 7. When the "fetch the descendants of and instantiate a module - // script" algorithm asynchronously completes with final result, - // asynchronously complete this algorithm with final result. - client_->NotifyModuleTreeLoadFinished(descendants_module_script_); + // Step 6. When the appropriate algorithm asynchronously completes with + // final result, asynchronously complete this algorithm with final result. + client_->NotifyModuleTreeLoadFinished(module_script_); } } -void ModuleTreeLinker::FetchSelf(const ModuleScriptFetchRequest& request, - ModuleGraphLevel level) { +void ModuleTreeLinker::FetchSelf(const ModuleScriptFetchRequest& request) { // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure // Step 1. Fetch a single module script given url, fetch client settings @@ -169,7 +172,7 @@ void ModuleTreeLinker::FetchSelf(const ModuleScriptFetchRequest& request, // If the caller of this algorithm specified custom perform the fetch steps, // pass those along while fetching a single module script. AdvanceState(State::kFetchingSelf); - modulator_->FetchSingle(request, level, this); + modulator_->FetchSingle(request, level_, this); // Step 2. Return from this algorithm, and run the following steps when // fetching a single module script asynchronously completes with result. @@ -180,23 +183,26 @@ void ModuleTreeLinker::FetchSelf(const ModuleScriptFetchRequest& request, void ModuleTreeLinker::NotifyModuleLoadFinished(ModuleScript* result) { // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure - // Step 3. "If result is null, ..." - // Step 4. "If result's state is "instantiated" or "errored", ..." - // TODO(kouhei): Update the spec references. - if (!result || result->State() == ModuleInstantiationState::kInstantiated || - result->IsErrored()) { + // Step 3. "If result is null, is errored, or has instantiated, asynchronously + // complete this algorithm with result, and abort these steps. + if (!result || result->IsErrored() || result->HasInstantiated()) { // "asynchronously complete this algorithm with result, and abort these // steps." - descendants_module_script_ = result; + module_script_ = result; AdvanceState(State::kFinished); return; } - // Step 5. Assert: result's state is "uninstantiated". - DCHECK_EQ(ModuleInstantiationState::kUninstantiated, result->State()); + // Step 4. Assert: result's state is "uninstantiated". + DCHECK_EQ(ScriptModuleState::kUninstantiated, result->RecordStatus()); - // Step 6. Fetch the descendants of and instantiate result given destination - // and an ancestor list obtained by appending url to ancestor list. + // Step 5. If the top-level module fetch flag is set, fetch the descendants of + // and instantiate result given destination and an ancestor list obtained by + // appending url to ancestor list. Otherwise, fetch the descendants of result + // given the same arguments. + // Note: top-level module fetch flag is checked at Instantiate(), where + // "fetch the descendants of and instantiate" procedure and + // "fetch the descendants" procedure actually diverge. module_script_ = result; FetchDescendants(); } @@ -210,9 +216,12 @@ class ModuleTreeLinker::DependencyModuleClient : public ModuleTreeClient { DEFINE_INLINE_TRACE() { visitor->Trace(module_tree_linker_); + visitor->Trace(result_); ModuleTreeClient::Trace(visitor); } + ModuleScript* Result() { return result_.Get(); } + private: explicit DependencyModuleClient(ModuleTreeLinker* module_tree_linker) : module_tree_linker_(module_tree_linker) { @@ -223,6 +232,7 @@ class ModuleTreeLinker::DependencyModuleClient : public ModuleTreeClient { void NotifyModuleTreeLoadFinished(ModuleScript*) override; Member module_tree_linker_; + Member result_; }; void ModuleTreeLinker::FetchDescendants() { @@ -231,86 +241,60 @@ void ModuleTreeLinker::FetchDescendants() { // [nospec] Abort the steps if the browsing context is discarded. if (!modulator_->HasValidContext()) { - descendants_module_script_ = nullptr; + module_script_ = nullptr; AdvanceState(State::kFinished); return; } - // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-and-instantiate-a-module-script + // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-a-module-script // Step 1. If ancestor list was not given, let it be the empty list. // Note: The "ancestor list" in spec corresponds to |ancestor_list_with_url_|. - // Step 2. If module script's state is "instantiated" or "errored", + // Step 2. If module script is errored or has instantiated, // asynchronously complete this algorithm with module script, and abort these // steps. - // TODO(kouhei): Update spec references - if (module_script_->State() == ModuleInstantiationState::kInstantiated || - module_script_->IsErrored()) { - descendants_module_script_ = module_script_; + if (module_script_->IsErrored() || module_script_->HasInstantiated()) { AdvanceState(State::kFinished); return; } - // Step 3. Assert: module script's state is "uninstantiated" - DCHECK_EQ(ModuleInstantiationState::kUninstantiated, module_script_->State()); - - // Step 4. Let record be module script's module record. + // Step 3. Let record be module script's module record. ScriptModule record = module_script_->Record(); DCHECK(!record.IsNull()); - // Step 5. If record.[[RequestedModules]] is empty, asynchronously complete + // Step 4. If record.[[RequestedModules]] is empty, asynchronously complete // this algorithm with module script. - // Note: We defer this bail-out until Step 9. Step 6-9 will be no-op anyway if - // record.[[RequestedModules]] is empty. + // Note: We defer this bail-out until the end of the procedure. The rest of + // the procedure will be no-op anyway if record.[[RequestedModules]] + // is empty. - // Step 6. Let urls be a new empty list. + // Step 5. Let urls be a new empty list. Vector urls; Vector positions; - // Step 7. For each string requested of record.[[RequestedModules]], + // Step 6. For each string requested of record.[[RequestedModules]], Vector module_requests = modulator_->ModuleRequestsFromScriptModule(record); for (const auto& module_request : module_requests) { - // Step 7.1. Let url be the result of resolving a module specifier given + // Step 6.1. Let url be the result of resolving a module specifier given // module script and requested. KURL url = Modulator::ResolveModuleSpecifier(module_request.specifier, module_script_->BaseURL()); - // Step 7.2. If url is failure: ... - if (url.IsNull()) { - // Step 7.2.1. Let error be a new TypeError exception. - ScriptState::Scope scope(modulator_->GetScriptState()); - v8::Isolate* isolate = modulator_->GetScriptState()->GetIsolate(); - v8::Local error = V8ThrowException::CreateTypeError( - isolate, "Failed to resolve module specifier \'" + - module_request.specifier + "'"); - - // Step 7.2.2. Error module script with error. - module_script_->SetErrorAndClearRecord( - ScriptValue(modulator_->GetScriptState(), error)); - - // Step 7.2.3. Abort this algorithm, and asynchronously complete it with - // module_sript_. Note: The return variable for "internal module script - // graph fetching procedure" is descendants_module_script_ per Step 7. - descendants_module_script_ = module_script_; - AdvanceState(State::kFinished); - return; - } - - // Step 7.3. Otherwise, if ancestor list does not contain url, append url to - // urls. + // Step 6.2. Assert: url is never failure, because resolving a module + // specifier must have been previously successful with these same two + // arguments. CHECK(url.IsValid()) << "Modulator::resolveModuleSpecifier() impl must " "return either a valid url or null."; + + // Step 6.3. if ancestor list does not contain url, append url to urls. if (!ancestor_list_with_url_.Contains(url)) { urls.push_back(url); positions.push_back(module_request.position); } } - // Step 8. Let descendants result be null. - DCHECK(!descendants_module_script_); - - // Step 9. For each url in urls, perform the internal module script graph + // Step 7. For each url in urls, perform the internal module script graph // fetching procedure given url, module script's credentials mode, module // script's cryptographic nonce, module script's parser state, destination, // module script's settings object, module script's settings object, ancestor @@ -320,19 +304,16 @@ void ModuleTreeLinker::FetchDescendants() { // fetching procedure. if (urls.IsEmpty()) { - // Step 5. If record.[[RequestedModules]] is empty, asynchronously + // Step 4. If record.[[RequestedModules]] is empty, asynchronously // complete this algorithm with module script. [spec text] - // Note: We actually proceed to Step 10 here. [nospec, but a pending PR - // fixes this.] // Also, if record.[[RequestedModules]] is not empty but |urls| is - // empty here, we can proceed to Step 10. - descendants_module_script_ = module_script_; + // empty here, we complete this algorithm. Instantiate(); return; } - // Step 9, when "urls" is non-empty. + // Step 7, when "urls" is non-empty. // These invocations of the internal module script graph fetching procedure // should be performed in parallel to each other. [spec text] CHECK_EQ(num_incomplete_descendants_, 0u); @@ -358,11 +339,22 @@ void ModuleTreeLinker::FetchDescendants() { void ModuleTreeLinker::DependencyModuleClient::NotifyModuleTreeLoadFinished( ModuleScript* module_script) { - module_tree_linker_->NotifyOneDescendantFinished(module_script); + result_ = module_script; + if (module_script) { + RESOURCE_LOADING_DVLOG(1) + << "ModuleTreeLinker[" << module_tree_linker_.Get() + << "]::DependencyModuleClient::NotifyModuleTreeLoadFinished() with " + << *module_script; + } else { + RESOURCE_LOADING_DVLOG(1) + << "ModuleTreeLinker[" << module_tree_linker_.Get() + << "]::DependencyModuleClient::NotifyModuleTreeLoadFinished() with " + << "nullptr."; + } + module_tree_linker_->NotifyOneDescendantFinished(); } -void ModuleTreeLinker::NotifyOneDescendantFinished( - ModuleScript* module_script) { +void ModuleTreeLinker::NotifyOneDescendantFinished() { if (state_ != State::kFetchingDependencies) { // We may reach here if one of the descendant failed to load, and the other // descendants fetches were in flight. @@ -374,207 +366,95 @@ void ModuleTreeLinker::NotifyOneDescendantFinished( CHECK_GT(num_incomplete_descendants_, 0u); --num_incomplete_descendants_; - // Step 9 of - // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-and-instantiate-a-module-script - // "If any invocation of the internal module script graph fetching procedure - // asynchronously completes with null, then ..." [spec text] - if (!module_script) { - RESOURCE_LOADING_DVLOG(1) - << "ModuleTreeLinker[" << this - << "]::NotifyOneDescendantFinished with null. " - << num_incomplete_descendants_ << " remaining descendants."; - - // "optionally abort all other invocations, " [spec text] - // TODO(kouhei) Implement this. - - // "set descendants result to null, and ... " [spec text] - DCHECK(!descendants_module_script_); + RESOURCE_LOADING_DVLOG(1) + << "ModuleTreeLinker[" << this << "]::NotifyOneDescendantFinished. " + << num_incomplete_descendants_ << " remaining descendants."; - // "proceed to the next step. (The un-fetched descendant will cause errors - // during instantiation.)" [spec text] - Instantiate(); + // Step 7 of + // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-a-module-script + // "Wait for all invocations of the internal module script graph fetching + // procedure to asynchronously complete, and ..." [spec text] + if (num_incomplete_descendants_) return; - } - RESOURCE_LOADING_DVLOG(1) - << "ModuleTreeLinker[" << this - << "]::NotifyOneDescendantFinished with a module script of state \"" - << ModuleInstantiationStateToString(module_script->State()) << "\". " - << num_incomplete_descendants_ << " remaining descendants."; + // "let results be a list of the results, corresponding to the same order they + // appeared in urls. Then, for each result of results:" [spec text] + for (const auto& client : dependency_clients_) { + ModuleScript* result = client->Result(); - // "If any invocation of the internal module script graph fetching procedure - // asynchronously completes with a module script whose state is "errored", - // then ..." [spec text] - // TODO(kouhei): Update spec references. - if (module_script->IsErrored()) { - // "optionally abort all other invocations, ..." [spec text] - // TODO(kouhei) Implement this. + // Step 7.1: "If result is null, ..." [spec text] + if (!result) { + // "asynchronously complete this algorithm with null, aborting these + // steps." [spec text] + module_script_ = nullptr; + AdvanceState(State::kFinished); + return; + } - // "set descendants result to module script, ..." [spec text] - descendants_module_script_ = module_script_; + // Step 7.2: "If result is errored, ..." [spec text] + if (result->IsErrored()) { + // "then set the pre-instantiation error for module script to result's + // error ..." [spec text] + ScriptValue error = modulator_->GetError(result); + module_script_->SetErrorAndClearRecord(error); - // "and proceed to the next step. (The errored descendant will cause errors - // during instantiation.)" [spec text] - Instantiate(); - return; + // "Asynchronously complete this algorithm with module script, aborting + // these steps." [spec text] + AdvanceState(State::kFinished); + return; + } } - // "Otherwise, wait for all of the internal module script graph fetching - // procedure invocations to asynchronously complete, with module scripts whose - // states are not "errored"." [spec text] - if (!num_incomplete_descendants_) { - // "Then, set descendants result to module script, and proceed to the next - // step." [spec text] - descendants_module_script_ = module_script_; - Instantiate(); - } + Instantiate(); } void ModuleTreeLinker::Instantiate() { CHECK(module_script_); AdvanceState(State::kInstantiating); - // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-and-instantiate-a-module-script + // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure + // Step 5. "If the top-level module fetch flag is set, fetch the descendants + // of and instantiate result given destination and an ancestor list obtained + // by appending url to ancestor list. Otherwise, fetch the descendants of + // result given the same arguments." [spec text] + if (level_ != ModuleGraphLevel::kTopLevelModuleFetch) { + // We don't proceed to instantiate steps if this is descendants module graph + // fetch. + DCHECK_EQ(level_, ModuleGraphLevel::kDependentModuleFetch); + AdvanceState(State::kFinished); + return; + } + // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-and-instantiate-a-module-script // [nospec] Abort the steps if the browsing context is discarded. if (!modulator_->HasValidContext()) { - descendants_module_script_ = nullptr; + module_script_ = nullptr; AdvanceState(State::kFinished); return; } - // Contrary to the spec, we don't store the "record" in Step 4 for its use in - // Step 10. If Instantiate() was called on descendant ModuleTreeLinker and - // failed, module_script_->Record() may be already cleared. - if (module_script_->IsErrored()) { + // Step 1-2 are "fetching the descendants of a module script", which we just + // executed. + + // Step 3. "If result is null or is errored, ..." [spec text] + if (!module_script_ || module_script_->IsErrored()) { + // "then asynchronously complete this algorithm with result." [spec text] + module_script_ = nullptr; AdvanceState(State::kFinished); return; } - // Step 4. Let record be result's module record. + // Step 4. "Let record be result's module record." [spec text] ScriptModule record = module_script_->Record(); - // Step 10. Let instantiationStatus be record.ModuleDeclarationInstantiation. - // Note: The |error| variable corresponds to spec variable - // "instantiationStatus". If |error| is empty, it indicates successful - // completion. - ScriptValue error = modulator_->InstantiateModule(record); - - // Step 11. For each script in module script's uninstantiated inclusive - // descendant module scripts, perform the following steps: - HeapHashSet> uninstantiated_set = - UninstantiatedInclusiveDescendants(); - for (const auto& descendant : uninstantiated_set) { - if (!error.IsEmpty()) { - // Step 11.1. If instantiationStatus is an abrupt completion, then - // error script with instantiationStatus.[[Value]] - descendant->SetErrorAndClearRecord(error); - } else { - // Step 11.2. Otherwise, set script's instantiation state to - // "instantiated". - descendant->SetInstantiationSuccess(); - } - } + // Step 5. "Perform record.ModuleDeclarationInstantiation()." [spec text] - // Step 12. Asynchronously complete this algorithm with descendants result. - AdvanceState(State::kFinished); -} + // "If this throws an exception, ignore it for now; it is stored as result's + // error, and will be reported when we run result." [spec text] + modulator_->InstantiateModule(record); -HeapHashSet> -ModuleTreeLinker::UninstantiatedInclusiveDescendants() { - // https://html.spec.whatwg.org/multipage/webappapis.html#uninstantiated-inclusive-descendant-module-scripts - // Step 1. If script's module record is null, return the empty set. - if (module_script_->HasEmptyRecord()) - return HeapHashSet>(); - - // Step 2. Let moduleMap be script's settings object's module map. - // Note: Modulator is our "settings object". - // Note: We won't reference the ModuleMap directly here to aid testing. - - // Step 3. Let stack be the stack << script >>. - // TODO(kouhei): Make stack a HeapLinkedHashSet for O(1) lookups. - HeapDeque> stack; - stack.push_front(module_script_); - - // Step 4. Let inclusive descendants be an empty set. - // Note: We use unordered set here as the order is not observable from web - // platform. - // This is allowed per spec: https://infra.spec.whatwg.org/#sets - HeapHashSet> inclusive_descendants; - - // Step 5. While stack is not empty: - while (!stack.IsEmpty()) { - // Step 5.1. Let current the result of popping from stack. - ModuleScript* current = stack.TakeFirst(); - - // Step 5.2. Assert: current is a module script (i.e., it is not "fetching" - // or null). - DCHECK(current); - - // Step 5.3. If inclusive descendants and stack both do not contain current, - // then: - if (inclusive_descendants.Contains(current)) - continue; - if (std::find(stack.begin(), stack.end(), current) != stack.end()) - continue; - - // Step 5.3.1. Append current to inclusive descendants. - inclusive_descendants.insert(current); - - // TODO(kouhei): This implementation is a direct transliteration of the - // spec. Omit intermediate vectors at the least. - - // Step 5.3.2. Let child specifiers be the value of current's module - // record's [[RequestedModules]] internal slot. - Vector child_requests = - modulator_->ModuleRequestsFromScriptModule(current->Record()); - // Step 5.3.3. Let child URLs be the list obtained by calling resolve a - // module specifier once for each item of child specifiers, given current - // and that item. Omit any failures. - Vector child_urls; - for (const auto& child_request : child_requests) { - KURL child_url = modulator_->ResolveModuleSpecifier( - child_request.specifier, current->BaseURL()); - if (child_url.IsValid()) - child_urls.push_back(child_url); - } - // Step 5.3.4. Let child modules be the list obtained by getting each value - // in moduleMap whose key is given by an item of child URLs. - HeapVector> child_modules; - for (const auto& child_url : child_urls) { - ModuleScript* module_script = - modulator_->GetFetchedModuleScript(child_url); - - child_modules.push_back(module_script); - } - // Step 5.3.5. For each s of child modules: - for (const auto& s : child_modules) { - // Step 5.3.5.2. If s is null, continue. - // Note: We do null check first, as Blink HashSet can't contain nullptr. - - // Step 5.3.5.3. Assert: s is a module script (i.e., it is not "fetching", - // since by this point all child modules must have been fetched). Note: - // GetFetchedModuleScript returns nullptr if "fetching" - if (!s) - continue; - - // Step 5.3.5.1. If inclusive descendants already contains s, continue. - if (inclusive_descendants.Contains(s)) - continue; - - // Step 5.3.5.4. Push s onto stack. - stack.push_front(s); - } - } - - // Step 6. Return a set containing all items of inclusive descendants whose - // instantiation state is "uninstantiated". - HeapHashSet> uninstantiated_set; - for (const auto& script : inclusive_descendants) { - if (script->State() == ModuleInstantiationState::kUninstantiated) - uninstantiated_set.insert(script); - } - return uninstantiated_set; + // Step 6. Asynchronously complete this algorithm with descendants result. + AdvanceState(State::kFinished); } } // namespace blink diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h index 292628babe0ef..b4a43465abe42 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h @@ -46,6 +46,7 @@ class CORE_EXPORT ModuleTreeLinker final : public SingleModuleClient { private: ModuleTreeLinker(const AncestorList& ancestor_list_with_url, + ModuleGraphLevel, Modulator*, ModuleTreeLinkerRegistry*, ModuleTreeClient*); @@ -65,30 +66,27 @@ class CORE_EXPORT ModuleTreeLinker final : public SingleModuleClient { #endif void AdvanceState(State); - void FetchSelf(const ModuleScriptFetchRequest&, ModuleGraphLevel); + void FetchSelf(const ModuleScriptFetchRequest&); // Implements SingleModuleClient void NotifyModuleLoadFinished(ModuleScript*) override; void FetchDescendants(); - void NotifyOneDescendantFinished(ModuleScript*); + void NotifyOneDescendantFinished(); void Instantiate(); - HeapHashSet> UninstantiatedInclusiveDescendants(); class DependencyModuleClient; friend class DependencyModuleClient; - Member modulator_; - Member registry_; - Member client_; - HashSet ancestor_list_with_url_; + const Member modulator_; + const Member registry_; + const Member client_; + const HashSet ancestor_list_with_url_; + const ModuleGraphLevel level_; State state_ = State::kInitial; // Correspond to _result_ in // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure TraceWrapperMember module_script_; - // Correspond to _descendants result_ in - // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure - TraceWrapperMember descendants_module_script_; size_t num_incomplete_descendants_ = 0; HeapHashSet> dependency_clients_; }; diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp index ddb1453cf160e..d2cc943453b6b 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp @@ -64,7 +64,7 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator { ModuleScript* ResolveSingleModuleScriptFetch( const KURL& url, const Vector& dependency_module_specifiers, - ModuleInstantiationState state) { + ScriptModuleState state) { ScriptState::Scope scope(script_state_.Get()); StringBuilder source_text; @@ -93,7 +93,7 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator { auto result_map = module_map_.insert(url, module_script); EXPECT_TRUE(result_map.is_new_entry); - if (state == ModuleInstantiationState::kErrored) { + if (state == ScriptModuleState::kErrored) { v8::Local error = V8ThrowException::CreateError( script_state_->GetIsolate(), "Instantiation failure."); module_script->SetErrorAndClearRecord( @@ -101,10 +101,8 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator { } EXPECT_EQ(url, pending_request_url_); - if (state == ModuleInstantiationState::kErrored) { + if (state == ScriptModuleState::kErrored) { EXPECT_TRUE(module_script->IsErrored()); - } else { - EXPECT_EQ(state, module_script->State()); } EXPECT_TRUE(pending_client_); pending_client_->NotifyModuleLoadFinished(module_script); @@ -283,11 +281,10 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeNoDeps) { EXPECT_FALSE(client->GetModuleScript()); GetModulator()->ResolveSingleModuleScriptFetch( - url, {}, ModuleInstantiationState::kUninstantiated); + url, {}, ScriptModuleState::kUninstantiated); EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_EQ(client->GetModuleScript()->State(), - ModuleInstantiationState::kInstantiated); + EXPECT_TRUE(client->GetModuleScript()->HasInstantiated()); } TEST_F(ModuleTreeLinkerTest, FetchTreeInstantiationFailure) { @@ -308,14 +305,16 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeInstantiationFailure) { EXPECT_FALSE(client->GetModuleScript()); GetModulator()->ResolveSingleModuleScriptFetch( - url, {}, ModuleInstantiationState::kUninstantiated); + url, {}, ScriptModuleState::kUninstantiated); // Modulator::InstantiateModule() fails here, as // we SetInstantiateShouldFail(true) earlier. EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_TRUE(client->GetModuleScript()->IsErrored()); + EXPECT_TRUE(client->GetModuleScript()->IsErrored()) + << "Expected errored module script but got " + << *client->GetModuleScript(); } TEST_F(ModuleTreeLinkerTest, FetchTreePreviousInstantiationFailure) { @@ -335,8 +334,8 @@ TEST_F(ModuleTreeLinkerTest, FetchTreePreviousInstantiationFailure) { // This emulates "previous instantiation failure", where // Modulator::FetchSingle resolves w/ "errored" module script. - GetModulator()->ResolveSingleModuleScriptFetch( - url, {}, ModuleInstantiationState::kErrored); + GetModulator()->ResolveSingleModuleScriptFetch(url, {}, + ScriptModuleState::kErrored); EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); EXPECT_TRUE(client->GetModuleScript()->IsErrored()); @@ -358,7 +357,7 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWithSingleDependency) { EXPECT_FALSE(client->GetModuleScript()); GetModulator()->ResolveSingleModuleScriptFetch( - url, {"./dep1.js"}, ModuleInstantiationState::kUninstantiated); + url, {"./dep1.js"}, ScriptModuleState::kUninstantiated); EXPECT_FALSE(client->WasNotifyFinished()); KURL url_dep1(kParsedURLString, "http://example.com/dep1.js"); @@ -372,8 +371,7 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWithSingleDependency) { EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_EQ(client->GetModuleScript()->State(), - ModuleInstantiationState::kInstantiated); + EXPECT_TRUE(client->GetModuleScript()->HasInstantiated()); } TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps) { @@ -393,7 +391,7 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps) { GetModulator()->ResolveSingleModuleScriptFetch( url, {"./dep1.js", "./dep2.js", "./dep3.js"}, - ModuleInstantiationState::kUninstantiated); + ScriptModuleState::kUninstantiated); EXPECT_FALSE(client->WasNotifyFinished()); Vector url_deps; @@ -423,8 +421,7 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps) { EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_EQ(client->GetModuleScript()->State(), - ModuleInstantiationState::kInstantiated); + EXPECT_TRUE(client->GetModuleScript()->HasInstantiated()); } TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps1Fail) { @@ -444,7 +441,7 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps1Fail) { GetModulator()->ResolveSingleModuleScriptFetch( url, {"./dep1.js", "./dep2.js", "./dep3.js"}, - ModuleInstantiationState::kUninstantiated); + ScriptModuleState::kUninstantiated); EXPECT_FALSE(client->WasNotifyFinished()); Vector url_deps; @@ -476,8 +473,8 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps1Fail) { GetModulator()->ResolveDependentTreeFetch( url_dep, ModuleTreeLinkerTestModulator::ResolveResult::kFailure); - EXPECT_TRUE(client->WasNotifyFinished()); - EXPECT_FALSE(client->GetModuleScript()); + // TODO(kouhei): This may not hold once we implement early failure reporting. + EXPECT_FALSE(client->WasNotifyFinished()); // Check below doesn't crash. url_dep = url_deps.back(); @@ -485,6 +482,9 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps1Fail) { GetModulator()->ResolveDependentTreeFetch( url_dep, ModuleTreeLinkerTestModulator::ResolveResult::kSuccess); EXPECT_TRUE(url_deps.IsEmpty()); + + EXPECT_TRUE(client->WasNotifyFinished()); + EXPECT_FALSE(client->GetModuleScript()); } TEST_F(ModuleTreeLinkerTest, FetchDependencyTree) { @@ -504,7 +504,7 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyTree) { EXPECT_FALSE(client->GetModuleScript()); GetModulator()->ResolveSingleModuleScriptFetch( - url, {"./depth2.js"}, ModuleInstantiationState::kUninstantiated); + url, {"./depth2.js"}, ScriptModuleState::kUninstantiated); KURL url_dep2(kParsedURLString, "http://example.com/depth2.js"); auto ancestor_list = GetModulator()->GetAncestorListForTreeFetch(url_dep2); @@ -519,8 +519,7 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyTree) { EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_EQ(client->GetModuleScript()->State(), - ModuleInstantiationState::kInstantiated); + EXPECT_FALSE(client->GetModuleScript()->HasInstantiated()); } TEST_F(ModuleTreeLinkerTest, FetchDependencyOfCyclicGraph) { @@ -540,15 +539,14 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyOfCyclicGraph) { EXPECT_FALSE(client->GetModuleScript()); GetModulator()->ResolveSingleModuleScriptFetch( - url, {"./a.js"}, ModuleInstantiationState::kUninstantiated); + url, {"./a.js"}, ScriptModuleState::kUninstantiated); auto ancestor_list = GetModulator()->GetAncestorListForTreeFetch(url); EXPECT_EQ(0u, ancestor_list.size()); EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_EQ(client->GetModuleScript()->State(), - ModuleInstantiationState::kInstantiated); + EXPECT_FALSE(client->GetModuleScript()->HasInstantiated()); } } // namespace blink