diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 07f76ac2a986..78f51772d41d 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -103,3 +103,4 @@ fix_check_issecureeventinputenabled_in_constructor_before_setting.patch skip_atk_toolchain_check.patch fix_missing_weakptr_check_in_preconnectmanager.patch revert_swiftshader_roll_in_deps.patch +worker_feat_add_hook_to_notify_script_ready.patch diff --git a/patches/chromium/worker_feat_add_hook_to_notify_script_ready.patch b/patches/chromium/worker_feat_add_hook_to_notify_script_ready.patch new file mode 100644 index 000000000000..a65cf58d1217 --- /dev/null +++ b/patches/chromium/worker_feat_add_hook_to_notify_script_ready.patch @@ -0,0 +1,92 @@ +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 8309bb7f3f284b0930c13a7e75a7f9de64fedb73..7c5c925cadc5caca5ef69eda80ccc04797b2c803 100644 +--- a/content/public/renderer/content_renderer_client.h ++++ b/content/public/renderer/content_renderer_client.h +@@ -405,6 +405,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 906e86480872394d33608a954440cbc92273d0ff..27bf2e12e5f2a150fca992f1bfed88936c7cee35 100644 +--- a/content/renderer/renderer_blink_platform_impl.cc ++++ b/content/renderer/renderer_blink_platform_impl.cc +@@ -877,6 +877,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 db58cb0be064b7944b17dd12f1d1e6c12cecfff7..5c80537e6020ae5845728130b46825f5db745a5e 100644 +--- a/content/renderer/renderer_blink_platform_impl.h ++++ b/content/renderer/renderer_blink_platform_impl.h +@@ -191,6 +191,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 754634fd4935cdf38168361f1491cf3aedaf8cdb..513cdea72aba0082d43268bdbfc5328fb21afb76 100644 +--- a/third_party/blink/public/platform/platform.h ++++ b/third_party/blink/public/platform/platform.h +@@ -638,6 +638,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 72b80a13cc7845236b58a4944be0ac8b314dc5fe..c227343c1c58acf94c7be2a9a2a12e921c1b5943 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 +@@ -328,6 +328,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 + } + diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index 6af3076eff42..25e99cf0516c 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -1263,6 +1263,16 @@ void ElectronBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories( URLLoaderFactoryType::kNavigation, factories); } +void ElectronBrowserClient:: + RegisterNonNetworkWorkerMainResourceURLLoaderFactories( + content::BrowserContext* browser_context, + NonNetworkURLLoaderFactoryMap* factories) { + auto* protocol_registry = + ProtocolRegistry::FromBrowserContext(browser_context); + protocol_registry->RegisterURLLoaderFactories( + URLLoaderFactoryType::kWorkerMainResource, factories); +} + #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS) namespace { diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h index 6968e3d86505..7a4af135b55f 100644 --- a/shell/browser/electron_browser_client.h +++ b/shell/browser/electron_browser_client.h @@ -178,6 +178,9 @@ class ElectronBrowserClient : public content::ContentBrowserClient, void RegisterNonNetworkNavigationURLLoaderFactories( int frame_tree_node_id, NonNetworkURLLoaderFactoryMap* factories) override; + void RegisterNonNetworkWorkerMainResourceURLLoaderFactories( + content::BrowserContext* browser_context, + NonNetworkURLLoaderFactoryMap* factories) override; void RegisterNonNetworkSubresourceURLLoaderFactories( int render_process_id, int render_frame_id, diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index 3901eb23bad3..464eae134b5f 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -200,11 +200,11 @@ bool ElectronRendererClient::ShouldFork(blink::WebLocalFrame* frame, return http_method == "GET"; } -void ElectronRendererClient::DidInitializeWorkerContextOnWorkerThread( +void ElectronRendererClient::WorkerScriptReadyForEvaluationOnWorkerThread( v8::Local context) { if (base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kNodeIntegrationInWorker)) { - WebWorkerObserver::GetCurrent()->ContextCreated(context); + WebWorkerObserver::GetCurrent()->WorkerScriptReadyForEvaluation(context); } } diff --git a/shell/renderer/electron_renderer_client.h b/shell/renderer/electron_renderer_client.h index 00acca0e4457..d3741d4470af 100644 --- a/shell/renderer/electron_renderer_client.h +++ b/shell/renderer/electron_renderer_client.h @@ -47,7 +47,7 @@ class ElectronRendererClient : public RendererClientBase { const std::string& http_method, bool is_initial_navigation, bool is_server_redirect) override; - void DidInitializeWorkerContextOnWorkerThread( + void WorkerScriptReadyForEvaluationOnWorkerThread( v8::Local context) override; void WillDestroyWorkerContextOnWorkerThread( v8::Local context) override; diff --git a/shell/renderer/web_worker_observer.cc b/shell/renderer/web_worker_observer.cc index a52d3305802f..79b05eb1976c 100644 --- a/shell/renderer/web_worker_observer.cc +++ b/shell/renderer/web_worker_observer.cc @@ -42,7 +42,8 @@ WebWorkerObserver::~WebWorkerObserver() { asar::ClearArchives(); } -void WebWorkerObserver::ContextCreated(v8::Local worker_context) { +void WebWorkerObserver::WorkerScriptReadyForEvaluation( + v8::Local worker_context) { v8::Context::Scope context_scope(worker_context); // Start the embed thread. diff --git a/shell/renderer/web_worker_observer.h b/shell/renderer/web_worker_observer.h index 4054efce5159..190efd569862 100644 --- a/shell/renderer/web_worker_observer.h +++ b/shell/renderer/web_worker_observer.h @@ -21,7 +21,7 @@ class WebWorkerObserver { // Returns the WebWorkerObserver for current worker thread. static WebWorkerObserver* GetCurrent(); - void ContextCreated(v8::Local context); + void WorkerScriptReadyForEvaluation(v8::Local context); void ContextWillDestroy(v8::Local context); private: diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index a5da960e282b..4713980e406b 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -441,6 +441,30 @@ describe('chromium features', () => { w.webContents.on('crashed', () => done(new Error('WebContents crashed.'))); w.loadFile(path.join(fixturesPath, 'pages', 'service-worker', 'index.html')); }); + + it('should not crash when nodeIntegration is enabled', (done) => { + const w = new BrowserWindow({ + show: false, + webPreferences: { + nodeIntegration: true, + nodeIntegrationInWorker: true + } + }); + + w.webContents.on('ipc-message', (event, channel, message) => { + if (channel === 'reload') { + w.webContents.reload(); + } else if (channel === 'error') { + done(`unexpected error : ${message}`); + } else if (channel === 'response') { + expect(message).to.equal('Hello from serviceWorker!'); + done(); + } + }); + + w.webContents.on('crashed', () => done(new Error('WebContents crashed.'))); + w.loadFile(path.join(fixturesPath, 'pages', 'service-worker', 'index.html')); + }); }); describe('navigator.geolocation', () => {