fix: loading dedicated/shared worker scripts over custom protocol (#20625) (#26142)

Co-authored-by: Robo <hop2deep@gmail.com>
This commit is contained in:
Milan Burda
2020-10-29 03:11:33 +01:00
committed by GitHub
parent 2f313e0aea
commit 86e4cced1f
9 changed files with 137 additions and 5 deletions

View File

@@ -138,3 +138,4 @@ cherry-pick-3b5f65c0aeca.patch
cherry-pick-1ed869ad4bb3.patch
cherry-pick-229fdaf8fc05.patch
cherry-pick-88f263f401b4.patch
worker_feat_add_hook_to_notify_script_ready.patch

View File

@@ -0,0 +1,92 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: deepak1556 <hop2deep@gmail.com>
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 2c372d68dace9c546eae3bdbd4994ec261a6fa22..55276754d4ec6d8a9644410eccfbafe372782f38 100644
--- a/content/public/renderer/content_renderer_client.h
+++ b/content/public/renderer/content_renderer_client.h
@@ -396,6 +396,11 @@ class CONTENT_EXPORT ContentRendererClient {
virtual void DidInitializeWorkerContextOnWorkerThread(
v8::Local<v8::Context> 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<v8::Context> 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 a1fd4ca3d56476246c1a560bfbb7299f1134abf3..fc450854c22c9477cf02c7ca89391ea0e87685b4 100644
--- a/content/renderer/renderer_blink_platform_impl.cc
+++ b/content/renderer/renderer_blink_platform_impl.cc
@@ -895,6 +895,12 @@ void RendererBlinkPlatformImpl::WorkerContextCreated(
worker);
}
+void RendererBlinkPlatformImpl::WorkerScriptReadyForEvaluation(
+ const v8::Local<v8::Context>& worker) {
+ GetContentClient()->renderer()->WorkerScriptReadyForEvaluationOnWorkerThread(
+ worker);
+}
+
bool RendererBlinkPlatformImpl::IsExcludedHeaderForServiceWorkerFetchEvent(
const blink::WebString& header_name) {
return GetContentClient()
diff --git a/content/renderer/renderer_blink_platform_impl.h b/content/renderer/renderer_blink_platform_impl.h
index 1995663c3ee97c51a81de076c9a7fe05ba0e73fc..373e7c0687cf7b932daf29361f7fc5ea50ff236d 100644
--- a/content/renderer/renderer_blink_platform_impl.h
+++ b/content/renderer/renderer_blink_platform_impl.h
@@ -181,6 +181,8 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl {
void DidStartWorkerThread() override;
void WillStopWorkerThread() override;
void WorkerContextCreated(const v8::Local<v8::Context>& worker) override;
+ void WorkerScriptReadyForEvaluation(
+ const v8::Local<v8::Context>& worker) override;
void WorkerContextWillDestroy(const v8::Local<v8::Context>& worker) override;
bool IsExcludedHeaderForServiceWorkerFetchEvent(
const blink::WebString& header_name) override;
diff --git a/third_party/blink/public/platform/platform.h b/third_party/blink/public/platform/platform.h
index e9f082fbe34022b165aeca1a37fc0f0fe5e6024a..9fa24291e3c7cdc6de3ab75a8f80b0a8a1c05338 100644
--- a/third_party/blink/public/platform/platform.h
+++ b/third_party/blink/public/platform/platform.h
@@ -623,6 +623,8 @@ class BLINK_PLATFORM_EXPORT Platform {
virtual void DidStartWorkerThread() {}
virtual void WillStopWorkerThread() {}
virtual void WorkerContextCreated(const v8::Local<v8::Context>& worker) {}
+ virtual void WorkerScriptReadyForEvaluation(
+ const v8::Local<v8::Context>& worker) {}
virtual void WorkerContextWillDestroy(const v8::Local<v8::Context>& 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 1463b2baaf931222a2c3e5819089ac1d93197311..398982014b9a9131d88d29eaa12acd6a94aa285b 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
@@ -320,6 +320,8 @@ void WorkerOrWorkletScriptController::PrepareForEvaluation() {
wrapper_type_info->InstallConditionalFeatures(
context, *world_, global_object, v8::Local<v8::Object>(),
v8::Local<v8::Function>(), global_interface_template);
+
+ Platform::Current()->WorkerScriptReadyForEvaluation(context);
}
void WorkerOrWorkletScriptController::DisableEvalInternal(

View File

@@ -982,6 +982,17 @@ void ElectronBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
protocol->RegisterURLLoaderFactories(factories);
}
void ElectronBrowserClient::
RegisterNonNetworkWorkerMainResourceURLLoaderFactories(
content::BrowserContext* browser_context,
NonNetworkURLLoaderFactoryMap* factories) {
api::ProtocolNS* protocol = api::ProtocolNS::FromWrappedClass(
v8::Isolate::GetCurrent(), browser_context);
if (protocol) {
protocol->RegisterURLLoaderFactories(factories);
}
}
void ElectronBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories(
int render_process_id,
int render_frame_id,

View File

@@ -170,6 +170,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,

View File

@@ -198,11 +198,11 @@ bool ElectronRendererClient::ShouldFork(blink::WebLocalFrame* frame,
return http_method == "GET";
}
void ElectronRendererClient::DidInitializeWorkerContextOnWorkerThread(
void ElectronRendererClient::WorkerScriptReadyForEvaluationOnWorkerThread(
v8::Local<v8::Context> context) {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kNodeIntegrationInWorker)) {
WebWorkerObserver::GetCurrent()->ContextCreated(context);
WebWorkerObserver::GetCurrent()->WorkerScriptReadyForEvaluation(context);
}
}

View File

@@ -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<v8::Context> context) override;
void WillDestroyWorkerContextOnWorkerThread(
v8::Local<v8::Context> context) override;

View File

@@ -41,7 +41,8 @@ WebWorkerObserver::~WebWorkerObserver() {
asar::ClearArchives();
}
void WebWorkerObserver::ContextCreated(v8::Local<v8::Context> worker_context) {
void WebWorkerObserver::WorkerScriptReadyForEvaluation(
v8::Local<v8::Context> worker_context) {
v8::Context::Scope context_scope(worker_context);
// Start the embed thread.

View File

@@ -21,7 +21,7 @@ class WebWorkerObserver {
// Returns the WebWorkerObserver for current worker thread.
static WebWorkerObserver* GetCurrent();
void ContextCreated(v8::Local<v8::Context> context);
void WorkerScriptReadyForEvaluation(v8::Local<v8::Context> context);
void ContextWillDestroy(v8::Local<v8::Context> context);
private:

View File

@@ -430,6 +430,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', () => {