From e81cbfd9f8b19631241e926ebfe66aabedc0ef53 Mon Sep 17 00:00:00 2001 From: Kouhei Ueno Date: Wed, 20 Sep 2017 05:22:13 +0000 Subject: [PATCH] Revert "Flatten ModuleTreeLinker" This reverts commit 5a168dfca798f0088b3257f21781a66b4132ba7b. Reason for revert: find-it says this bloke the linux build Original change's description: > Flatten ModuleTreeLinker > > 1. This CL applies the spec change by > https://github.com/whatwg/html/pull/2971. > Since [1], the code has already been behaving like the spec PR 2971, and this CL > makes the code align with PR 2971 more completely, by removing AncestorList > and adding spec comments. > > 2. This CL also flattens the code structure of ModuleTreeLinker, which is > enabled by the spec PR 2971, to make the code simpler to make further > optimizations easy. > That is, instead of creating a ModuleTreeLinker for each module script > (i.e. for each "fetch the descendants" call) in a module graph, this CL > creates a single ModuleTreeLinker that corresponds to a top-level module graph > script. > > Most of fetching-related classes/methods, including > - ModuleTreeLinker::DependencyModuleClient > - ModuleTreeLinker::FetchDescendants() > - ModuleTreeLinker::NotifyOneDescendantFinished() > are merged into the single method NotifyModuleLoadFinished() that implements > the main body of "fetch the descendants" and > "internal module script graph fetching procedure". > > This also removes ModuleTreeReachedUrlSet and instead uses HashSet > directly, as we no longer have to share it across multiple ModuleTreeLinkers. > Modulator::FetchTreeInternal() is removed as we no longer create > ModuleTreeLinkers for subtree fetching via Modulator. > > 3. This CL applies a part of > https://github.com/whatwg/html/pull/2991, particularly introduces > FindFirstParseError() that corresponds to "find the first parse error" and > use it to find the error to be reported, instead of propagating errors > based on Step 6.1 and 6.2 of "fetch the descendants". > > (*) In some subtle cases, this might cause behavior changes in error reporting, > but these changes shouldn't be significant, because anyway the spec before > PR 2991 (and thus the previous implementation) behaves nondeterministically > in some similarly subtle cases. > > This CL is an intermediate step to apply spec PRs 2971 and 2991. > This CL refactors largely ModuleTreeLinker with keeping the existing behavior > mostly (except for (*)), and subsequent CLs will apply the behavior changes > and remaining code structure changes introduced by PR 2991. > > Bug: 763597 > Change-Id: I0ef38c5ebf462fa7f02093f1725ea0014b80585d > Reviewed-on: https://chromium-review.googlesource.com/583552 > Commit-Queue: Hiroshige Hayashizaki > Reviewed-by: Kouhei Ueno > Cr-Commit-Position: refs/heads/master@{#503034} TBR=hiroshige@chromium.org,ksakamoto@chromium.org,kouhei@chromium.org,nhiroki@chromium.org Change-Id: I297bca8d91c91f49e4ef1e46a01a1af90a7e67ef No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 763597 Reviewed-on: https://chromium-review.googlesource.com/674784 Reviewed-by: Kouhei Ueno Commit-Queue: Kouhei Ueno Cr-Commit-Position: refs/heads/master@{#503059} --- .../WebKit/Source/core/dom/AncestorList.h | 22 + third_party/WebKit/Source/core/dom/BUILD.gn | 1 + .../WebKit/Source/core/dom/Modulator.h | 9 + .../Source/core/dom/ModulatorImplBase.cpp | 22 +- .../Source/core/dom/ModulatorImplBase.h | 6 + .../WebKit/Source/core/loader/BUILD.gn | 1 + .../loader/modulescript/ModuleTreeLinker.cpp | 599 +++++++++--------- .../loader/modulescript/ModuleTreeLinker.h | 69 +- .../modulescript/ModuleTreeLinkerRegistry.cpp | 7 +- .../modulescript/ModuleTreeLinkerRegistry.h | 6 + .../modulescript/ModuleTreeLinkerTest.cpp | 141 ++++- .../modulescript/ModuleTreeReachedUrlSet.h | 59 ++ .../Source/core/testing/DummyModulator.cpp | 8 + .../Source/core/testing/DummyModulator.h | 5 + 14 files changed, 571 insertions(+), 384 deletions(-) create mode 100644 third_party/WebKit/Source/core/dom/AncestorList.h create mode 100644 third_party/WebKit/Source/core/loader/modulescript/ModuleTreeReachedUrlSet.h diff --git a/third_party/WebKit/Source/core/dom/AncestorList.h b/third_party/WebKit/Source/core/dom/AncestorList.h new file mode 100644 index 0000000000000..1eb020e903640 --- /dev/null +++ b/third_party/WebKit/Source/core/dom/AncestorList.h @@ -0,0 +1,22 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef AncestorList_h +#define AncestorList_h + +#include "platform/weborigin/KURL.h" +#include "platform/weborigin/KURLHash.h" +#include "platform/wtf/HashSet.h" + +namespace blink { + +// Maps to "ancestor list" concept referenced in multiple module script +// algorithms. +// Example: +// https://html.spec.whatwg.org/#internal-module-script-graph-fetching-procedure +using AncestorList = HashSet; + +} // namespace blink + +#endif diff --git a/third_party/WebKit/Source/core/dom/BUILD.gn b/third_party/WebKit/Source/core/dom/BUILD.gn index 2b4232f69ebb7..b8af9ab2848ef 100644 --- a/third_party/WebKit/Source/core/dom/BUILD.gn +++ b/third_party/WebKit/Source/core/dom/BUILD.gn @@ -16,6 +16,7 @@ blink_core_sources("dom") { "AccessibleNode.h", "AccessibleNodeList.cpp", "AccessibleNodeList.h", + "AncestorList.h", "AnimationWorkletProxyClient.cpp", "AnimationWorkletProxyClient.h", "Attr.cpp", diff --git a/third_party/WebKit/Source/core/dom/Modulator.h b/third_party/WebKit/Source/core/dom/Modulator.h index 2c6b28fce823f..a47e5e5555464 100644 --- a/third_party/WebKit/Source/core/dom/Modulator.h +++ b/third_party/WebKit/Source/core/dom/Modulator.h @@ -7,6 +7,7 @@ #include "bindings/core/v8/ScriptModule.h" #include "core/CoreExport.h" +#include "core/dom/AncestorList.h" #include "platform/bindings/ScriptWrappable.h" #include "platform/bindings/V8PerContextData.h" #include "platform/heap/Handle.h" @@ -23,6 +24,7 @@ class ModuleScript; class ModuleScriptFetcher; class ModuleScriptFetchRequest; class ModuleScriptLoaderClient; +class ModuleTreeReachedUrlSet; class ScriptModuleResolver; class ScriptState; class ScriptValue; @@ -87,6 +89,13 @@ class CORE_EXPORT Modulator : public GarbageCollectedFinalized, virtual void FetchTree(const ModuleScriptFetchRequest&, ModuleTreeClient*) = 0; + // https://html.spec.whatwg.org/#internal-module-script-graph-fetching-procedure + virtual void FetchTreeInternal(const ModuleScriptFetchRequest&, + const AncestorList&, + ModuleGraphLevel, + ModuleTreeReachedUrlSet*, + ModuleTreeClient*) = 0; + // Asynchronously retrieve a module script from the module map, or fetch it // and put it in the map if it's not there already. // https://html.spec.whatwg.org/#fetch-a-single-module-script diff --git a/third_party/WebKit/Source/core/dom/ModulatorImplBase.cpp b/third_party/WebKit/Source/core/dom/ModulatorImplBase.cpp index 27c0561d86054..b7eca15fa66ae 100644 --- a/third_party/WebKit/Source/core/dom/ModulatorImplBase.cpp +++ b/third_party/WebKit/Source/core/dom/ModulatorImplBase.cpp @@ -56,11 +56,9 @@ void ModulatorImplBase::FetchTree(const ModuleScriptFetchRequest& request, // its argument. DCHECK(request.GetReferrer().IsNull()); - // We ensure module-related code is not executed without the flag. - // https://crbug.com/715376 - CHECK(RuntimeEnabledFeatures::ModuleScriptsEnabled()); - - tree_linker_registry_->Fetch(request, this, client); + AncestorList empty_ancestor_list; + FetchTreeInternal(request, empty_ancestor_list, + ModuleGraphLevel::kTopLevelModuleFetch, nullptr, client); // Step 2. When the internal module script graph fetching procedure // asynchronously completes with result, asynchronously complete this @@ -68,6 +66,20 @@ void ModulatorImplBase::FetchTree(const ModuleScriptFetchRequest& request, // Note: We delegate to ModuleTreeLinker to notify ModuleTreeClient. } +void ModulatorImplBase::FetchTreeInternal( + const ModuleScriptFetchRequest& request, + const AncestorList& ancestor_list, + ModuleGraphLevel level, + ModuleTreeReachedUrlSet* reached_url_set, + ModuleTreeClient* client) { + // We ensure module-related code is not executed without the flag. + // https://crbug.com/715376 + CHECK(RuntimeEnabledFeatures::ModuleScriptsEnabled()); + + tree_linker_registry_->Fetch(request, ancestor_list, level, this, + reached_url_set, client); +} + void ModulatorImplBase::FetchDescendantsForInlineScript( ModuleScript* module_script, ModuleTreeClient* client) { diff --git a/third_party/WebKit/Source/core/dom/ModulatorImplBase.h b/third_party/WebKit/Source/core/dom/ModulatorImplBase.h index 56d7e4cb726dc..6c5d2bf870230 100644 --- a/third_party/WebKit/Source/core/dom/ModulatorImplBase.h +++ b/third_party/WebKit/Source/core/dom/ModulatorImplBase.h @@ -18,6 +18,7 @@ class ExecutionContext; class ModuleMap; class ModuleScriptLoaderRegistry; class ModuleTreeLinkerRegistry; +class ModuleTreeReachedUrlSet; class ScriptState; class WebTaskRunner; @@ -50,6 +51,11 @@ class ModulatorImplBase : public Modulator { void FetchTree(const ModuleScriptFetchRequest&, ModuleTreeClient*) override; void FetchDescendantsForInlineScript(ModuleScript*, ModuleTreeClient*) override; + void FetchTreeInternal(const ModuleScriptFetchRequest&, + const AncestorList&, + ModuleGraphLevel, + ModuleTreeReachedUrlSet*, + ModuleTreeClient*) override; void FetchSingle(const ModuleScriptFetchRequest&, ModuleGraphLevel, SingleModuleClient*) override; diff --git a/third_party/WebKit/Source/core/loader/BUILD.gn b/third_party/WebKit/Source/core/loader/BUILD.gn index 3e2f3c163e2dc..47a2c7fc42d0e 100644 --- a/third_party/WebKit/Source/core/loader/BUILD.gn +++ b/third_party/WebKit/Source/core/loader/BUILD.gn @@ -94,6 +94,7 @@ blink_core_sources("loader") { "modulescript/ModuleTreeLinker.h", "modulescript/ModuleTreeLinkerRegistry.cpp", "modulescript/ModuleTreeLinkerRegistry.h", + "modulescript/ModuleTreeReachedUrlSet.h", "modulescript/WorkletModuleScriptFetcher.cpp", "modulescript/WorkletModuleScriptFetcher.h", "private/FrameClientHintsPreferencesContext.cpp", diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp index 2e4693c3140ea..f054d9ab9b11e 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp @@ -5,9 +5,11 @@ #include "core/loader/modulescript/ModuleTreeLinker.h" #include "bindings/core/v8/ScriptModule.h" +#include "core/dom/AncestorList.h" #include "core/dom/ModuleScript.h" #include "core/loader/modulescript/ModuleScriptFetchRequest.h" #include "core/loader/modulescript/ModuleTreeLinkerRegistry.h" +#include "core/loader/modulescript/ModuleTreeReachedUrlSet.h" #include "platform/WebTaskRunner.h" #include "platform/bindings/V8ThrowException.h" #include "platform/loader/fetch/ResourceLoadingLog.h" @@ -16,30 +18,21 @@ namespace blink { -// Note: The current implementation is based on a mixture of the HTML specs -// of a little different versions, in order to incrementally update the code -// structure and the behavior. -// -// The followings are based on the spec BEFORE -// https://github.com/whatwg/html/pull/2991: -// - The cited spec statements of [IMSGF] and [FD]. -// - The behavior (of the whole module script implementation in Blink). -// -// The followings are based on the spec AFTER that spec PR: -// - The cited spec statements of [FDaI] and [FFPE]. -// - The code structure of Instantiate() and FindFirstParseError(). -// These methods are written based on the structure of the latest spec but -// emunates the old behavior. -// -// TODO(hiroshige): The things based on the old spec should be updated shortly. - ModuleTreeLinker* ModuleTreeLinker::Fetch( const ModuleScriptFetchRequest& request, + const AncestorList& ancestor_list, + ModuleGraphLevel level, Modulator* modulator, + ModuleTreeReachedUrlSet* reached_url_set, ModuleTreeLinkerRegistry* registry, ModuleTreeClient* client) { - ModuleTreeLinker* fetcher = new ModuleTreeLinker(modulator, registry, client); - fetcher->FetchRoot(request); + AncestorList ancestor_list_with_url = ancestor_list; + ancestor_list_with_url.insert(request.Url()); + + ModuleTreeLinker* fetcher = + new ModuleTreeLinker(ancestor_list_with_url, level, modulator, + reached_url_set, registry, client); + fetcher->FetchSelf(request); return fetcher; } @@ -48,31 +41,67 @@ ModuleTreeLinker* ModuleTreeLinker::FetchDescendantsForInlineScript( Modulator* modulator, ModuleTreeLinkerRegistry* registry, ModuleTreeClient* client) { + AncestorList empty_ancestor_list; + + // Substep 4 in "module" case in Step 22 of "prepare a script":" + // https://html.spec.whatwg.org/#prepare-a-script DCHECK(module_script); - ModuleTreeLinker* fetcher = new ModuleTreeLinker(modulator, registry, client); - fetcher->FetchRootInline(module_script); + + // 4. "Fetch the descendants of script (using an empty ancestor list)." + ModuleTreeLinker* fetcher = new ModuleTreeLinker( + empty_ancestor_list, ModuleGraphLevel::kTopLevelModuleFetch, modulator, + nullptr, registry, client); + fetcher->module_script_ = module_script; + fetcher->AdvanceState(State::kFetchingSelf); + + // "When this asynchronously completes, set the script's script to + // the result. At that time, the script is ready." + // + // Currently we execute "internal module script graph + // fetching procedure" Step 5- in addition to "fetch the descendants", + // which is not specced yet. https://github.com/whatwg/html/issues/2544 + // TODO(hiroshige): Fix the implementation and/or comments once the spec + // is updated. + modulator->TaskRunner()->PostTask( + BLINK_FROM_HERE, + WTF::Bind(&ModuleTreeLinker::FetchDescendants, WrapPersistent(fetcher))); return fetcher; } -ModuleTreeLinker::ModuleTreeLinker(Modulator* modulator, +ModuleTreeLinker::ModuleTreeLinker(const AncestorList& ancestor_list_with_url, + ModuleGraphLevel level, + Modulator* modulator, + ModuleTreeReachedUrlSet* reached_url_set, ModuleTreeLinkerRegistry* registry, ModuleTreeClient* client) - : modulator_(modulator), registry_(registry), client_(client) { + : modulator_(modulator), + reached_url_set_( + level == ModuleGraphLevel::kTopLevelModuleFetch + ? ModuleTreeReachedUrlSet::CreateFromTopLevelAncestorList( + ancestor_list_with_url) + : reached_url_set), + registry_(registry), + client_(client), + ancestor_list_with_url_(ancestor_list_with_url), + level_(level) { CHECK(modulator); + CHECK(reached_url_set_); CHECK(registry); CHECK(client); } DEFINE_TRACE(ModuleTreeLinker) { visitor->Trace(modulator_); + visitor->Trace(reached_url_set_); visitor->Trace(registry_); visitor->Trace(client_); - visitor->Trace(result_); + visitor->Trace(module_script_); + visitor->Trace(dependency_clients_); SingleModuleClient::Trace(visitor); } DEFINE_TRACE_WRAPPERS(ModuleTreeLinker) { - visitor->TraceWrappers(result_); + visitor->TraceWrappers(module_script_); } #if DCHECK_IS_ON() @@ -103,11 +132,11 @@ void ModuleTreeLinker::AdvanceState(State new_state) { switch (state_) { case State::kInitial: - CHECK_EQ(num_incomplete_fetches_, 0u); + CHECK_EQ(num_incomplete_descendants_, 0u); CHECK_EQ(new_state, State::kFetchingSelf); break; case State::kFetchingSelf: - CHECK_EQ(num_incomplete_fetches_, 0u); + CHECK_EQ(num_incomplete_descendants_, 0u); CHECK(new_state == State::kFetchingDependencies || new_state == State::kFinished); break; @@ -126,9 +155,10 @@ void ModuleTreeLinker::AdvanceState(State new_state) { state_ = new_state; if (state_ == State::kFinished) { - if (result_) { - RESOURCE_LOADING_DVLOG(1) << "ModuleTreeLinker[" << this - << "] finished with final result " << *result_; + 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."; @@ -136,378 +166,319 @@ void ModuleTreeLinker::AdvanceState(State new_state) { registry_->ReleaseFinishedFetcher(this); - // [IMSGF] Step 7. When the appropriate algorithm asynchronously completes - // with final result, asynchronously complete this algorithm with final - // result. - client_->NotifyModuleTreeLoadFinished(result_); + // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure + // Step 6. When the appropriate algorithm asynchronously completes with + // final result, asynchronously complete this algorithm with final result. + client_->NotifyModuleTreeLoadFinished(module_script_); } } -void ModuleTreeLinker::FetchRoot(const ModuleScriptFetchRequest& request) { - // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-module-script-tree +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 + // object, destination, cryptographic nonce, parser state, credentials mode, + // module map settings object, referrer, and the top-level module fetch flag. + // 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); - // Step 1 is done in InitiateInternalModuleScriptGraphFetching(). - - // Step 2. Perform the internal module script graph fetching procedure given - // ... with the top-level module fetch flag set. ... - InitiateInternalModuleScriptGraphFetching( - request, ModuleGraphLevel::kTopLevelModuleFetch); + // Step 2. Return from this algorithm, and run the following steps when + // fetching a single module script asynchronously completes with result. + // Note: Modulator::FetchSingle asynchronously notifies result to + // ModuleTreeLinker::NotifyModuleLoadFinished(). } -void ModuleTreeLinker::FetchRootInline(ModuleScript* module_script) { - // Top-level entry point for [FDaI] for an inline module script. - - AdvanceState(State::kFetchingSelf); +void ModuleTreeLinker::NotifyModuleLoadFinished(ModuleScript* result) { + // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure - // Store the |module_script| here which will be used as result of the - // algorithm when success. Also, this ensures that the |module_script| is - // TraceWrappers()ed via ModuleTreeLinker. - result_ = module_script; - AdvanceState(State::kFetchingDependencies); - - modulator_->TaskRunner()->PostTask( - BLINK_FROM_HERE, - WTF::Bind(&ModuleTreeLinker::FetchDescendants, WrapPersistent(this), - WrapPersistent(module_script))); -} - -void ModuleTreeLinker::InitiateInternalModuleScriptGraphFetching( - const ModuleScriptFetchRequest& request, - ModuleGraphLevel level) { - DCHECK(!visited_set_.Contains(request.Url())); - - // This step originates from the callers of [IMSGF]: - // - // https://html.spec.whatwg.org/#fetch-a-module-script-tree - // https://html.spec.whatwg.org/#fetch-a-module-worker-script-tree - // Step 1. Let visited set be << url >>. - // - // [FD] Step 5.3.2. Append url to visited set. - visited_set_.insert(request.Url()); - - // [IMSGF] Step 1. Assert: visited set contains url. - // - // This is ensured by the insert() just above. - - ++num_incomplete_fetches_; - - // [IMSGF] Step 2. Fetch a single module script given ... - modulator_->FetchSingle(request, level, this); + // 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." + module_script_ = result; + AdvanceState(State::kFinished); + return; + } - // [IMSGF] Step 3-- are executed when NotifyModuleLoadFinished() is called. + // Step 4. Assert: result's state is "uninstantiated". + DCHECK_EQ(ScriptModuleState::kUninstantiated, result->RecordStatus()); + + // 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(); } -void ModuleTreeLinker::NotifyModuleLoadFinished(ModuleScript* module_script) { - // [IMSGF] Step 3. Return from this algorithm, and run the following steps - // when fetching a single module script asynchronously completes with result: - - CHECK_GT(num_incomplete_fetches_, 0u); - --num_incomplete_fetches_; - - if (module_script) { - RESOURCE_LOADING_DVLOG(1) - << "ModuleTreeLinker[" << this << "]::NotifyModuleLoadFinished() with " - << *module_script; - } else { - RESOURCE_LOADING_DVLOG(1) - << "ModuleTreeLinker[" << this << "]::NotifyModuleLoadFinished() with " - << "nullptr."; +class ModuleTreeLinker::DependencyModuleClient : public ModuleTreeClient { + public: + static DependencyModuleClient* Create(ModuleTreeLinker* module_tree_linker) { + return new DependencyModuleClient(module_tree_linker); } + virtual ~DependencyModuleClient() = default; - if (state_ == State::kFetchingSelf) { - // Corresponds to top-level calls to - // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-and-instantiate-a-module-script - // i.e. [IMSGF] with the top-level module fetch flag set (external), or - // Step 22 of "prepare a script" (inline). - // |module_script| is the top-level module, and will be instantiated - // and returned later. - result_ = module_script; - AdvanceState(State::kFetchingDependencies); + DEFINE_INLINE_TRACE() { + visitor->Trace(module_tree_linker_); + visitor->Trace(result_); + ModuleTreeClient::Trace(visitor); } - if (state_ != State::kFetchingDependencies) { - // We may reach here if one of the descendant failed to load, and the other - // descendants fetches were in flight. - return; + ModuleScript* Result() { return result_.Get(); } + + private: + explicit DependencyModuleClient(ModuleTreeLinker* module_tree_linker) + : module_tree_linker_(module_tree_linker) { + CHECK(module_tree_linker); } - // Note: top-level module fetch flag is implemented so that Instantiate() - // is called once after all descendants are fetched, which corresponds to - // the single invocation of "fetch the descendants of and instantiate". + // Implements ModuleTreeClient + void NotifyModuleTreeLoadFinished(ModuleScript*) override; - // [IMSGF] Steps 4 and 5 are merged to FetchDescendants(). + Member module_tree_linker_; + Member result_; +}; - // [IMSGF] Step 6. If the top-level module fetch flag is set, fetch the - // descendants of and instantiate result given destination and visited set. - // Otherwise, fetch the descendants of result given the same arguments. - FetchDescendants(module_script); -} +void ModuleTreeLinker::FetchDescendants() { + CHECK(module_script_); + AdvanceState(State::kFetchingDependencies); -void ModuleTreeLinker::FetchDescendants(ModuleScript* module_script) { // [nospec] Abort the steps if the browsing context is discarded. if (!modulator_->HasValidContext()) { - result_ = nullptr; + module_script_ = nullptr; AdvanceState(State::kFinished); return; } - // [FD] Step 1. If module script is errored or has instantiated, + // 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 is errored or has instantiated, // asynchronously complete this algorithm with module script, and abort these // steps. - // - // [IMSGF] Step 4. If result is null, is errored, or has instantiated, - // asynchronously complete this algorithm with result, and abort these steps. - if (!module_script || module_script->IsErrored()) { - found_error_ = true; - // We don't early-exit here and wait until all module scripts to be - // loaded, because we might be not sure which error to be reported. - // - // It is possible to determine whether the error to be reported can be - // determined without waiting for loading module scripts, and thus to - // early-exit here if possible. However, the complexity of such early-exit - // implementation might be high, and optimizing error cases with the - // implementation cost might be not worth doing. - FinalizeFetchDescendantsForOneModuleScript(); - return; - } - if (module_script->HasInstantiated()) { - FinalizeFetchDescendantsForOneModuleScript(); + if (module_script_->IsErrored() || module_script_->HasInstantiated()) { + AdvanceState(State::kFinished); return; } - // [IMSGF] Step 5. Assert: result's record's [[Status]] is "uninstantiated". - DCHECK_EQ(ScriptModuleState::kUninstantiated, module_script->RecordStatus()); - - // [FD] Step 2. Let record be module script's record. - ScriptModule record = module_script->Record(); + // Step 3. Let record be module script's module record. + ScriptModule record = module_script_->Record(); DCHECK(!record.IsNull()); - // [FD] Step 3. If record.[[RequestedModules]] is empty, asynchronously - // complete this algorithm with module script. - // + // Step 4. If record.[[RequestedModules]] is empty, asynchronously complete + // this algorithm with module script. // 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. + // the procedure will be no-op anyway if record.[[RequestedModules]] + // is empty. - // [FD] Step 4. Let urls be a new empty list. + // Step 5. Let urls be a new empty list. Vector urls; Vector positions; - // [FD] Step 5. 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) { - // [FD] Step 5.1. Let url be the result of resolving a module specifier - // given module script and requested. + // 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()); + module_script_->BaseURL()); - // [FD] Step 5.2. Assert: url is never failure, because resolving a module + // 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."; - // [FD] Step 5.3. If visited set does not contain url, then: - if (!visited_set_.Contains(url)) { - // [FD] Step 5.3.1. Append url to urls. - urls.push_back(url); + // Step 6.3. if ancestor list does not contain url, append url to urls. + if (ancestor_list_with_url_.Contains(url)) + continue; - // [FD] Step 5.3.2. Append url to visited set. - // - // This step is deferred to InitiateInternalModuleScriptGraphFetching() - // below. + // [unspec] If we already have started a sub-graph fetch for the |url| for + // this top-level module graph starting from |top_level_linker_|, we can + // safely rely on other module graph node ModuleTreeLinker to handle it. + if (reached_url_set_->IsAlreadyBeingFetched(url)) { + // We can't skip any sub-graph fetches directly made from the top level + // module, as we may end up proceeding to ModuleDeclarationInstantiation() + // with part of the graph still fetching. + CHECK_NE(level_, ModuleGraphLevel::kTopLevelModuleFetch); - positions.push_back(module_request.position); + continue; } + + urls.push_back(url); + positions.push_back(module_request.position); } + // 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 + // list, module script's base URL, and with the top-level module fetch flag + // unset. If the caller of this algorithm specified custom perform the fetch + // steps, pass those along while performing the internal module script graph + // fetching procedure. + if (urls.IsEmpty()) { - // [FD] Step 3. If record.[[RequestedModules]] is empty, asynchronously - // complete this algorithm with module script. - // + // Step 4. If record.[[RequestedModules]] is empty, asynchronously + // complete this algorithm with module script. [spec text] + // Also, if record.[[RequestedModules]] is not empty but |urls| is // empty here, we complete this algorithm. - FinalizeFetchDescendantsForOneModuleScript(); + Instantiate(); return; } - // [FD] Step 6. For each url in urls, ... - // - // [FD] Step 6. These invocations of the internal module script graph fetching - // procedure should be performed in parallel to each other. + // 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); + num_incomplete_descendants_ = urls.size(); for (size_t i = 0; i < urls.size(); ++i) { - // [FD] Step 6. ... perform the internal module script graph fetching - // procedure given ... with the top-level module fetch flag unset. ... + DependencyModuleClient* dependency_client = + DependencyModuleClient::Create(this); + reached_url_set_->ObserveModuleTreeLink(urls[i]); + dependency_clients_.insert(dependency_client); + ModuleScriptFetchRequest request( - urls[i], module_script->Nonce(), module_script->ParserState(), - module_script->CredentialsMode(), module_script->BaseURL().GetString(), - positions[i]); - InitiateInternalModuleScriptGraphFetching( - request, ModuleGraphLevel::kDependentModuleFetch); + urls[i], module_script_->Nonce(), module_script_->ParserState(), + module_script_->CredentialsMode(), + module_script_->BaseURL().GetString(), positions[i]); + modulator_->FetchTreeInternal(request, ancestor_list_with_url_, + ModuleGraphLevel::kDependentModuleFetch, + reached_url_set_.Get(), dependency_client); } - // Asynchronously continue processing after NotifyModuleLoadFinished() is - // called num_incomplete_fetches_ times. - CHECK_GT(num_incomplete_fetches_, 0u); + // Asynchronously continue processing after NotifyOneDescendantFinished() is + // called num_incomplete_descendants_ times. + CHECK_GT(num_incomplete_descendants_, 0u); } -void ModuleTreeLinker::FinalizeFetchDescendantsForOneModuleScript() { - // [FD] of a single module script is completed here: - // - // [FD] Step 6. Wait for all invocations of the internal module script graph - // fetching procedure to asynchronously complete, ... - - // And, if |num_incomplete_fetches_| is 0, all the invocations of [FD] - // (called from [FDaI] Step 2) of the root module script is completed here - // and thus we proceed to [FDaI] Step 4 implemented by Instantiate(). - if (num_incomplete_fetches_ == 0) - Instantiate(); -} - -void ModuleTreeLinker::Instantiate() { - // [nospec] Abort the steps if the browsing context is discarded. - if (!modulator_->HasValidContext()) { - result_ = nullptr; - AdvanceState(State::kFinished); - return; +void ModuleTreeLinker::DependencyModuleClient::NotifyModuleTreeLoadFinished( + ModuleScript* 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(); +} - // [FDaI] Step 4. If result is null, then asynchronously complete this - // algorithm with result. - if (!result_) { - AdvanceState(State::kFinished); +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. return; } + DCHECK_EQ(state_, State::kFetchingDependencies); + DCHECK(module_script_); - // [FDaI] Step 5. Let parse error be the result of finding the first parse - // error given result. - // - // [Optimization] If |found_error_| is false (i.e. no errors were found during - // fetching), we are sure that |parse error| is null and thus skip - // FindFirstParseError() call. - if (found_error_) { - // [FFPE] Step 2. If discoveredSet was not given, let it be an empty set. - HeapHashSet> discovered_set; - DCHECK(FindFirstParseError(result_, &discovered_set)); - } + CHECK_GT(num_incomplete_descendants_, 0u); + --num_incomplete_descendants_; - // [FDaI] Step 6. If parse error is null, then: - // - // [FDaI] Step 7. Otherwise, set result's error to rethrow to parse error. - // - // [old spec] TODO(hiroshige): Update this. - if (!result_ || result_->IsErrored()) { - AdvanceState(State::kFinished); + RESOURCE_LOADING_DVLOG(1) + << "ModuleTreeLinker[" << this << "]::NotifyOneDescendantFinished. " + << num_incomplete_descendants_ << " remaining descendants."; + + // 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; - } - // In the case of parse error is not null: - - DCHECK(result_); - AdvanceState(State::kInstantiating); - - // [FDaI] Step 6.1. Let record be result's record. - ScriptModule record = result_->Record(); + // "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(); + + // 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; + } - // [FDaI] Step 6.2. Perform record.Instantiate(). If this throws an - // exception, set result's error to rethrow to that exception. - modulator_->InstantiateModule(record); + // 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); + + // "Asynchronously complete this algorithm with module script, aborting + // these steps." [spec text] + AdvanceState(State::kFinished); + return; + } + } - // [FDaI] Step 8. Asynchronously complete this algorithm with result. - AdvanceState(State::kFinished); + Instantiate(); } -// [FFPE] https://html.spec.whatwg.org/#finding-the-first-parse-error -// -// TODO(hiroshige): The code structure below aligns the spec after -// https://github.com/whatwg/html/pull/2991, but the behavior aligns the -// spec before that PR (that originates from Step 6.1. of [FD]), and thus -// contains [old spec] statements. -// Update the behavior according to the PR. -// -// This returns true if an error is found, and updates |result_| or -// |result_|'s error accordingly. -// -// TODO(hiroshige): This is also because the behavior is based on the old spec. -// Make FindFirstParseError to return ScriptValue once the behavior is updated. -bool ModuleTreeLinker::FindFirstParseError( - ModuleScript* module_script, - HeapHashSet>* discovered_set) { - // [FFPE] Step 1. Let moduleMap be moduleScript's settings object's module - // map. - // - // This is accessed via |modulator_|. +void ModuleTreeLinker::Instantiate() { + CHECK(module_script_); + AdvanceState(State::kInstantiating); - // [FFPE] Step 2 is done before calling this in Instantiate(). + // 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; + } - // [old spec] [FD] Step 6.1. If result is null, asynchronously complete this - // algorithm with null, aborting these steps. - if (!module_script) { - result_ = nullptr; - return true; + // 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()) { + module_script_ = nullptr; + AdvanceState(State::kFinished); + return; } - // [FFPE] Step 3. Append moduleScript to discoveredSet. - discovered_set->insert(module_script); + // Step 1-2 are "fetching the descendants of a module script", which we just + // executed. - // [FFPE] Step 4. If moduleScript's record is null, then return moduleScript's - // parse error. - // - // [old spec] [FD] Step 6.2. If result is errored, then set the - // pre-instantiation error for module script to result's error. Asynchronously - // complete this algorithm with module script, aborting these steps. - if (module_script->IsErrored()) { - result_->SetErrorAndClearRecord(modulator_->GetError(module_script)); - return true; + // 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; } - // [FFPE] Step 5. Let childSpecifiers be the value of moduleScript's record's - // [[RequestedModules]] internal slot. - ScriptModule record = module_script->Record(); - DCHECK(!record.IsNull()); - Vector child_specifiers = - modulator_->ModuleRequestsFromScriptModule(record); + // Step 4. "Let record be result's module record." [spec text] + ScriptModule record = module_script_->Record(); - for (const auto& module_request : child_specifiers) { - // [FFPE] Step 6. Let childURLs be the list obtained by calling resolve a - // module specifier once for each item of childSpecifiers, given - // moduleScript and that item. ... - KURL child_url = Modulator::ResolveModuleSpecifier( - module_request.specifier, module_script->BaseURL()); - - // [FFPE] Step 6. ... (None of these will ever fail, as otherwise - // moduleScript would have been marked as itself having a parse error.) - CHECK(child_url.IsValid()) - << "Modulator::ResolveModuleSpecifier() impl must " - "return either a valid url or null."; - - // [FFPE] Step 7. Let childModules be the list obtained by getting each - // value in moduleMap whose key is given by an item of childURLs. - // - // [FFPE] Step 8. For each childModule of childModules: - ModuleScript* child_module = modulator_->GetFetchedModuleScript(child_url); - - // [FFPE] Step 8.2. If discoveredSet already contains childModule, continue. - // - // TODO(hiroshige): if |child_module| is null, we skip Contains() call - // because HashSet forbids nullptr. Anyway |child_module| can be nullptr - // because this is based on the [old spec], so remove this hack once we - // update the behavior. - if (child_module && discovered_set->Contains(child_module)) - continue; + // Step 5. "Perform record.ModuleDeclarationInstantiation()." [spec text] - // [FFPE] Step 8.3. Let childParseError be the result of finding the first - // parse error given childModule and discoveredSet. - // - // [FFPE] Step 8.4. If childParseError is not null, return childParseError. - if (FindFirstParseError(child_module, discovered_set)) - return true; - } + // "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); - // [FFPE] Step 9. Return null. - return false; + // 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 489aa726e4896..f6bb1210a82da 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h @@ -6,41 +6,30 @@ #define ModuleTreeLinker_h #include "core/CoreExport.h" +#include "core/dom/AncestorList.h" #include "core/dom/Modulator.h" #include "platform/bindings/ScriptWrappable.h" #include "platform/bindings/TraceWrapperMember.h" -#include "platform/weborigin/KURL.h" -#include "platform/weborigin/KURLHash.h" -#include "platform/wtf/HashSet.h" namespace blink { class ModuleScriptFetchRequest; class ModuleTreeLinkerRegistry; +class ModuleTreeReachedUrlSet; // A ModuleTreeLinker is responsible for running and keeping intermediate states -// for a top-level [IMSGF] "internal module script graph fetching procedure" or -// a top-level [FDaI] "fetch the descendants of and instantiate", and all the -// invocations of [IMSGF] and [FD] "fetch the descendants" under that. -// -// Spec links: -// [IMSGF] -// https://html.spec.whatwg.org/#internal-module-script-graph-fetching-procedure -// [FD] -// https://html.spec.whatwg.org/#fetch-the-descendants-of-a-module-script -// [FDaI] -// https://html.spec.whatwg.org/#fetch-the-descendants-of-and-instantiate-a-module-script -// [FFPE] -// https://html.spec.whatwg.org/#finding-the-first-parse-error +// for "internal module script graph fetching procedure" for a module graph tree +// node. +// https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure class CORE_EXPORT ModuleTreeLinker final : public SingleModuleClient { public: - // https://html.spec.whatwg.org/#fetch-a-module-script-tree static ModuleTreeLinker* Fetch(const ModuleScriptFetchRequest&, + const AncestorList&, + ModuleGraphLevel, Modulator*, + ModuleTreeReachedUrlSet*, ModuleTreeLinkerRegistry*, ModuleTreeClient*); - - // [FDaI] for an inline script. static ModuleTreeLinker* FetchDescendantsForInlineScript( ModuleScript*, Modulator*, @@ -57,7 +46,12 @@ class CORE_EXPORT ModuleTreeLinker final : public SingleModuleClient { bool HasFinished() const { return state_ == State::kFinished; } private: - ModuleTreeLinker(Modulator*, ModuleTreeLinkerRegistry*, ModuleTreeClient*); + ModuleTreeLinker(const AncestorList& ancestor_list_with_url, + ModuleGraphLevel, + Modulator*, + ModuleTreeReachedUrlSet*, + ModuleTreeLinkerRegistry*, + ModuleTreeClient*); enum class State { kInitial, @@ -74,41 +68,30 @@ class CORE_EXPORT ModuleTreeLinker final : public SingleModuleClient { #endif void AdvanceState(State); - void FetchRoot(const ModuleScriptFetchRequest&); - void FetchRootInline(ModuleScript*); - - // Steps 1--2 of [IMSGF]. - void InitiateInternalModuleScriptGraphFetching( - const ModuleScriptFetchRequest&, - ModuleGraphLevel); - - // Steps 3--7 of [IMSGF], and [FD]/[FDaI] called from [IMSGF]. - // TODO(hiroshige): Currently + void FetchSelf(const ModuleScriptFetchRequest&); + // Implements SingleModuleClient void NotifyModuleLoadFinished(ModuleScript*) override; - void FetchDescendants(ModuleScript*); - // Completion of [FD]. - void FinalizeFetchDescendantsForOneModuleScript(); + void FetchDescendants(); + void NotifyOneDescendantFinished(); - // [FDaI] Steps 4--8. void Instantiate(); - // [FFPE] - bool FindFirstParseError(ModuleScript*, HeapHashSet>*); + class DependencyModuleClient; + friend class DependencyModuleClient; const Member modulator_; - HashSet visited_set_; + const Member reached_url_set_; 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 result_; - - bool found_error_ = false; - - size_t num_incomplete_fetches_ = 0; + TraceWrapperMember module_script_; + size_t num_incomplete_descendants_ = 0; + HeapHashSet> dependency_clients_; }; } // namespace blink diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.cpp b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.cpp index 6e7b6db58438b..7bf7bfa3afc45 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.cpp +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.cpp @@ -21,10 +21,13 @@ DEFINE_TRACE_WRAPPERS(ModuleTreeLinkerRegistry) { ModuleTreeLinker* ModuleTreeLinkerRegistry::Fetch( const ModuleScriptFetchRequest& request, + const AncestorList& ancestor_list, + ModuleGraphLevel level, Modulator* modulator, + ModuleTreeReachedUrlSet* reached_url_set, ModuleTreeClient* client) { - ModuleTreeLinker* fetcher = - ModuleTreeLinker::Fetch(request, modulator, this, client); + ModuleTreeLinker* fetcher = ModuleTreeLinker::Fetch( + request, ancestor_list, level, modulator, reached_url_set, this, client); DCHECK(fetcher->IsFetching()); active_tree_linkers_.insert(fetcher); return fetcher; diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.h b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.h index 7bd7a4709534b..f96879f664b58 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.h +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.h @@ -6,6 +6,7 @@ #define ModuleTreeLinkerRegistry_h #include "core/CoreExport.h" +#include "core/dom/AncestorList.h" #include "platform/bindings/ScriptWrappable.h" #include "platform/bindings/TraceWrapperMember.h" #include "platform/heap/Handle.h" @@ -16,7 +17,9 @@ class Modulator; class ModuleScriptFetchRequest; class ModuleTreeClient; class ModuleTreeLinker; +enum class ModuleGraphLevel; class ModuleScript; +class ModuleTreeReachedUrlSet; // ModuleTreeLinkerRegistry keeps active ModuleTreeLinkers alive. class CORE_EXPORT ModuleTreeLinkerRegistry @@ -30,7 +33,10 @@ class CORE_EXPORT ModuleTreeLinkerRegistry DECLARE_TRACE_WRAPPERS(); ModuleTreeLinker* Fetch(const ModuleScriptFetchRequest&, + const AncestorList&, + ModuleGraphLevel, Modulator*, + ModuleTreeReachedUrlSet*, ModuleTreeClient*); ModuleTreeLinker* FetchDescendantsForInlineScript(ModuleScript*, Modulator*, diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp index 27714dad1ae7e..a486a36ee8261 100644 --- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp @@ -11,6 +11,7 @@ #include "core/dom/ModuleScript.h" #include "core/loader/modulescript/ModuleScriptFetchRequest.h" #include "core/loader/modulescript/ModuleTreeLinkerRegistry.h" +#include "core/loader/modulescript/ModuleTreeReachedUrlSet.h" #include "core/testing/DummyModulator.h" #include "core/testing/DummyPageHolder.h" #include "platform/bindings/ScriptState.h" @@ -100,24 +101,54 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator { ScriptValue(script_state_.Get(), error)); } + EXPECT_EQ(url, pending_request_url_); if (state == ScriptModuleState::kErrored) { EXPECT_TRUE(module_script->IsErrored()); } - EXPECT_TRUE(pending_clients_.Contains(url)); - pending_clients_.Take(url)->NotifyModuleLoadFinished(module_script); + EXPECT_TRUE(pending_client_); + pending_client_->NotifyModuleLoadFinished(module_script); + pending_client_.Clear(); return module_script; } + // Get AncestorList specified in |Modulator::FetchTreeInternal()| call for + // request matching |url|. + AncestorList GetAncestorListForTreeFetch(const KURL& url) const { + const auto& it = pending_tree_ancestor_list_.find(url); + if (it == pending_tree_ancestor_list_.end()) + return AncestorList(); + return it->value; + } + + // Resolve |Modulator::FetchTreeInternal()| for given url. void ResolveDependentTreeFetch(const KURL& url, ResolveResult result) { + const auto& it = pending_tree_client_map_.find(url); + EXPECT_NE(pending_tree_client_map_.end(), it); + auto pending_client = it->value; + EXPECT_TRUE(pending_client); + pending_tree_client_map_.erase(it); + if (result == ResolveResult::kFailure) { - EXPECT_TRUE(pending_clients_.Contains(url)); - pending_clients_.Take(url)->NotifyModuleLoadFinished(nullptr); + pending_client->NotifyModuleTreeLoadFinished(nullptr); return; } EXPECT_EQ(ResolveResult::kSuccess, result); - ResolveSingleModuleScriptFetch(url, Vector(), - ScriptModuleState::kUninstantiated); + + ScriptState::Scope scope(script_state_.Get()); + + ScriptModule script_module = ScriptModule::Compile( + script_state_->GetIsolate(), "export default 'pineapples';", + url.GetString(), kSharableCrossOrigin, + WebURLRequest::kFetchCredentialsModeOmit, "", kParserInserted, + TextPosition::MinimumPosition(), ASSERT_NO_EXCEPTION); + ModuleScript* module_script = ModuleScript::CreateForTest( + this, script_module, url, "", kParserInserted, + WebURLRequest::kFetchCredentialsModeOmit); + auto result_map = module_map_.insert(url, module_script); + EXPECT_TRUE(result_map.is_new_entry); + + pending_client->NotifyModuleTreeLoadFinished(module_script); } void SetInstantiateShouldFail(bool b) { instantiate_should_fail_ = b; } @@ -130,8 +161,26 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator { void FetchSingle(const ModuleScriptFetchRequest& request, ModuleGraphLevel, SingleModuleClient* client) override { - EXPECT_FALSE(pending_clients_.Contains(request.Url())); - pending_clients_.Set(request.Url(), client); + pending_request_url_ = request.Url(); + EXPECT_FALSE(pending_client_); + pending_client_ = client; + } + + void FetchTreeInternal(const ModuleScriptFetchRequest& request, + const AncestorList& list, + ModuleGraphLevel level, + ModuleTreeReachedUrlSet* reached_url_set, + ModuleTreeClient* client) override { + const auto& url = request.Url(); + + auto ancestor_result = pending_tree_ancestor_list_.insert(url, list); + EXPECT_TRUE(ancestor_result.is_new_entry); + + EXPECT_EQ(ModuleGraphLevel::kDependentModuleFetch, level); + EXPECT_TRUE(reached_url_set); + + auto result_map = pending_tree_client_map_.insert(url, client); + EXPECT_TRUE(result_map.is_new_entry); } ModuleScript* GetFetchedModuleScript(const KURL& url) override { @@ -181,17 +230,21 @@ class ModuleTreeLinkerTestModulator final : public DummyModulator { } RefPtr script_state_; - HeapHashMap> pending_clients_; + KURL pending_request_url_; + Member pending_client_; HashMap> dependency_module_requests_map_; HeapHashMap> module_map_; + HeapHashMap> pending_tree_client_map_; + HashMap pending_tree_ancestor_list_; HashSet instantiated_records_; HashSet errored_records_; bool instantiate_should_fail_ = false; }; DEFINE_TRACE(ModuleTreeLinkerTestModulator) { - visitor->Trace(pending_clients_); + visitor->Trace(pending_client_); visitor->Trace(module_map_); + visitor->Trace(pending_tree_client_map_); DummyModulator::Trace(visitor); } @@ -223,7 +276,9 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeNoDeps) { ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch(module_request, AncestorList(), + ModuleGraphLevel::kTopLevelModuleFetch, GetModulator(), + nullptr, client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -245,7 +300,9 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeInstantiationFailure) { ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch(module_request, AncestorList(), + ModuleGraphLevel::kTopLevelModuleFetch, GetModulator(), + nullptr, client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -271,7 +328,9 @@ TEST_F(ModuleTreeLinkerTest, FetchTreePreviousInstantiationFailure) { ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch(module_request, AncestorList(), + ModuleGraphLevel::kTopLevelModuleFetch, GetModulator(), + nullptr, client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -293,7 +352,9 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWithSingleDependency) { ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch(module_request, AncestorList(), + ModuleGraphLevel::kTopLevelModuleFetch, GetModulator(), + nullptr, client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -304,6 +365,10 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWithSingleDependency) { EXPECT_FALSE(client->WasNotifyFinished()); KURL url_dep1(kParsedURLString, "http://example.com/dep1.js"); + auto ancestor_list = GetModulator()->GetAncestorListForTreeFetch(url_dep1); + EXPECT_EQ(1u, ancestor_list.size()); + EXPECT_TRUE(ancestor_list.Contains( + KURL(kParsedURLString, "http://example.com/root.js"))); GetModulator()->ResolveDependentTreeFetch( url_dep1, ModuleTreeLinkerTestModulator::ResolveResult::kSuccess); @@ -320,7 +385,9 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps) { ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch(module_request, AncestorList(), + ModuleGraphLevel::kTopLevelModuleFetch, GetModulator(), + nullptr, client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -342,6 +409,14 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps) { url_deps.push_back(url_dep); } + for (const auto& url_dep : url_deps) { + SCOPED_TRACE(url_dep.GetString()); + auto ancestor_list = GetModulator()->GetAncestorListForTreeFetch(url_dep); + EXPECT_EQ(1u, ancestor_list.size()); + EXPECT_TRUE(ancestor_list.Contains( + KURL(kParsedURLString, "http://example.com/root.js"))); + } + for (const auto& url_dep : url_deps) { EXPECT_FALSE(client->WasNotifyFinished()); GetModulator()->ResolveDependentTreeFetch( @@ -360,7 +435,9 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps1Fail) { ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch(module_request, AncestorList(), + ModuleGraphLevel::kTopLevelModuleFetch, GetModulator(), + nullptr, client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -384,6 +461,10 @@ TEST_F(ModuleTreeLinkerTest, FetchTreeWith3Deps1Fail) { for (const auto& url_dep : url_deps) { SCOPED_TRACE(url_dep.GetString()); + auto ancestor_list = GetModulator()->GetAncestorListForTreeFetch(url_dep); + EXPECT_EQ(1u, ancestor_list.size()); + EXPECT_TRUE(ancestor_list.Contains( + KURL(kParsedURLString, "http://example.com/root.js"))); } auto url_dep = url_deps.back(); @@ -416,8 +497,14 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyTree) { KURL url(kParsedURLString, "http://example.com/depth1.js"); ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); + ModuleTreeReachedUrlSet* reached_url_set = + ModuleTreeReachedUrlSet::CreateFromTopLevelAncestorList(AncestorList()); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch( + module_request, + AncestorList{KURL(kParsedURLString, "http://example.com/root.js")}, + ModuleGraphLevel::kDependentModuleFetch, GetModulator(), reached_url_set, + client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -427,13 +514,19 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyTree) { url, {"./depth2.js"}, ScriptModuleState::kUninstantiated); KURL url_dep2(kParsedURLString, "http://example.com/depth2.js"); + auto ancestor_list = GetModulator()->GetAncestorListForTreeFetch(url_dep2); + EXPECT_EQ(2u, ancestor_list.size()); + EXPECT_TRUE(ancestor_list.Contains( + KURL(kParsedURLString, "http://example.com/root.js"))); + EXPECT_TRUE(ancestor_list.Contains( + KURL(kParsedURLString, "http://example.com/depth1.js"))); GetModulator()->ResolveDependentTreeFetch( url_dep2, ModuleTreeLinkerTestModulator::ResolveResult::kSuccess); EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_TRUE(client->GetModuleScript()->HasInstantiated()); + EXPECT_FALSE(client->GetModuleScript()->HasInstantiated()); } TEST_F(ModuleTreeLinkerTest, FetchDependencyOfCyclicGraph) { @@ -442,8 +535,13 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyOfCyclicGraph) { KURL url(kParsedURLString, "http://example.com/a.js"); ModuleScriptFetchRequest module_request( url, String(), kParserInserted, WebURLRequest::kFetchCredentialsModeOmit); + AncestorList ancestor_list{KURL(kParsedURLString, "http://example.com/a.js")}; + ModuleTreeReachedUrlSet* reached_url_set = + ModuleTreeReachedUrlSet::CreateFromTopLevelAncestorList(ancestor_list); TestModuleTreeClient* client = new TestModuleTreeClient; - registry->Fetch(module_request, GetModulator(), client); + registry->Fetch(module_request, ancestor_list, + ModuleGraphLevel::kDependentModuleFetch, GetModulator(), + reached_url_set, client); EXPECT_FALSE(client->WasNotifyFinished()) << "ModuleTreeLinker should always finish asynchronously."; @@ -452,9 +550,12 @@ TEST_F(ModuleTreeLinkerTest, FetchDependencyOfCyclicGraph) { GetModulator()->ResolveSingleModuleScriptFetch( url, {"./a.js"}, ScriptModuleState::kUninstantiated); + auto ancestor_list2 = GetModulator()->GetAncestorListForTreeFetch(url); + EXPECT_EQ(0u, ancestor_list2.size()); + EXPECT_TRUE(client->WasNotifyFinished()); ASSERT_TRUE(client->GetModuleScript()); - EXPECT_TRUE(client->GetModuleScript()->HasInstantiated()); + EXPECT_FALSE(client->GetModuleScript()->HasInstantiated()); } } // namespace blink diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeReachedUrlSet.h b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeReachedUrlSet.h new file mode 100644 index 0000000000000..c178a5db5cad7 --- /dev/null +++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeReachedUrlSet.h @@ -0,0 +1,59 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef ModuleTreeReachedUrlSet_h +#define ModuleTreeReachedUrlSet_h + +#include "core/CoreExport.h" +#include "core/dom/AncestorList.h" +#include "platform/heap/Handle.h" +#include "platform/weborigin/KURL.h" +#include "platform/weborigin/KURLHash.h" +#include "platform/wtf/HashSet.h" + +namespace blink { + +// ModuleTreeReachedUrlSet aims to reduce number of ModuleTreeLinker +// involved in loading a module graph. +// +// ModuleTreeReachedUrlSet is created per top-level ModuleTreeLinker +// invocations. The instance is shared among all descendants fetch +// ModuleTreeLinker to track a set of module sub-graph root URLs +// which we have started ModuleTreeLinker on. +// +// We consult this ModuleTreeReachedUrlSet before creating +// ModuleTreeLinker for a descendant module sub-graph. +// If the ModuleTreeReachedUrlSet indicates that the sub-graph +// fetch is already taken care of by a existing ModuleTreeLinker, +// a parent ModuleTreeLinker will defer to it and not instantiate a new +// ModuleTreeLinker. +class CORE_EXPORT ModuleTreeReachedUrlSet + : public GarbageCollectedFinalized { + public: + static ModuleTreeReachedUrlSet* CreateFromTopLevelAncestorList( + const AncestorList& list) { + ModuleTreeReachedUrlSet* set = new ModuleTreeReachedUrlSet; + CHECK_LE(list.size(), 1u); + set->set_ = list; + return set; + } + + void ObserveModuleTreeLink(const KURL& url) { + auto result = set_.insert(url); + CHECK(result.is_new_entry); + } + + bool IsAlreadyBeingFetched(const KURL& url) const { + return set_.Contains(url); + } + + DEFINE_INLINE_TRACE() {} + + private: + HashSet set_; +}; + +} // namespace blink + +#endif // ModuleTreeReachedUrlSet_h diff --git a/third_party/WebKit/Source/core/testing/DummyModulator.cpp b/third_party/WebKit/Source/core/testing/DummyModulator.cpp index 604f7f0d83f7e..e40390fd2549c 100644 --- a/third_party/WebKit/Source/core/testing/DummyModulator.cpp +++ b/third_party/WebKit/Source/core/testing/DummyModulator.cpp @@ -68,6 +68,14 @@ void DummyModulator::FetchTree(const ModuleScriptFetchRequest&, NOTREACHED(); } +void DummyModulator::FetchTreeInternal(const ModuleScriptFetchRequest&, + const AncestorList&, + ModuleGraphLevel, + ModuleTreeReachedUrlSet*, + ModuleTreeClient*) { + NOTREACHED(); +}; + void DummyModulator::FetchSingle(const ModuleScriptFetchRequest&, ModuleGraphLevel, SingleModuleClient*) { diff --git a/third_party/WebKit/Source/core/testing/DummyModulator.h b/third_party/WebKit/Source/core/testing/DummyModulator.h index 702d25328e985..49eb0bb445eb3 100644 --- a/third_party/WebKit/Source/core/testing/DummyModulator.h +++ b/third_party/WebKit/Source/core/testing/DummyModulator.h @@ -38,6 +38,11 @@ class DummyModulator : public Modulator { ScriptState* GetScriptState() override; void FetchTree(const ModuleScriptFetchRequest&, ModuleTreeClient*) override; + void FetchTreeInternal(const ModuleScriptFetchRequest&, + const AncestorList&, + ModuleGraphLevel, + ModuleTreeReachedUrlSet*, + ModuleTreeClient*) override; void FetchSingle(const ModuleScriptFetchRequest&, ModuleGraphLevel, SingleModuleClient*) override;