Worker: Remove unnecessary error handling in WorkerOrWorkletScriptController::Initialize()
This is a follow-up for comments in the previous code review:
https://chromium-review.googlesource.com/c/chromium/src/+/1614645/6/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.cc#157
Error cases handled in WorkerOrWorkletScriptController::Initialize() never
happen according to the comments. This CL removes the cases.
Bug: n/a
Change-Id: I44ab6db7213cecb5cd6e31a66b70bb53b3829560
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1621791
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661659}
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 26a0f26..7f25150 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
@@ -142,7 +142,7 @@
script_state_->DissociateContext();
}
-bool WorkerOrWorkletScriptController::Initialize(const KURL& url_for_debugger) {
+void WorkerOrWorkletScriptController::Initialize(const KURL& url_for_debugger) {
v8::HandleScope handle_scope(isolate_);
DCHECK(!IsContextInitialized());
@@ -154,8 +154,7 @@
script_wrappable->GetWrapperTypeInfo();
v8::Local<v8::FunctionTemplate> global_interface_template =
wrapper_type_info->DomTemplate(isolate_, *world_);
- if (global_interface_template.IsEmpty())
- return false;
+ DCHECK(!global_interface_template.IsEmpty());
v8::Local<v8::ObjectTemplate> global_template =
global_interface_template->InstanceTemplate();
v8::Local<v8::Context> context;
@@ -174,8 +173,7 @@
v8::DeserializeInternalFieldsCallback(),
agent->event_loop()->microtask_queue());
}
- if (context.IsEmpty())
- return false;
+ DCHECK(!context.IsEmpty());
script_state_ = MakeGarbageCollected<ScriptState>(context, world_);
@@ -269,8 +267,6 @@
// call this here.
PrepareForEvaluation();
}
-
- return true;
}
void WorkerOrWorkletScriptController::PrepareForEvaluation() {
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 fd3f5a3a..b67f3f8d 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
@@ -73,7 +73,7 @@
// For main thread WorkletGlobalScope, WorkerOrWorkletGlobalScope::Name() is
// used for setting DOMWrapperWorld's human readable name.
// This should be called only once.
- bool Initialize(const KURL& url_for_debugger);
+ void Initialize(const KURL& url_for_debugger);
// Prepares for script evaluation. This must be called after Initialize()
// before Evaluate().
diff --git a/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.cc b/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.cc
index 5920b3d..1eb519d 100644
--- a/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.cc
+++ b/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.cc
@@ -32,7 +32,6 @@
auto* global_scope = MakeGarbageCollected<LayoutWorkletGlobalScope>(
frame, std::move(creation_params), reporting_proxy,
pending_layout_registry);
- // TODO(bashi): Handle a case where the script controller fails to initialize.
global_scope->ScriptController()->Initialize(NullURL());
MainThreadDebugger::Instance()->ContextCreated(
global_scope->ScriptController()->GetScriptState(),
diff --git a/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc b/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
index f8e6a18d..9e3ca31 100644
--- a/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
+++ b/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
@@ -194,7 +194,7 @@
MakeGarbageCollected<WorkletModuleResponsesMap>());
global_scope_ = MakeGarbageCollected<WorkletGlobalScope>(
std::move(creation_params), *reporting_proxy_, &GetFrame());
- ASSERT_TRUE(global_scope_->ScriptController()->Initialize(NullURL()));
+ global_scope_->ScriptController()->Initialize(NullURL());
modulator_ = MakeGarbageCollected<ModuleScriptLoaderTestModulator>(
global_scope_->ScriptController()->GetScriptState());
}
diff --git a/third_party/blink/renderer/core/workers/worker_reporting_proxy.h b/third_party/blink/renderer/core/workers/worker_reporting_proxy.h
index dffa510..1fb08610 100644
--- a/third_party/blink/renderer/core/workers/worker_reporting_proxy.h
+++ b/third_party/blink/renderer/core/workers/worker_reporting_proxy.h
@@ -69,13 +69,10 @@
virtual void DidCreateWorkerGlobalScope(WorkerOrWorkletGlobalScope*) {}
// Invoked when the WorkerGlobalScope is initialized on
- // WorkerThread::InitializeOnWorkerThread.
+ // WorkerThread::InitializeOnWorkerThread. This is synchronously called after
+ // WillInitializeWorkerContext().
virtual void DidInitializeWorkerContext() {}
- // Invoked when the WorkerGlobalScope initialization failed on
- // WorkerThread::InitializeOnWorkerThread.
- virtual void DidFailToInitializeWorkerContext() {}
-
// Invoked when the worker's main script is loaded on
// WorkerThread::InitializeOnWorkerThread(). Only invoked when the script was
// loaded on the worker thread, i.e., via InstalledScriptsManager rather than
diff --git a/third_party/blink/renderer/core/workers/worker_thread.cc b/third_party/blink/renderer/core/workers/worker_thread.cc
index 6a2463a..0da8aa29d 100644
--- a/third_party/blink/renderer/core/workers/worker_thread.cc
+++ b/third_party/blink/renderer/core/workers/worker_thread.cc
@@ -543,18 +543,11 @@
WorkerThreadDebugger::From(GetIsolate()))
debugger->WorkerThreadCreated(this);
- if (GlobalScope()->ScriptController()->Initialize(url_for_debugger)) {
- worker_reporting_proxy_.DidInitializeWorkerContext();
- v8::HandleScope handle_scope(GetIsolate());
- Platform::Current()->WorkerContextCreated(
- GlobalScope()->ScriptController()->GetContext());
- } else {
- // TODO(nhiroki): Handle a case where the script controller fails to
- // initialize the context. Specifically, we need to terminate this worker
- // thread from the the parent thread. Currently we only record trace
- // event.
- worker_reporting_proxy_.DidFailToInitializeWorkerContext();
- }
+ GlobalScope()->ScriptController()->Initialize(url_for_debugger);
+ worker_reporting_proxy_.DidInitializeWorkerContext();
+ v8::HandleScope handle_scope(GetIsolate());
+ Platform::Current()->WorkerContextCreated(
+ GlobalScope()->ScriptController()->GetContext());
inspector_task_runner_->InitIsolate(GetIsolate());
SetThreadState(ThreadState::kRunning);
diff --git a/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc b/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc
index 065a744..347872f 100644
--- a/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc
+++ b/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc
@@ -103,7 +103,6 @@
auto* global_scope = MakeGarbageCollected<PaintWorkletGlobalScope>(
frame, std::move(creation_params), reporting_proxy,
pending_generator_registry);
- // TODO(bashi): Handle a case where the script controller fails to initialize.
global_scope->ScriptController()->Initialize(NullURL());
MainThreadDebugger::Instance()->ContextCreated(
global_scope->ScriptController()->GetScriptState(),
diff --git a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.cc b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.cc
index c71a6691..384323d 100644
--- a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.cc
+++ b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.cc
@@ -647,9 +647,8 @@
void ServiceWorkerGlobalScopeProxy::WillInitializeWorkerContext() {
DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
- TRACE_EVENT_ASYNC_BEGIN0(
- "ServiceWorker", "ServiceWorkerGlobalScopeProxy::InitializeWorkerContext",
- this);
+ TRACE_EVENT_BEGIN0("ServiceWorker",
+ "ServiceWorkerGlobalScopeProxy::InitializeWorkerContext");
}
void ServiceWorkerGlobalScopeProxy::DidCreateWorkerGlobalScope(
@@ -674,16 +673,8 @@
WorkerGlobalScope()->ScriptController()->GetScriptState());
Client().DidInitializeWorkerContext(
WorkerGlobalScope()->ScriptController()->GetContext());
- TRACE_EVENT_ASYNC_END1(
- "ServiceWorker", "ServiceWorkerGlobalScopeProxy::InitializeWorkerContext",
- this, "success", true);
-}
-
-void ServiceWorkerGlobalScopeProxy::DidFailToInitializeWorkerContext() {
- DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
- TRACE_EVENT_ASYNC_END1(
- "ServiceWorker", "ServiceWorkerGlobalScopeProxy::InitializeWorkerContext",
- this, "success", false);
+ TRACE_EVENT_END0("ServiceWorker",
+ "ServiceWorkerGlobalScopeProxy::InitializeWorkerContext");
}
void ServiceWorkerGlobalScopeProxy::DidLoadClassicScript() {
diff --git a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.h b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.h
index fa37add7..8079939 100644
--- a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.h
+++ b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope_proxy.h
@@ -165,7 +165,6 @@
void WillInitializeWorkerContext() override;
void DidCreateWorkerGlobalScope(WorkerOrWorkletGlobalScope*) override;
void DidInitializeWorkerContext() override;
- void DidFailToInitializeWorkerContext() override;
void DidLoadClassicScript() override;
void DidFailToLoadClassicScript() override;
void DidFetchScript() override;