From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 17 Oct 2019 18:00:32 -0700 Subject: feat: add hook to notify script ready from WorkerScriptController In Off-the-main-thread fetch, the WorkerGlobalScope will be in a half initialized state until the script is finished downloading. Doc: https://docs.google.com/document/d/1JCv8TD2nPLNC2iRCp_D1OM4I3uTS0HoEobuTymaMqgw/edit During this stage if the global object is transformed for ex: copying properties in DidInitializeWorkerContextOnWorkerThread hook then an access to property like location will result in a crash WorkerGlobalScope::Url() because the script has not been set with response URL yet. This issue cannot happen in chromium with existing usage, but can surface when an embedder tries to integrate Node.js in the worker. Hence, this new hook is proposed that clearly establishes the worker script is ready for evaluation with the scope initialized. diff --git a/content/public/renderer/content_renderer_client.h b/content/public/renderer/content_renderer_client.h index f501a2ab0bd6d4664dad13913671c4d7d5a96c1e..e633d1755405feeae8c45c0bcbf90f672aacadc5 100644 --- a/content/public/renderer/content_renderer_client.h +++ b/content/public/renderer/content_renderer_client.h @@ -381,6 +381,11 @@ class CONTENT_EXPORT ContentRendererClient { virtual void DidInitializeWorkerContextOnWorkerThread( v8::Local context) {} + // Notifies that a worker script has been downloaded, scope initialized and + // ready for evaluation. This function is called from the worker thread. + virtual void WorkerScriptReadyForEvaluationOnWorkerThread( + v8::Local context) {} + // Notifies that a worker context will be destroyed. This function is called // from the worker thread. virtual void WillDestroyWorkerContextOnWorkerThread( diff --git a/content/renderer/renderer_blink_platform_impl.cc b/content/renderer/renderer_blink_platform_impl.cc index 6f8d94872d0419da5907d4cf3617c9a1582cc11f..9f24498f35719aca325bb7a34c7141408812370e 100644 --- a/content/renderer/renderer_blink_platform_impl.cc +++ b/content/renderer/renderer_blink_platform_impl.cc @@ -910,6 +910,12 @@ void RendererBlinkPlatformImpl::WorkerContextCreated( worker); } +void RendererBlinkPlatformImpl::WorkerScriptReadyForEvaluation( + const v8::Local& worker) { + GetContentClient()->renderer()->WorkerScriptReadyForEvaluationOnWorkerThread( + worker); +} + bool RendererBlinkPlatformImpl::AllowScriptExtensionForServiceWorker( const blink::WebSecurityOrigin& script_origin) { return GetContentClient()->renderer()->AllowScriptExtensionForServiceWorker( diff --git a/content/renderer/renderer_blink_platform_impl.h b/content/renderer/renderer_blink_platform_impl.h index 801eaf387e3af6b800b65a667d356e5b26e57fcf..364b832172f2c0008f5ae62048c74bd39ff4fafc 100644 --- a/content/renderer/renderer_blink_platform_impl.h +++ b/content/renderer/renderer_blink_platform_impl.h @@ -194,6 +194,8 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl { void DidStartWorkerThread() override; void WillStopWorkerThread() override; void WorkerContextCreated(const v8::Local& worker) override; + void WorkerScriptReadyForEvaluation( + const v8::Local& worker) override; void WorkerContextWillDestroy(const v8::Local& worker) override; bool AllowScriptExtensionForServiceWorker( const blink::WebSecurityOrigin& script_origin) override; diff --git a/third_party/blink/public/platform/platform.h b/third_party/blink/public/platform/platform.h index 8dfef4f1d53c61aae04f46040a7cec683534b870..685f38c7604cd0b3393851b4fe74fbd75c4c2672 100644 --- a/third_party/blink/public/platform/platform.h +++ b/third_party/blink/public/platform/platform.h @@ -690,6 +690,8 @@ class BLINK_PLATFORM_EXPORT Platform { virtual void DidStartWorkerThread() {} virtual void WillStopWorkerThread() {} virtual void WorkerContextCreated(const v8::Local& worker) {} + virtual void WorkerScriptReadyForEvaluation( + const v8::Local& worker) {} virtual void WorkerContextWillDestroy(const v8::Local& worker) {} virtual bool AllowScriptExtensionForServiceWorker( const WebSecurityOrigin& script_origin) { 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 a14ccefd7d315bc9757dd995723ec9705ccc2416..c735b617969235ced5a41fc7a1915de0177c71ce 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 @@ -273,6 +273,7 @@ void WorkerOrWorkletScriptController::PrepareForEvaluation() { V8PerContextData* per_context_data = script_state_->PerContextData(); ignore_result(per_context_data->ConstructorForType( global_scope_->GetWrapperTypeInfo())); + Platform::Current()->WorkerScriptReadyForEvaluation(script_state_->GetContext()); #else // USE_BLINK_V8_BINDING_NEW_IDL_INTERFACE ScriptState::Scope scope(script_state_); v8::Local context = script_state_->GetContext(); @@ -298,6 +299,8 @@ void WorkerOrWorkletScriptController::PrepareForEvaluation() { wrapper_type_info->InstallConditionalFeatures( context, *world_, global_object, v8::Local(), v8::Local(), global_interface_template); + + Platform::Current()->WorkerScriptReadyForEvaluation(context); #endif // USE_BLINK_V8_BINDING_NEW_IDL_INTERFACE }