Skip to content
This repository has been archived by the owner on Jul 18, 2018. It is now read-only.

Commit

Permalink
Revert "Flatten ModuleTreeLinker"
Browse files Browse the repository at this point in the history
This reverts commit 5a168df.

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
> whatwg/html#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<KURL>
> 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
> whatwg/html#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 <hiroshige@chromium.org>
> Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> 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 <kouhei@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503059}
  • Loading branch information
nyaxt authored and Commit Bot committed Sep 20, 2017
1 parent 551c6b8 commit e81cbfd
Show file tree
Hide file tree
Showing 14 changed files with 571 additions and 384 deletions.
22 changes: 22 additions & 0 deletions third_party/WebKit/Source/core/dom/AncestorList.h
Original file line number Diff line number Diff line change
@@ -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<KURL>;

} // namespace blink

#endif
1 change: 1 addition & 0 deletions third_party/WebKit/Source/core/dom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ blink_core_sources("dom") {
"AccessibleNode.h",
"AccessibleNodeList.cpp",
"AccessibleNodeList.h",
"AncestorList.h",
"AnimationWorkletProxyClient.cpp",
"AnimationWorkletProxyClient.h",
"Attr.cpp",
Expand Down
9 changes: 9 additions & 0 deletions third_party/WebKit/Source/core/dom/Modulator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -23,6 +24,7 @@ class ModuleScript;
class ModuleScriptFetcher;
class ModuleScriptFetchRequest;
class ModuleScriptLoaderClient;
class ModuleTreeReachedUrlSet;
class ScriptModuleResolver;
class ScriptState;
class ScriptValue;
Expand Down Expand Up @@ -87,6 +89,13 @@ class CORE_EXPORT Modulator : public GarbageCollectedFinalized<Modulator>,
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
Expand Down
22 changes: 17 additions & 5 deletions third_party/WebKit/Source/core/dom/ModulatorImplBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,30 @@ 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
// algorithm with result.
// 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) {
Expand Down
6 changes: 6 additions & 0 deletions third_party/WebKit/Source/core/dom/ModulatorImplBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class ExecutionContext;
class ModuleMap;
class ModuleScriptLoaderRegistry;
class ModuleTreeLinkerRegistry;
class ModuleTreeReachedUrlSet;
class ScriptState;
class WebTaskRunner;

Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions third_party/WebKit/Source/core/loader/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading

0 comments on commit e81cbfd

Please sign in to comment.