Commit 93db51c0 authored by Camillo Bruni's avatar Camillo Bruni Committed by Chromium LUCI CQ

[blink] Don't share ScriptResources between module and classic scripts

Add ScriptResource::ScriptType in preparation for module streaming and
stop sharing ScriptResources with different ScriptTypes.

Currently the ScriptResource creates the ScriptStreamer which in turn is
responsible for creating the v8::ScriptCompiler::ScriptStreamingTask.
To support module streaming compilation we need to pass on the script
type from the fetch location to V8.

Bug: 1061857
Change-Id: I6d8d032b0f42dca904493f1e80b0ff31878aa123
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2556942
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarLeszek Swirski <leszeks@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833325}
parent d5454a72
......@@ -485,6 +485,8 @@ bool ScriptStreamer::TryStartStreamingTask() {
source_ = std::make_unique<v8::ScriptCompiler::StreamedSource>(
std::move(stream_ptr), encoding_);
// TODO(crbug.com/1061857) Pass ScriptResource::ScriptType to the streaming
// task.
std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask>
script_streaming_task(base::WrapUnique(v8::ScriptCompiler::StartStreaming(
V8PerIsolateData::MainThreadIsolate(), source_.get())));
......
......@@ -100,6 +100,7 @@ Resource* PreloadRequest::Start(Document* document) {
DCHECK_EQ(resource_type_, ResourceType::kScript);
params.SetCrossOriginAccessControl(
origin, ScriptLoader::ModuleScriptCredentialsMode(cross_origin_));
params.SetModuleScript();
} else if (cross_origin_ != kCrossOriginAttributeNotSet) {
params.SetCrossOriginAccessControl(origin, cross_origin_);
}
......
......@@ -4,7 +4,9 @@
#include "third_party/blink/renderer/core/loader/modulescript/document_module_script_fetcher.h"
#include "third_party/blink/public/mojom/script/script_type.mojom-blink-forward.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
#include "third_party/blink/renderer/core/loader/resource/script_resource.h"
#include "third_party/blink/renderer/platform/bindings/parkable_string.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
#include "third_party/blink/renderer/platform/wtf/vector.h"
......@@ -20,6 +22,7 @@ void DocumentModuleScriptFetcher::Fetch(
ResourceFetcher* fetch_client_settings_object_fetcher,
ModuleGraphLevel level,
ModuleScriptFetcher::Client* client) {
DCHECK_EQ(fetch_params.GetScriptType(), mojom::blink::ScriptType::kModule);
DCHECK(fetch_client_settings_object_fetcher);
DCHECK(!client_);
client_ = client;
......
......@@ -30,6 +30,7 @@ void InstalledServiceWorkerModuleScriptFetcher::Fetch(
ResourceFetcher*,
ModuleGraphLevel level,
ModuleScriptFetcher::Client* client) {
DCHECK_EQ(fetch_params.GetScriptType(), mojom::blink::ScriptType::kModule);
DCHECK(global_scope_->IsContextThread());
auto* installed_scripts_manager = global_scope_->GetInstalledScriptsManager();
DCHECK(installed_scripts_manager);
......
......@@ -7,6 +7,7 @@
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/renderer/core/dom/dom_implementation.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
#include "third_party/blink/renderer/core/loader/resource/script_resource.h"
#include "third_party/blink/renderer/core/loader/subresource_integrity_helper.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
#include "third_party/blink/renderer/platform/loader/cors/cors.h"
......@@ -34,10 +35,11 @@ void ModuleScriptFetcher::Trace(Visitor* visitor) const {
// <specdef href="https://html.spec.whatwg.org/C/#fetch-a-single-module-script">
bool ModuleScriptFetcher::WasModuleLoadSuccessful(
Resource* resource,
ScriptResource* resource,
HeapVector<Member<ConsoleMessage>>* error_messages,
ModuleScriptCreationParams::ModuleType* module_type) {
DCHECK(error_messages);
DCHECK_EQ(resource->GetScriptType(), mojom::blink::ScriptType::kModule);
if (resource) {
SubresourceIntegrityHelper::GetConsoleMessages(
......
......@@ -53,7 +53,7 @@ class CORE_EXPORT ModuleScriptFetcher : public ResourceClient {
protected:
static bool WasModuleLoadSuccessful(
Resource* resource,
ScriptResource* resource,
HeapVector<Member<ConsoleMessage>>* error_messages,
ModuleScriptCreationParams::ModuleType* module_type);
};
......
......@@ -154,6 +154,7 @@ void ModuleScriptLoader::FetchInternal(
// Note: |options| should not be modified after here.
FetchParameters fetch_params(std::move(resource_request), options);
fetch_params.SetModuleScript();
// <spec label="SMSR">... its integrity metadata to options's integrity
// metadata, ...</spec>
......
......@@ -37,6 +37,7 @@ void WorkerModuleScriptFetcher::Fetch(
ResourceFetcher* fetch_client_settings_object_fetcher,
ModuleGraphLevel level,
ModuleScriptFetcher::Client* client) {
DCHECK_EQ(fetch_params.GetScriptType(), mojom::blink::ScriptType::kModule);
DCHECK(global_scope_->IsContextThread());
DCHECK(!fetch_client_settings_object_fetcher_);
fetch_client_settings_object_fetcher_ = fetch_client_settings_object_fetcher;
......
......@@ -19,6 +19,7 @@ void WorkletModuleScriptFetcher::Fetch(
ResourceFetcher* fetch_client_settings_object_fetcher,
ModuleGraphLevel level,
ModuleScriptFetcher::Client* client) {
DCHECK_EQ(fetch_params.GetScriptType(), mojom::blink::ScriptType::kModule);
if (module_responses_map_->GetEntry(
fetch_params.Url(), client,
fetch_client_settings_object_fetcher->GetTaskRunner())) {
......
......@@ -84,31 +84,35 @@ ScriptResource* ScriptResource::Fetch(FetchParameters& params,
DCHECK(IsRequestContextSupported(
params.GetResourceRequest().GetRequestContext()));
auto* resource = To<ScriptResource>(fetcher->RequestResource(
params, ScriptResourceFactory(streaming_allowed), client));
params, ScriptResourceFactory(streaming_allowed, params.GetScriptType()),
client));
return resource;
}
ScriptResource* ScriptResource::CreateForTest(
const KURL& url,
const WTF::TextEncoding& encoding) {
const WTF::TextEncoding& encoding,
mojom::blink::ScriptType script_type) {
ResourceRequest request(url);
request.SetCredentialsMode(network::mojom::CredentialsMode::kOmit);
ResourceLoaderOptions options(nullptr /* world */);
TextResourceDecoderOptions decoder_options(
TextResourceDecoderOptions::kPlainTextContent, encoding);
return MakeGarbageCollected<ScriptResource>(request, options, decoder_options,
kNoStreaming);
kNoStreaming, script_type);
}
ScriptResource::ScriptResource(
const ResourceRequest& resource_request,
const ResourceLoaderOptions& options,
const TextResourceDecoderOptions& decoder_options,
StreamingAllowed streaming_allowed)
StreamingAllowed streaming_allowed,
mojom::blink::ScriptType script_type)
: TextResource(resource_request,
ResourceType::kScript,
options,
decoder_options) {
decoder_options),
script_type_(script_type) {
static bool script_streaming_enabled =
base::FeatureList::IsEnabled(features::kScriptStreaming);
......@@ -130,6 +134,13 @@ void ScriptResource::Trace(Visitor* visitor) const {
TextResource::Trace(visitor);
}
Resource::MatchStatus ScriptResource::CanReuse(
const FetchParameters& params) const {
if (script_type_ != params.GetScriptType())
return Resource::MatchStatus::kScriptTypeDoesNotMatch;
return Resource::CanReuse(params);
}
void ScriptResource::OnMemoryDump(WebMemoryDumpLevelOfDetail level_of_detail,
WebProcessMemoryDump* memory_dump) const {
Resource::OnMemoryDump(level_of_detail, memory_dump);
......
......@@ -28,6 +28,8 @@
#include <memory>
#include "third_party/blink/public/mojom/script/script_type.mojom-blink-forward.h"
#include "third_party/blink/public/mojom/script/script_type.mojom-shared.h"
#include "third_party/blink/renderer/bindings/core/v8/script_streamer.h"
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/loader/resource/text_resource.h"
......@@ -46,7 +48,8 @@ class ScriptCachedMetadataHandler;
class SingleCachedMetadataHandler;
// ScriptResource is a resource representing a JavaScript, either a classic or
// module script.
// module script. ScriptResources are not shared between classic and module
// scripts.
//
// In addition to loading the script, a ScriptResource can optionally stream the
// script to the JavaScript parser/compiler, using a ScriptStreamer. In this
......@@ -68,15 +71,20 @@ class CORE_EXPORT ScriptResource final : public TextResource {
StreamingAllowed);
// Public for testing
static ScriptResource* CreateForTest(const KURL& url,
const WTF::TextEncoding& encoding);
static ScriptResource* CreateForTest(
const KURL& url,
const WTF::TextEncoding& encoding,
mojom::blink::ScriptType = mojom::blink::ScriptType::kClassic);
ScriptResource(const ResourceRequest&,
const ResourceLoaderOptions&,
const TextResourceDecoderOptions&,
StreamingAllowed);
StreamingAllowed,
mojom::blink::ScriptType);
~ScriptResource() override;
Resource::MatchStatus CanReuse(const FetchParameters& params) const override;
size_t CodeCacheSize() const override;
void ResponseReceived(const ResourceResponse&) override;
void ResponseBodyReceived(
......@@ -98,6 +106,8 @@ class CORE_EXPORT ScriptResource final : public TextResource {
SingleCachedMetadataHandler* CacheHandler();
mojom::blink::ScriptType GetScriptType() const { return script_type_; }
// Gets the script streamer from the ScriptResource, clearing the resource's
// streamer so that it cannot be used twice.
ScriptStreamer* TakeStreamer();
......@@ -146,21 +156,24 @@ class CORE_EXPORT ScriptResource final : public TextResource {
class ScriptResourceFactory : public ResourceFactory {
public:
explicit ScriptResourceFactory(StreamingAllowed streaming_allowed)
explicit ScriptResourceFactory(StreamingAllowed streaming_allowed,
mojom::blink::ScriptType script_type)
: ResourceFactory(ResourceType::kScript,
TextResourceDecoderOptions::kPlainTextContent),
streaming_allowed_(streaming_allowed) {}
streaming_allowed_(streaming_allowed),
script_type_(script_type) {}
Resource* Create(
const ResourceRequest& request,
const ResourceLoaderOptions& options,
const TextResourceDecoderOptions& decoder_options) const override {
return MakeGarbageCollected<ScriptResource>(
request, options, decoder_options, streaming_allowed_);
request, options, decoder_options, streaming_allowed_, script_type_);
}
private:
StreamingAllowed streaming_allowed_;
mojom::blink::ScriptType script_type_;
};
bool CanUseCacheValidator() const override;
......@@ -182,6 +195,7 @@ class CORE_EXPORT ScriptResource final : public TextResource {
ScriptStreamer::NotStreamingReason::kInvalid;
StreamingState streaming_state_ = StreamingState::kWaitingForDataPipe;
Member<ScriptCachedMetadataHandler> cached_metadata_handler_;
const mojom::blink::ScriptType script_type_;
};
template <>
......
......@@ -114,6 +114,7 @@ class ModuleMapTestModulator final : public DummyModulator {
ResourceFetcher*,
ModuleGraphLevel,
ModuleScriptFetcher::Client* client) override {
CHECK_EQ(request.GetScriptType(), mojom::blink::ScriptType::kModule);
TestRequest* test_request = MakeGarbageCollected<TestRequest>(
ModuleScriptCreationParams(
request.Url(),
......
......@@ -77,6 +77,7 @@ class WorkletModuleResponsesMapTest : public testing::Test {
mojom::blink::RequestContextType::SCRIPT);
FetchParameters fetch_params =
FetchParameters::CreateForTest(std::move(resource_request));
fetch_params.SetModuleScript();
WorkletModuleScriptFetcher* module_fetcher =
MakeGarbageCollected<WorkletModuleScriptFetcher>(
map_.Get(), ModuleScriptLoader::CreatePassKeyForTests());
......
......@@ -130,4 +130,9 @@ void FetchParameters::SetLazyImageNonBlocking() {
image_request_behavior_ = kNonBlockingImage;
}
void FetchParameters::SetModuleScript() {
DCHECK_EQ(mojom::blink::ScriptType::kClassic, script_type_);
script_type_ = mojom::blink::ScriptType::kModule;
}
} // namespace blink
......@@ -27,6 +27,8 @@
#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_LOADER_FETCH_FETCH_PARAMETERS_H_
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink-forward.h"
#include "third_party/blink/public/mojom/script/script_type.mojom-blink-forward.h"
#include "third_party/blink/public/mojom/script/script_type.mojom-shared.h"
#include "third_party/blink/public/platform/web_url_request.h"
#include "third_party/blink/renderer/platform/loader/fetch/client_hints_preferences.h"
#include "third_party/blink/renderer/platform/loader/fetch/cross_origin_attribute_value.h"
......@@ -192,6 +194,10 @@ class PLATFORM_EXPORT FetchParameters {
void SetLazyImageDeferred();
void SetLazyImageNonBlocking();
mojom::blink::ScriptType GetScriptType() const { return script_type_; }
void SetModuleScript();
// See documentation in blink::ResourceRequest.
bool IsFromOriginDirtyStyleSheet() const {
return is_from_origin_dirty_style_sheet_;
......@@ -216,6 +222,7 @@ class PLATFORM_EXPORT FetchParameters {
ResourceWidth resource_width_;
ClientHintsPreferences client_hint_preferences_;
ImageRequestBehavior image_request_behavior_;
mojom::blink::ScriptType script_type_ = mojom::blink::ScriptType::kClassic;
bool is_stale_revalidation_ = false;
bool is_from_origin_dirty_style_sheet_ = false;
};
......
......@@ -140,6 +140,9 @@ class PLATFORM_EXPORT Resource : public GarbageCollected<Resource>,
// Match fails due to different request headers.
kRequestHeadersDoNotMatch,
// Match fails due to different script types.
kScriptTypeDoesNotMatch,
};
~Resource() override;
......
......@@ -1449,6 +1449,9 @@ void ResourceFetcher::PrintPreloadWarning(Resource* resource,
case Resource::MatchStatus::kRequestHeadersDoNotMatch:
builder.Append("because the request headers do not match.");
break;
case Resource::MatchStatus::kScriptTypeDoesNotMatch:
builder.Append("because the script type does not match.");
break;
}
console_logger_->AddConsoleMessage(mojom::ConsoleMessageSource::kOther,
mojom::ConsoleMessageLevel::kWarning,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment