Worker: Make sure to install features controlled by origin trial after OriginTrialContext::AddTokens()
JavaScript's properties controlled by origin trials are installed for workers in
WorkerOrWorkletScriptController::Initialize(). This is too early for
shared/service workers with off-the-main-thread worker script fetch. This is
because their origin trial tokens are served via worker script's response
headers, and are applied to OriginTrialContext *after*
WorkerOrWorkletScriptController::Initialize().
This CL separates WorkerOrWorkletScriptController::Initialize() into 2 parts:
Initialize() to create and initialize the global object, and ReadyToEvaluate()
to install the conditional features. Then, this makes sure to call
ReadyToEvaluate() after the tokens are applied.
This change fixes a following test with off-the-main-thread service worker
script fetch:
- virtual/origin-trials-runtimeflags-disabled/http/tests/origin_trials/webexposed/cookiestore-origin-trial-interfaces.html
Change-Id: I024915b470600d6dc4835d7d3aef297da616da3e
Bug: 945215
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1614645
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Auto-Submit: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661245}
diff --git a/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.cc b/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.cc
index 9083897..26a0f26 100644
--- a/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.cc
+++ b/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.cc
@@ -100,7 +100,6 @@
v8::Isolate* isolate)
: global_scope_(global_scope),
isolate_(isolate),
- execution_forbidden_(false),
rejected_promises_(RejectedPromises::Create()),
execution_state_(nullptr) {
DCHECK(isolate);
@@ -220,10 +219,6 @@
V8DOMWrapper::SetNativeInfo(isolate_, global_object, wrapper_type_info,
script_wrappable);
- // All interfaces must be registered to V8PerContextData.
- // So we explicitly call constructorForType for the global object.
- V8PerContextData::From(context)->ConstructorForType(wrapper_type_info);
-
if (global_scope_->IsMainThreadWorkletGlobalScope()) {
// Set the human readable name for the world.
DCHECK(!global_scope_->Name().IsEmpty());
@@ -237,20 +232,91 @@
context);
}
- wrapper_type_info->InstallConditionalFeatures(
- context, *world_, global_object, v8::Local<v8::Object>(),
- v8::Local<v8::Function>(), global_interface_template);
-
if (!disable_eval_pending_.IsEmpty()) {
DisableEvalInternal(disable_eval_pending_);
disable_eval_pending_ = String();
}
+ // This is a workaround for worker with on-the-main-thread script fetch and
+ // worklets.
+ // - For workers with off-the-main-thread worker script fetch,
+ // PrepareForEvaluation() is called in WorkerGlobalScope::Initialize() after
+ // top-level worker script fetch and before script evaluation.
+ // - For workers with on-the-main-thread worker script fetch, it's too early
+ // to call PrepareForEvaluation() in WorkerGlobalScope::Initialize() because
+ // it's called immediately after WorkerGlobalScope's constructor, that is,
+ // before WorkerOrWorkletScriptController::Initialize(). Therefore, we
+ // ignore the first call of PrepareForEvaluation() from
+ // WorkerGlobalScope::Initialize(), and call it here again.
+ // TODO(nhiroki): Remove this workaround once off-the-main-thread worker
+ // script fetch is enabled by default for all worker types.
+ //
+ // - For worklets, there is no appropriate timing to call
+ // PrepareForEvaluation() other than here because worklets have various
+ // initialization sequences depending on thread model (on-main-thread vs.
+ // off-main-thread) and unique script fetch (fetching a top-level script per
+ // addModule() call in JS).
+ // TODO(nhiroki): Unify worklet initialization sequences, and move this to an
+ // appropriate place.
+ if (global_scope_->GetOffMainThreadWorkerScriptFetchOption() ==
+ OffMainThreadWorkerScriptFetchOption::kDisabled ||
+ global_scope_->IsWorkletGlobalScope()) {
+ // This should be called after origin trial tokens are applied for
+ // OriginTrialContext in WorkerGlobalScope::Initialize() to install origin
+ // trial features in JavaScript's global object. Workers with
+ // on-the-main-thread script fetch and worklets apply origin trial tokens
+ // before WorkerOrWorkletScriptController::initialize(), so it's safe to
+ // call this here.
+ PrepareForEvaluation();
+ }
+
+ return true;
+}
+
+void WorkerOrWorkletScriptController::PrepareForEvaluation() {
+ if (!IsContextInitialized()) {
+ // For workers with off-the-main-thread worker script fetch, this can be
+ // called before WorkerOrWorkletScriptController::Initialize() via
+ // WorkerGlobalScope creation function. In this case, PrepareForEvaluation()
+ // calls this function again. See comments in PrepareForEvaluation().
+ DCHECK(global_scope_->IsWorkerGlobalScope());
+ DCHECK_EQ(OffMainThreadWorkerScriptFetchOption::kDisabled,
+ global_scope_->GetOffMainThreadWorkerScriptFetchOption());
+ return;
+ }
+ DCHECK(!is_ready_to_evaluate_);
+ is_ready_to_evaluate_ = true;
+
+ v8::HandleScope handle_scope(isolate_);
+
+ ScriptState::Scope scope(script_state_);
+ v8::Local<v8::Context> context = script_state_->GetContext();
+
+ auto* script_wrappable = static_cast<ScriptWrappable*>(global_scope_);
+ const WrapperTypeInfo* wrapper_type_info =
+ script_wrappable->GetWrapperTypeInfo();
+
+ // All interfaces must be registered to V8PerContextData.
+ // So we explicitly call constructorForType for the global object.
+ // This should be called after OriginTrialContext::AddTokens() in
+ // WorkerGlobalScope::Initialize() to install origin trial features.
+ V8PerContextData::From(context)->ConstructorForType(wrapper_type_info);
+
+ v8::Local<v8::Object> global_object =
+ context->Global()->GetPrototype().As<v8::Object>();
+ DCHECK(!global_object.IsEmpty());
+
+ v8::Local<v8::FunctionTemplate> global_interface_template =
+ wrapper_type_info->DomTemplate(isolate_, *world_);
+ DCHECK(!global_interface_template.IsEmpty());
+
+ wrapper_type_info->InstallConditionalFeatures(
+ context, *world_, global_object, v8::Local<v8::Object>(),
+ v8::Local<v8::Function>(), global_interface_template);
+
// This can only be called after the global object is fully initialised, as it
// reads values from it.
InitializeV8ExtrasBinding(script_state_);
-
- return true;
}
void WorkerOrWorkletScriptController::DisableEvalInternal(
@@ -269,6 +335,7 @@
SanitizeScriptErrors sanitize_script_errors,
V8CacheOptions v8_cache_options) {
DCHECK(IsContextInitialized());
+ DCHECK(is_ready_to_evaluate_);
TRACE_EVENT1("devtools.timeline", "EvaluateScript", "data",
inspector_evaluate_script_event::Data(
diff --git a/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h b/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h
index 03f829b..fd3f5a3a 100644
--- a/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h
+++ b/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h
@@ -75,6 +75,10 @@
// This should be called only once.
bool Initialize(const KURL& url_for_debugger);
+ // Prepares for script evaluation. This must be called after Initialize()
+ // before Evaluate().
+ void PrepareForEvaluation();
+
// Used by WorkerGlobalScope:
void RethrowExceptionFromImportedScript(ErrorEvent*, ExceptionState&);
// Disables `eval()` on JavaScript. This must be called before Evaluate().
@@ -125,7 +129,8 @@
// Keeps the error message for `eval()` on JavaScript until Initialize().
String disable_eval_pending_;
- bool execution_forbidden_;
+ bool is_ready_to_evaluate_ = false;
+ bool execution_forbidden_ = false;
scoped_refptr<RejectedPromises> rejected_promises_;
diff --git a/third_party/blink/renderer/core/workers/dedicated_worker_global_scope.cc b/third_party/blink/renderer/core/workers/dedicated_worker_global_scope.cc
index 7431ad9..1666ed9 100644
--- a/third_party/blink/renderer/core/workers/dedicated_worker_global_scope.cc
+++ b/third_party/blink/renderer/core/workers/dedicated_worker_global_scope.cc
@@ -138,8 +138,11 @@
InitContentSecurityPolicyFromVector(OutsideContentSecurityPolicyHeaders());
BindContentSecurityPolicyToExecutionContext();
+ // This should be called after OriginTrialContext::AddTokens() to install
+ // origin trial features in JavaScript's global object.
// DedicatedWorkerGlobalScope inherits the outside's OriginTrialTokens in the
// constructor instead of the response origin trial tokens.
+ ScriptController()->PrepareForEvaluation();
}
// https://html.spec.whatwg.org/C/#worker-processing-model
diff --git a/third_party/blink/renderer/core/workers/experimental/thread_pool_thread.cc b/third_party/blink/renderer/core/workers/experimental/thread_pool_thread.cc
index 61cb937..a78a82e 100644
--- a/third_party/blink/renderer/core/workers/experimental/thread_pool_thread.cc
+++ b/third_party/blink/renderer/core/workers/experimental/thread_pool_thread.cc
@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/core/workers/experimental/thread_pool_thread.h"
+#include "third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/origin_trials/origin_trial_context.h"
#include "third_party/blink/renderer/core/workers/experimental/task_worklet_global_scope.h"
@@ -52,6 +53,10 @@
BindContentSecurityPolicyToExecutionContext();
OriginTrialContext::AddTokens(this, response_origin_trial_tokens);
+
+ // This should be called after OriginTrialContext::AddTokens() to install
+ // origin trial features in JavaScript's global object.
+ ScriptController()->PrepareForEvaluation();
}
void FetchAndRunClassicScript(
const KURL& script_url,
diff --git a/third_party/blink/renderer/core/workers/shared_worker_global_scope.cc b/third_party/blink/renderer/core/workers/shared_worker_global_scope.cc
index 13acda4..9e47feb 100644
--- a/third_party/blink/renderer/core/workers/shared_worker_global_scope.cc
+++ b/third_party/blink/renderer/core/workers/shared_worker_global_scope.cc
@@ -34,6 +34,7 @@
#include "base/feature_list.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/renderer/bindings/core/v8/source_location.h"
+#include "third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h"
#include "third_party/blink/renderer/core/events/message_event.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
@@ -126,6 +127,10 @@
BindContentSecurityPolicyToExecutionContext();
OriginTrialContext::AddTokens(this, response_origin_trial_tokens);
+
+ // This should be called after OriginTrialContext::AddTokens() to install
+ // origin trial features in JavaScript's global object.
+ ScriptController()->PrepareForEvaluation();
}
// https://html.spec.whatwg.org/C/#worker-processing-model
diff --git a/third_party/blink/renderer/core/workers/worker_global_scope.cc b/third_party/blink/renderer/core/workers/worker_global_scope.cc
index b16111a..ea2197e 100644
--- a/third_party/blink/renderer/core/workers/worker_global_scope.cc
+++ b/third_party/blink/renderer/core/workers/worker_global_scope.cc
@@ -411,6 +411,7 @@
base::TimeTicks time_origin)
: WorkerOrWorkletGlobalScope(
thread->GetIsolate(),
+ creation_params->off_main_thread_fetch_option,
creation_params->global_scope_name,
creation_params->parent_devtools_token,
creation_params->v8_cache_options,
diff --git a/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.cc b/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.cc
index fbff9c2..b5c7400 100644
--- a/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.cc
+++ b/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.cc
@@ -166,6 +166,7 @@
WorkerOrWorkletGlobalScope::WorkerOrWorkletGlobalScope(
v8::Isolate* isolate,
+ OffMainThreadWorkerScriptFetchOption off_main_thread_fetch_option,
const String& name,
const base::UnguessableToken& parent_devtools_token,
V8CacheOptions v8_cache_options,
@@ -173,6 +174,7 @@
scoped_refptr<WebWorkerFetchContext> web_worker_fetch_context,
WorkerReportingProxy& reporting_proxy)
: ExecutionContext(isolate, Agent::CreateForWorkerOrWorklet(isolate)),
+ off_main_thread_fetch_option_(off_main_thread_fetch_option),
name_(name),
parent_devtools_token_(parent_devtools_token),
worker_clients_(worker_clients),
diff --git a/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h b/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h
index c9feb18..f401caf 100644
--- a/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h
+++ b/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h
@@ -17,6 +17,7 @@
#include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
#include "third_party/blink/renderer/core/frame/web_feature_forward.h"
#include "third_party/blink/renderer/core/script/modulator.h"
+#include "third_party/blink/renderer/core/workers/global_scope_creation_params.h"
#include "third_party/blink/renderer/core/workers/worker_clients.h"
#include "third_party/blink/renderer/core/workers/worker_navigator.h"
#include "third_party/blink/renderer/platform/scheduler/public/worker_scheduler.h"
@@ -45,6 +46,7 @@
WorkerOrWorkletGlobalScope(
v8::Isolate*,
+ OffMainThreadWorkerScriptFetchOption,
const String& name,
const base::UnguessableToken& parent_devtools_token,
V8CacheOptions,
@@ -135,6 +137,11 @@
scheduler::WorkerScheduler* GetScheduler() override;
scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner(TaskType) override;
+ OffMainThreadWorkerScriptFetchOption GetOffMainThreadWorkerScriptFetchOption()
+ const {
+ return off_main_thread_fetch_option_;
+ }
+
protected:
// Sets outside's CSP used for off-main-thread top-level worker script
// fetch.
@@ -167,6 +174,7 @@
bool web_fetch_context_initialized_ = false;
+ const OffMainThreadWorkerScriptFetchOption off_main_thread_fetch_option_;
const String name_;
const base::UnguessableToken parent_devtools_token_;
diff --git a/third_party/blink/renderer/core/workers/worker_thread_test_helper.h b/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
index 93afffd..30bf1e2 100644
--- a/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
+++ b/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
@@ -14,6 +14,7 @@
#include "third_party/blink/renderer/bindings/core/v8/source_location.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_cache_options.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_gc_controller.h"
+#include "third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h"
#include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
#include "third_party/blink/renderer/core/frame/settings.h"
#include "third_party/blink/renderer/core/inspector/console_message.h"
@@ -73,6 +74,10 @@
BindContentSecurityPolicyToExecutionContext();
OriginTrialContext::AddTokens(this, response_origin_trial_tokens);
+
+ // This should be called after OriginTrialContext::AddTokens() to install
+ // origin trial features in JavaScript's global object.
+ ScriptController()->PrepareForEvaluation();
}
void FetchAndRunClassicScript(
const KURL& script_url,
diff --git a/third_party/blink/renderer/core/workers/worklet_global_scope.cc b/third_party/blink/renderer/core/workers/worklet_global_scope.cc
index b0cc318..9ce4921 100644
--- a/third_party/blink/renderer/core/workers/worklet_global_scope.cc
+++ b/third_party/blink/renderer/core/workers/worklet_global_scope.cc
@@ -62,6 +62,7 @@
WorkerThread* worker_thread)
: WorkerOrWorkletGlobalScope(
isolate,
+ creation_params->off_main_thread_fetch_option,
creation_params->global_scope_name,
creation_params->parent_devtools_token,
creation_params->v8_cache_options,
diff --git a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
index 71d551c..1d1c826 100644
--- a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
+++ b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
@@ -473,6 +473,10 @@
// TODO(nhiroki): Clarify mappings between the steps 4.8-4.11 and
// implementation.
+
+ // This should be called after OriginTrialContext::AddTokens() to install
+ // origin trial features in JavaScript's global object.
+ ScriptController()->PrepareForEvaluation();
}
// https://w3c.github.io/ServiceWorker/#run-service-worker-algorithm
