Commit 2f57e742 authored by Eriko Kurimoto's avatar Eriko Kurimoto Committed by Commit Bot

SharedWorker: Fire an error event when type or credentials mismatch

This CL adds the step to check if type and credentials values are same
with these of matched SharedWorkerGlobalScope before connecting to it.

This is just added to the spec.
Spec issue: https://github.com/whatwg/html/issues/5235
Pull request: https://github.com/whatwg/html/pull/5258
WPT: https://github.com/web-platform-tests/wpt/pull/21651

This changes the behavior, and WPT is already added from the public pull
requests to WPT GitHub:
wpt/workers/shared-worker-options-mismatch.html

In this CL, http/tests/workers/shared-worker-shared.html is modified.
Since the way to catch worker's error had changed, this test was no
longer catching the error properly. This test was attempting to create
SharedWorker with non-existing script url so ErrorEvent had been fired.
However, this test couldn't successfully catch ErrorEvent and this is
found out since the console error log added in this CL is printed as
"text diff". Fixing this test is out-of-scope, so this CL avoid this
console error by using an existing script url.

Bug: 1050438
Change-Id: I03232ff19db89560d99d3ddc6a25cdd8f1898222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041386
Commit-Queue: Eriko Kurimoto <elkurin@google.com>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741848}
parent df6eaa77
......@@ -190,7 +190,8 @@ void MockSharedWorkerClient::OnConnected(
on_connected_features_.insert(feature);
}
void MockSharedWorkerClient::OnScriptLoadFailed() {
void MockSharedWorkerClient::OnScriptLoadFailed(
const std::string& error_message) {
DCHECK(!on_script_load_failed_);
on_script_load_failed_ = true;
}
......
......@@ -133,7 +133,7 @@ class MockSharedWorkerClient : public blink::mojom::SharedWorkerClient {
creation_context_type) override;
void OnConnected(
const std::vector<blink::mojom::WebFeature>& features_used) override;
void OnScriptLoadFailed() override;
void OnScriptLoadFailed(const std::string& error_message) override;
void OnFeatureUsed(blink::mojom::WebFeature feature) override;
mojo::Receiver<blink::mojom::SharedWorkerClient> receiver_{this};
......
......@@ -44,7 +44,7 @@ void SharedWorkerConnectorImpl::Connect(
if (!host) {
mojo::Remote<blink::mojom::SharedWorkerClient> remote_client(
std::move(client));
remote_client->OnScriptLoadFailed();
remote_client->OnScriptLoadFailed(/*error_message=*/"");
return;
}
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory;
......
......@@ -123,7 +123,7 @@ SharedWorkerHost::~SharedWorkerHost() {
} else {
// Tell clients that this worker failed to start.
for (const ClientInfo& info : clients_)
info.client->OnScriptLoadFailed();
info.client->OnScriptLoadFailed(/*error_message=*/"");
}
}
......@@ -377,9 +377,9 @@ void SharedWorkerHost::OnReadyForInspection(
}
}
void SharedWorkerHost::OnScriptLoadFailed() {
void SharedWorkerHost::OnScriptLoadFailed(const std::string& error_message) {
for (const ClientInfo& info : clients_)
info.client->OnScriptLoadFailed();
info.client->OnScriptLoadFailed(error_message);
}
void SharedWorkerHost::OnFeatureUsed(blink::mojom::WebFeature feature) {
......
......@@ -169,7 +169,7 @@ class CONTENT_EXPORT SharedWorkerHost : public blink::mojom::SharedWorkerHost,
void OnReadyForInspection(
mojo::PendingRemote<blink::mojom::DevToolsAgent>,
mojo::PendingReceiver<blink::mojom::DevToolsAgentHost>) override;
void OnScriptLoadFailed() override;
void OnScriptLoadFailed(const std::string& error_message) override;
void OnFeatureUsed(blink::mojom::WebFeature feature) override;
// RenderProcessHostObserver:
......
......@@ -122,7 +122,7 @@ void SharedWorkerServiceImpl::ConnectToWorker(
if (!render_frame_host) {
// TODO(crbug.com/31666): Support the case where the requester is a worker
// (i.e., nested worker).
ScriptLoadFailed(std::move(client));
ScriptLoadFailed(std::move(client), /*error_message=*/"");
return;
}
......@@ -134,7 +134,7 @@ void SharedWorkerServiceImpl::ConnectToWorker(
if (is_cross_origin &&
!GetContentClient()->browser()->DoesSchemeAllowCrossOriginSharedWorker(
constructor_origin.scheme())) {
ScriptLoadFailed(std::move(client));
ScriptLoadFailed(std::move(client), /*error_message=*/"");
return;
}
......@@ -149,7 +149,7 @@ void SharedWorkerServiceImpl::ConnectToWorker(
->GetBrowserContext(),
client_render_frame_host_id.child_id,
client_render_frame_host_id.frame_routing_id)) {
ScriptLoadFailed(std::move(client));
ScriptLoadFailed(std::move(client), /*error_message=*/"");
return;
}
......@@ -159,7 +159,20 @@ void SharedWorkerServiceImpl::ConnectToWorker(
// Non-secure contexts cannot connect to secure workers, and secure contexts
// cannot connect to non-secure workers:
if (host->instance().creation_context_type() != creation_context_type) {
ScriptLoadFailed(std::move(client));
ScriptLoadFailed(std::move(client), /*error_message=*/"");
return;
}
// Step 11.4: "If worker global scope is not null, then check if worker
// global scope's type and credentials match the options values. If not,
// queue a task to fire an event named error and abort these steps."
// https://html.spec.whatwg.org/C/#dom-sharedworker
if (host->instance().script_type() != info->options->type ||
host->instance().credentials_mode() != info->options->credentials) {
ScriptLoadFailed(
std::move(client),
"Failed to connect an existing shared worker because the type or "
"credentials given on the SharedWorker constructor doesn't match "
"the existing shared worker's type or credentials.");
return;
}
......@@ -173,7 +186,7 @@ void SharedWorkerServiceImpl::ConnectToWorker(
// Get a storage domain.
SiteInstance* site_instance = render_frame_host->GetSiteInstance();
if (!site_instance) {
ScriptLoadFailed(std::move(client));
ScriptLoadFailed(std::move(client), /*error_message=*/"");
return;
}
std::string storage_domain;
......@@ -401,10 +414,11 @@ SharedWorkerHost* SharedWorkerServiceImpl::FindMatchingSharedWorkerHost(
}
void SharedWorkerServiceImpl::ScriptLoadFailed(
mojo::PendingRemote<blink::mojom::SharedWorkerClient> client) {
mojo::PendingRemote<blink::mojom::SharedWorkerClient> client,
const std::string& error_message) {
mojo::Remote<blink::mojom::SharedWorkerClient> remote_client(
std::move(client));
remote_client->OnScriptLoadFailed();
remote_client->OnScriptLoadFailed(error_message);
}
} // namespace content
......@@ -121,7 +121,8 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
const url::Origin& constructor_origin);
void ScriptLoadFailed(
mojo::PendingRemote<blink::mojom::SharedWorkerClient> client);
mojo::PendingRemote<blink::mojom::SharedWorkerClient> client,
const std::string& error_message);
// The ID that the next SharedWorkerInstance will be assigned.
int64_t next_shared_worker_instance_id_ = 0;
......
......@@ -132,8 +132,9 @@ void EmbeddedSharedWorkerStub::WorkerReadyForInspection(
host_->OnReadyForInspection(std::move(remote), std::move(receiver));
}
void EmbeddedSharedWorkerStub::WorkerScriptLoadFailed() {
host_->OnScriptLoadFailed();
void EmbeddedSharedWorkerStub::WorkerScriptLoadFailed(
const std::string& error_message) {
host_->OnScriptLoadFailed(error_message);
pending_channels_.clear();
}
......
......@@ -84,7 +84,7 @@ class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient,
void WorkerReadyForInspection(
mojo::ScopedMessagePipeHandle devtools_agent_ptr_info,
mojo::ScopedMessagePipeHandle devtools_agent_host_request) override;
void WorkerScriptLoadFailed() override;
void WorkerScriptLoadFailed(const std::string& error_message) override;
void WorkerScriptEvaluated(bool success) override;
scoped_refptr<blink::WebWorkerFetchContext> CreateWorkerFetchContext()
override;
......
......@@ -19,7 +19,9 @@ interface SharedWorkerClient {
OnConnected(array<WebFeature> features_used);
// Indicates that the shared worker script failed to load.
OnScriptLoadFailed();
// If |error_message| is not empty, it will be printed in the creator frame's
// console.
OnScriptLoadFailed(string error_message);
// Indicates that the shared worker used a feature. This is intended to be
// logged by the client-side feature logging infrastructure.
......
......@@ -26,7 +26,9 @@ interface SharedWorkerHost {
pending_receiver<blink.mojom.DevToolsAgentHost> agent_host);
// Indicates that the script failed to load.
OnScriptLoadFailed();
// If |error_message| is not empty, it will be ptinted in the creator frame's
// console.
OnScriptLoadFailed(string error_message);
// Indicates that the shared worker used a feature. This is intended to be
// logged by the client-side feature logging infrastructure.
......
......@@ -52,7 +52,7 @@ class WebSharedWorkerClient {
virtual void WorkerReadyForInspection(
mojo::ScopedMessagePipeHandle devtools_agent_ptr_info,
mojo::ScopedMessagePipeHandle devtools_agent_host_request) {}
virtual void WorkerScriptLoadFailed() = 0;
virtual void WorkerScriptLoadFailed(const std::string& error_message) = 0;
virtual void WorkerScriptEvaluated(bool success) = 0;
// Called on the main thread during initialization. Creates a new
......
......@@ -86,7 +86,7 @@ void WebSharedWorkerImpl::TerminateWorkerThread() {
asked_to_terminate_ = true;
if (!worker_thread_) {
client_->WorkerScriptLoadFailed();
client_->WorkerScriptLoadFailed(/*error_message=*/"");
// The worker thread hasn't been started yet. Immediately notify the client
// of worker termination.
client_->WorkerContextDestroyed();
......@@ -104,14 +104,14 @@ void WebSharedWorkerImpl::CountFeature(WebFeature feature) {
void WebSharedWorkerImpl::DidFailToFetchClassicScript() {
DCHECK(IsMainThread());
client_->WorkerScriptLoadFailed();
client_->WorkerScriptLoadFailed("Failed to fetch a worker script.");
TerminateWorkerThread();
// DidTerminateWorkerThread() will be called asynchronously.
}
void WebSharedWorkerImpl::DidFailToFetchModuleScript() {
DCHECK(IsMainThread());
client_->WorkerScriptLoadFailed();
client_->WorkerScriptLoadFailed("Failed to fetch a worker script.");
TerminateWorkerThread();
// DidTerminateWorkerThread() will be called asynchronously.
}
......
......@@ -7,6 +7,7 @@
#include "base/logging.h"
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
#include "third_party/blink/renderer/core/workers/shared_worker.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
......@@ -41,8 +42,14 @@ void SharedWorkerClient::OnConnected(
OnFeatureUsed(feature);
}
void SharedWorkerClient::OnScriptLoadFailed() {
void SharedWorkerClient::OnScriptLoadFailed(const String& error_message) {
worker_->SetIsBeingConnected(false);
if (!error_message.IsEmpty()) {
worker_->GetExecutionContext()->AddConsoleMessage(
MakeGarbageCollected<ConsoleMessage>(
mojom::blink::ConsoleMessageSource::kWorker,
mojom::blink::ConsoleMessageLevel::kError, error_message));
}
worker_->DispatchEvent(*Event::CreateCancelable(event_type_names::kError));
// |this| can be destroyed at this point, for example, when a frame hosting
// this shared worker is detached in the error handler, and closes mojo's
......
......@@ -25,7 +25,7 @@ class SharedWorkerClient final : public mojom::blink::SharedWorkerClient {
// mojom::blink::SharedWorkerClient overrides.
void OnCreated(mojom::SharedWorkerCreationContextType) override;
void OnConnected(const Vector<mojom::WebFeature>& features_used) override;
void OnScriptLoadFailed() override;
void OnScriptLoadFailed(const String& error_message) override;
void OnFeatureUsed(mojom::WebFeature feature) override;
private:
......
......@@ -3024,12 +3024,6 @@ crbug.com/626703 [ Win ] external/wpt/css/css-text/letter-spacing/letter-spacing
crbug.com/626703 [ Linux ] virtual/web-components-v0-disabled/external/wpt/html/semantics/embedded-content/the-img-element/image-loading-lazy-in-cross-origin-ifame-001.sub.html [ Timeout ]
crbug.com/626703 [ Mac ] virtual/web-components-v0-disabled/external/wpt/html/semantics/embedded-content/the-img-element/image-loading-lazy-in-cross-origin-ifame-001.sub.html [ Timeout ]
crbug.com/626703 [ Win ] virtual/web-components-v0-disabled/external/wpt/html/semantics/embedded-content/the-img-element/image-loading-lazy-in-cross-origin-ifame-001.sub.html [ Timeout ]
crbug.com/626703 [ Linux ] virtual/omt-worker-fetch/external/wpt/workers/shared-worker-options-mismatch.html [ Timeout ]
crbug.com/626703 [ Mac ] virtual/omt-worker-fetch/external/wpt/workers/shared-worker-options-mismatch.html [ Timeout ]
crbug.com/626703 [ Win ] virtual/omt-worker-fetch/external/wpt/workers/shared-worker-options-mismatch.html [ Timeout ]
crbug.com/626703 [ Linux ] external/wpt/workers/shared-worker-options-mismatch.html [ Timeout ]
crbug.com/626703 [ Mac ] external/wpt/workers/shared-worker-options-mismatch.html [ Timeout ]
crbug.com/626703 [ Win ] external/wpt/workers/shared-worker-options-mismatch.html [ Timeout ]
crbug.com/626703 [ Retina ] virtual/sxg-subresource/external/wpt/signed-exchange/reporting/sxg-reporting-navigation-parse_error.tentative.html [ Timeout ]
crbug.com/626703 [ Linux ] external/wpt/css/css-fonts/variations/font-slant-1.html [ Failure ]
crbug.com/626703 [ Mac ] external/wpt/css/css-fonts/variations/font-slant-1.html [ Failure ]
......
......@@ -18,6 +18,13 @@ promise_test(async () => {
assert_equals(msg_event.data, 'TypeError');
}, 'importScripts() on module worker should throw an exception.');
promise_test(() => {
const scriptURL = 'resources/non-existent-worker.js';
const worker = new SharedWorker(scriptURL, { type: 'module' });
return new Promise(resolve => worker.onerror = resolve);
}, 'Worker construction for non-existent script should dispatch an ' +
'ErrorEvent.');
test(() => {
const scriptURL = 'http://invalid:123$';
assert_throws_dom('SyntaxError',
......
......@@ -18,7 +18,7 @@ var worker = new SharedWorker('resources/shared-worker-common.js', 'name');
var worker2 = new SharedWorker('resources/shared-worker-common.js', 'name');
try {
new SharedWorker('resources/some-other-url.js', 'name');
new SharedWorker('resources/empty.js', 'name');
log("PASS: Creating SharedWorker with different URLs but the same name");
} catch (ex) {
var exString = ex.toString();
......@@ -26,7 +26,6 @@ try {
log("FAIL: Exception thrown when creating SharedWorker with different URLs but same name: " + exString);
}
// Set something in global context in one worker, read value back on other worker, to make sure they are truly shared.
worker.port.postMessage("eval self.foo");
worker.port.onmessage = function(event)
......
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