Commit 3e1adea2 authored by Yutaka Hirano's avatar Yutaka Hirano Committed by Commit Bot

Pass AccessControlStatus to WorkerOrWorkletScriptController::Evaluate

As part of the effort to deprecate kNotSharableCrossOrigin, stop using
it in WorkerOrWorkletScriptController::Evaluate. There are three users:
 - worker toplevel scripts
 - importScripts
 - ScheduledAction

Workers are origin-bound, so we can use kSharableCrossOrigin (the name
is misleading, I will rename it, after all the refactoring is done).

importScripts() always use "no-cors" mode, so we can calculate the
AccessControlStatus by using SecurityOrigin::CanReadContent. I don't
want to place the ad-hoc logic, but plumbing it to response type is
hard because of LoadScriptFromInstalledScriptsManager.

For ScheduledAction, I gave up removing kNotSharableCrossOrigin this
time, as it seems consistent with Execute(LocalFrame*). I will remove
the occurrence in a subsequent CL.

Bug: 875153
Change-Id: I0d3c65e5fc152caed0265e7868e45ea9882ce487
Reviewed-on: https://chromium-review.googlesource.com/1180638
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591255}
parent ee21cec2
......@@ -206,8 +206,10 @@ void ScheduledAction::Execute(WorkerGlobalScope* worker) {
function, worker, script_state_->GetContext()->Global(), info.size(),
info.data(), script_state_->GetIsolate());
} else {
worker->ScriptController()->Evaluate(ScriptSourceCode(
code_, ScriptSourceLocationType::kEvalForScheduledAction));
worker->ScriptController()->Evaluate(
ScriptSourceCode(code_,
ScriptSourceLocationType::kEvalForScheduledAction),
kNotSharableCrossOrigin);
}
}
......
......@@ -261,6 +261,7 @@ bool WorkerOrWorkletScriptController::InitializeContextIfNeeded(
ScriptValue WorkerOrWorkletScriptController::EvaluateInternal(
const ScriptSourceCode& source_code,
AccessControlStatus access_control_status,
V8CacheOptions v8_cache_options) {
DCHECK(IsContextInitialized());
......@@ -284,7 +285,7 @@ ScriptValue WorkerOrWorkletScriptController::EvaluateInternal(
std::tie(compile_options, produce_cache_options, no_cache_reason) =
V8CodeCache::GetCompileOptions(v8_cache_options, source_code);
if (V8ScriptRunner::CompileScript(script_state_, source_code,
kSharableCrossOrigin, compile_options,
access_control_status, compile_options,
no_cache_reason, referrer_info)
.ToLocal(&compiled_script)) {
maybe_result = V8ScriptRunner::RunCompiledScript(isolate_, compiled_script,
......@@ -319,13 +320,14 @@ ScriptValue WorkerOrWorkletScriptController::EvaluateInternal(
bool WorkerOrWorkletScriptController::Evaluate(
const ScriptSourceCode& source_code,
AccessControlStatus access_control_status,
ErrorEvent** error_event,
V8CacheOptions v8_cache_options) {
if (IsExecutionForbidden())
return false;
ExecutionState state(this);
EvaluateInternal(source_code, v8_cache_options);
EvaluateInternal(source_code, access_control_status, v8_cache_options);
if (IsExecutionForbidden())
return false;
......@@ -338,19 +340,19 @@ bool WorkerOrWorkletScriptController::Evaluate(
return false;
}
if (global_scope_->ShouldSanitizeScriptError(state.location_->Url(),
kNotSharableCrossOrigin)) {
access_control_status)) {
*error_event = ErrorEvent::CreateSanitizedError(world_.get());
} else {
*error_event =
ErrorEvent::Create(state.error_message, state.location_->Clone(),
state.exception, world_.get());
StoreExceptionForInspector(script_state_, *error_event,
state.exception.V8Value(),
script_state_->GetContext()->Global());
}
StoreExceptionForInspector(script_state_, *error_event,
state.exception.V8Value(),
script_state_->GetContext()->Global());
} else {
DCHECK(!global_scope_->ShouldSanitizeScriptError(
state.location_->Url(), kNotSharableCrossOrigin));
DCHECK(!global_scope_->ShouldSanitizeScriptError(state.location_->Url(),
access_control_status));
ErrorEvent* event = nullptr;
if (state.error_event_from_imported_script_) {
event = state.error_event_from_imported_script_.Release();
......@@ -359,7 +361,7 @@ bool WorkerOrWorkletScriptController::Evaluate(
ErrorEvent::Create(state.error_message, state.location_->Clone(),
state.exception, world_.get());
}
global_scope_->DispatchErrorEvent(event, kNotSharableCrossOrigin);
global_scope_->DispatchErrorEvent(event, access_control_status);
}
return false;
}
......@@ -369,7 +371,7 @@ bool WorkerOrWorkletScriptController::Evaluate(
ScriptValue WorkerOrWorkletScriptController::EvaluateAndReturnValueForTest(
const ScriptSourceCode& source_code) {
ExecutionState state(this);
return EvaluateInternal(source_code, kV8CacheOptionsDefault);
return EvaluateInternal(source_code, kOpaqueResource, kV8CacheOptionsDefault);
}
void WorkerOrWorkletScriptController::ForbidExecution() {
......
......@@ -36,6 +36,7 @@
#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_cache_options.h"
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/platform/loader/fetch/access_control_status.h"
#include "third_party/blink/renderer/platform/wtf/allocator.h"
#include "third_party/blink/renderer/platform/wtf/text/text_position.h"
#include "v8/include/v8.h"
......@@ -61,6 +62,7 @@ class CORE_EXPORT WorkerOrWorkletScriptController final
// Returns true if the evaluation completed with no uncaught exception.
bool Evaluate(const ScriptSourceCode&,
AccessControlStatus access_control_status,
ErrorEvent** = nullptr,
V8CacheOptions = kV8CacheOptionsDefault);
......@@ -107,6 +109,7 @@ class CORE_EXPORT WorkerOrWorkletScriptController final
// Evaluate a script file in the current execution environment.
ScriptValue EvaluateInternal(const ScriptSourceCode&,
AccessControlStatus,
V8CacheOptions);
void DisposeContextIfNeeded();
......
......@@ -367,9 +367,10 @@ void WebSharedWorkerImpl::ContinueOnScriptLoaderFinished() {
worker_inspector_proxy_->WorkerThreadCreated(document, GetWorkerThread(),
script_response_url);
// TODO(nhiroki): Support module workers (https://crbug.com/680046).
GetWorkerThread()->EvaluateClassicScript(script_response_url, source_code,
nullptr /* cached_meta_data */,
v8_inspector::V8StackTraceId());
// Shared worker is origin-bound, so use kSharableCrossOrigin.
GetWorkerThread()->EvaluateClassicScript(
script_response_url, kSharableCrossOrigin, source_code,
nullptr /* cached_meta_data */, v8_inspector::V8StackTraceId());
client_->WorkerScriptLoaded();
}
......
......@@ -56,8 +56,10 @@ void DedicatedWorkerMessagingProxy::StartWorkerGlobalScope(
CreateBackingThreadStartupData(ToIsolate(GetExecutionContext())));
if (options.type() == "classic") {
// Dedicated worker is origin-bound, so use kSharableCrossOrigin.
GetWorkerThread()->EvaluateClassicScript(
script_url, source_code, nullptr /* cached_meta_data */, stack_id);
script_url, kSharableCrossOrigin, source_code,
nullptr /* cached_meta_data */, stack_id);
} else if (options.type() == "module") {
network::mojom::FetchCredentialsMode credentials_mode;
bool result =
......
......@@ -22,6 +22,7 @@
#include "third_party/blink/renderer/core/workers/worker_thread.h"
#include "third_party/blink/renderer/core/workers/worker_thread_test_helper.h"
#include "third_party/blink/renderer/platform/cross_thread_functional.h"
#include "third_party/blink/renderer/platform/loader/fetch/access_control_status.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
#include "third_party/blink/renderer/platform/weborigin/security_policy.h"
......@@ -143,9 +144,9 @@ class DedicatedWorkerMessagingProxyForTest
WorkerBackingThreadStartupData(
WorkerBackingThreadStartupData::HeapLimitMode::kDefault,
WorkerBackingThreadStartupData::AtomicsWaitMode::kAllow));
GetWorkerThread()->EvaluateClassicScript(script_url, source,
nullptr /* cached_meta_data */,
v8_inspector::V8StackTraceId());
GetWorkerThread()->EvaluateClassicScript(
script_url, kOpaqueResource, source, nullptr /* cached_meta_data */,
v8_inspector::V8StackTraceId());
}
DedicatedWorkerThreadForTest* GetDedicatedWorkerThread() {
......
......@@ -195,6 +195,14 @@ void WorkerGlobalScope::importScripts(const Vector<String>& urls,
return;
}
// importScripts always uses "no-cors", so simply checking the origin is
// enough.
// TODO(yhirano): Remove this ad-hoc logic and use the response type.
const AccessControlStatus access_control_status =
execution_context.GetSecurityOrigin()->CanReadContent(response_url)
? kSharableCrossOrigin
: kOpaqueResource;
ErrorEvent* error_event = nullptr;
SingleCachedMetadataHandler* handler(
CreateWorkerScriptCachedMetadataHandler(complete_url,
......@@ -204,7 +212,7 @@ void WorkerGlobalScope::importScripts(const Vector<String>& urls,
ScriptController()->Evaluate(
ScriptSourceCode(source_code, ScriptSourceLocationType::kUnknown,
handler, response_url),
&error_event, v8_cache_options_);
access_control_status, &error_event, v8_cache_options_);
if (error_event) {
ScriptController()->RethrowExceptionFromImportedScript(error_event,
exception_state);
......@@ -322,20 +330,22 @@ void WorkerGlobalScope::TasksWereUnpaused() {
void WorkerGlobalScope::EvaluateClassicScriptPausable(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data,
const v8_inspector::V8StackTraceId& stack_id) {
if (IsContextPaused()) {
paused_calls_.push_back(
WTF::Bind(&WorkerGlobalScope::EvaluateClassicScriptPausable,
WrapWeakPersistent(this), script_url, source_code,
WTF::Passed(std::move(cached_meta_data)), stack_id));
paused_calls_.push_back(WTF::Bind(
&WorkerGlobalScope::EvaluateClassicScriptPausable,
WrapWeakPersistent(this), script_url, access_control_status,
source_code, WTF::Passed(std::move(cached_meta_data)), stack_id));
return;
}
ThreadDebugger* debugger = ThreadDebugger::From(GetThread()->GetIsolate());
if (debugger)
debugger->ExternalAsyncTaskStarted(stack_id);
EvaluateClassicScript(script_url, source_code, std::move(cached_meta_data));
EvaluateClassicScript(script_url, access_control_status, source_code,
std::move(cached_meta_data));
if (debugger)
debugger->ExternalAsyncTaskFinished(stack_id);
}
......@@ -382,6 +392,7 @@ void WorkerGlobalScope::ReceiveMessagePausable(
void WorkerGlobalScope::EvaluateClassicScript(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data) {
DCHECK(IsContextThread());
......@@ -395,7 +406,7 @@ void WorkerGlobalScope::EvaluateClassicScript(
bool success = ScriptController()->Evaluate(
ScriptSourceCode(source_code, ScriptSourceLocationType::kUnknown, handler,
script_url),
nullptr /* error_event */, v8_cache_options_);
access_control_status, nullptr /* error_event */, v8_cache_options_);
ReportingProxy().DidEvaluateClassicScript(success);
}
......
......@@ -138,6 +138,7 @@ class CORE_EXPORT WorkerGlobalScope
// so that WorkerGlobalScope can be paused.
void EvaluateClassicScriptPausable(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data,
const v8_inspector::V8StackTraceId& stack_id);
......@@ -177,6 +178,7 @@ class CORE_EXPORT WorkerGlobalScope
// Evaluates the given top-level classic script.
virtual void EvaluateClassicScript(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data);
......
......@@ -167,6 +167,7 @@ void WorkerThread::Start(
void WorkerThread::EvaluateClassicScript(
const KURL& script_url,
AccessControlStatus access_control_status,
const String& source_code,
std::unique_ptr<Vector<char>> cached_meta_data,
const v8_inspector::V8StackTraceId& stack_id) {
......@@ -174,7 +175,8 @@ void WorkerThread::EvaluateClassicScript(
PostCrossThreadTask(
*GetTaskRunner(TaskType::kInternalWorker), FROM_HERE,
CrossThreadBind(&WorkerThread::EvaluateClassicScriptOnWorkerThread,
CrossThreadUnretained(this), script_url, source_code,
CrossThreadUnretained(this), script_url,
access_control_status, source_code,
WTF::Passed(std::move(cached_meta_data)), stack_id));
}
......@@ -496,11 +498,13 @@ void WorkerThread::InitializeOnWorkerThread(
void WorkerThread::EvaluateClassicScriptOnWorkerThread(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data,
const v8_inspector::V8StackTraceId& stack_id) {
ToWorkerGlobalScope(GlobalScope())
->EvaluateClassicScriptPausable(script_url, std::move(source_code),
->EvaluateClassicScriptPausable(script_url, access_control_status,
std::move(source_code),
std::move(cached_meta_data), stack_id);
}
......
......@@ -43,6 +43,7 @@
#include "third_party/blink/renderer/core/workers/parent_execution_context_task_runners.h"
#include "third_party/blink/renderer/core/workers/worker_backing_thread_startup_data.h"
#include "third_party/blink/renderer/core/workers/worker_inspector_proxy.h"
#include "third_party/blink/renderer/platform/loader/fetch/access_control_status.h"
#include "third_party/blink/renderer/platform/scheduler/public/worker_scheduler.h"
#include "third_party/blink/renderer/platform/web_task_runner.h"
#include "third_party/blink/renderer/platform/wtf/allocator.h"
......@@ -106,6 +107,7 @@ class CORE_EXPORT WorkerThread : public WebThread::TaskObserver {
// Posts a task to evaluate a top-level classic script on the worker thread.
// Called on the main thread after Start().
void EvaluateClassicScript(const KURL& script_url,
AccessControlStatus access_control_status,
const String& source_code,
std::unique_ptr<Vector<char>> cached_meta_data,
const v8_inspector::V8StackTraceId& stack_id);
......@@ -281,6 +283,7 @@ class CORE_EXPORT WorkerThread : public WebThread::TaskObserver {
void EvaluateClassicScriptOnWorkerThread(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data,
const v8_inspector::V8StackTraceId& stack_id);
......
......@@ -27,6 +27,7 @@
#include "third_party/blink/renderer/core/workers/worker_thread.h"
#include "third_party/blink/renderer/platform/cross_thread_functional.h"
#include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/loader/fetch/access_control_status.h"
#include "third_party/blink/renderer/platform/network/content_security_policy_parsers.h"
#include "third_party/blink/renderer/platform/waitable_event.h"
#include "third_party/blink/renderer/platform/web_thread_supporting_gc.h"
......@@ -103,7 +104,8 @@ class WorkerThreadForTest : public WorkerThread {
WorkerBackingThreadStartupData::CreateDefault(),
WorkerInspectorProxy::PauseOnWorkerStart::kDontPause,
parent_execution_context_task_runners);
EvaluateClassicScript(script_url, source, nullptr /* cached_meta_data */,
EvaluateClassicScript(script_url, kOpaqueResource, source,
nullptr /* cached_meta_data */,
v8_inspector::V8StackTraceId());
}
......
......@@ -262,13 +262,14 @@ class AnimationWorkletGlobalScopeTest : public PageTestBase {
ScriptState::Scope scope(script_state);
global_scope->ScriptController()->Evaluate(ScriptSourceCode(
R"JS(
R"JS(
registerAnimator('test', class {
animate (currentTime, effect) {
effect.localTime = 123;
}
});
)JS"));
)JS"),
kSharableCrossOrigin);
// Passing a new input state with a new animation id should cause the
// worklet to create and animate an animator.
......@@ -307,13 +308,14 @@ class AnimationWorkletGlobalScopeTest : public PageTestBase {
ScriptState::Scope scope(script_state);
global_scope->ScriptController()->Evaluate(ScriptSourceCode(
R"JS(
R"JS(
registerAnimator('test', class {
animate (currentTime, effect) {
effect.localTime = 123;
}
});
)JS"));
)JS"),
kSharableCrossOrigin);
cc::WorkletAnimationId animation_id = {1, 1};
AnimationWorkletInput state;
......@@ -355,13 +357,14 @@ class AnimationWorkletGlobalScopeTest : public PageTestBase {
ScriptState::Scope scope(script_state);
global_scope->ScriptController()->Evaluate(ScriptSourceCode(
R"JS(
R"JS(
registerAnimator('test', class {
animate (currentTime, effect) {
effect.localTime = 123;
}
});
)JS"));
)JS"),
kSharableCrossOrigin);
cc::WorkletAnimationId animation_id = {1, 1};
AnimationWorkletInput state;
......
......@@ -106,7 +106,8 @@ class PaintWorkletTest : public PageTestBase {
TEST_F(PaintWorkletTest, GarbageCollectionOfCSSPaintDefinition) {
PaintWorkletGlobalScope* global_scope = GetProxy()->global_scope();
global_scope->ScriptController()->Evaluate(
ScriptSourceCode("registerPaint('foo', class { paint() { } });"));
ScriptSourceCode("registerPaint('foo', class { paint() { } });"),
kSharableCrossOrigin);
CSSPaintDefinition* definition = global_scope->FindDefinition("foo");
DCHECK(definition);
......@@ -147,7 +148,8 @@ TEST_F(PaintWorkletTest, GarbageCollectionOfCSSPaintDefinition) {
TEST_F(PaintWorkletTest, PaintWithNullPaintArguments) {
PaintWorkletGlobalScope* global_scope = GetProxy()->global_scope();
global_scope->ScriptController()->Evaluate(
ScriptSourceCode("registerPaint('foo', class { paint() { } });"));
ScriptSourceCode("registerPaint('foo', class { paint() { } });"),
kSharableCrossOrigin);
CSSPaintDefinition* definition = global_scope->FindDefinition("foo");
ASSERT_TRUE(definition);
......@@ -168,7 +170,8 @@ TEST_F(PaintWorkletTest, PaintWithNullPaintArguments) {
TEST_F(PaintWorkletTest, SinglyRegisteredDocumentDefinitionNotUsed) {
PaintWorkletGlobalScope* global_scope = GetProxy()->global_scope();
global_scope->ScriptController()->Evaluate(
ScriptSourceCode("registerPaint('foo', class { paint() { } });"));
ScriptSourceCode("registerPaint('foo', class { paint() { } });"),
kSharableCrossOrigin);
CSSPaintImageGeneratorImpl* generator =
static_cast<CSSPaintImageGeneratorImpl*>(
......
......@@ -469,9 +469,10 @@ void WebEmbeddedWorkerImpl::StartWorkerThread() {
// > "classic": Fetch a classic worker script given job’s serialized script
// > url, job’s client, "serviceworker", and the to-be-created environment
// > settings object for this service worker.
// Service worker is origin-bound, so use kSharableCrossOrigin.
worker_thread_->EvaluateClassicScript(
worker_start_data_.script_url, source_code, std::move(cached_meta_data),
v8_inspector::V8StackTraceId());
worker_start_data_.script_url, kSharableCrossOrigin, source_code,
std::move(cached_meta_data), v8_inspector::V8StackTraceId());
} else {
// > "module": Fetch a module worker script graph given job’s serialized
// > script url, job’s client, "serviceworker", "omit", and the
......
......@@ -116,6 +116,7 @@ void ServiceWorkerGlobalScope::ReadyToEvaluateScript() {
void ServiceWorkerGlobalScope::EvaluateClassicScript(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data) {
DCHECK(IsContextThread());
......@@ -123,8 +124,8 @@ void ServiceWorkerGlobalScope::EvaluateClassicScript(
if (!evaluate_script_ready_) {
evaluate_script_ =
WTF::Bind(&ServiceWorkerGlobalScope::EvaluateClassicScript,
WrapWeakPersistent(this), script_url, std::move(source_code),
std::move(cached_meta_data));
WrapWeakPersistent(this), script_url, access_control_status,
std::move(source_code), std::move(cached_meta_data));
return;
}
......@@ -164,7 +165,8 @@ void ServiceWorkerGlobalScope::EvaluateClassicScript(
ReportingProxy().DidLoadInstalledScript();
}
WorkerGlobalScope::EvaluateClassicScript(script_url, source_code,
WorkerGlobalScope::EvaluateClassicScript(script_url, access_control_status,
source_code,
std::move(cached_meta_data));
}
......
......@@ -71,6 +71,7 @@ class MODULES_EXPORT ServiceWorkerGlobalScope final : public WorkerGlobalScope {
// Implements WorkerGlobalScope.
void EvaluateClassicScript(
const KURL& script_url,
AccessControlStatus access_control_status,
String source_code,
std::unique_ptr<Vector<char>> cached_meta_data) override;
void ImportModuleScript(
......
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