fix: loading dedicated/shared worker scripts over custom protocol (#2…

…4750) * fix: add patch to avoid crash in worker with nodeintegration enabled [worker_global_scope.cc(111)] Check failed: url_.IsValid(). * update patches * ProtocolRegistry does not exist yet * use custom partition for sw tests Co-authored-by: deepak1556 <hop2deep@gmail.com> Co-authored-by: Electron Bot <anonymous@electronjs.org> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
electron · Aug 17, 2020 · e2c4e34 · e2c4e34
1 parent 79f6ba4
commit e2c4e34
Show file tree

Hide file tree

Showing 9 changed files with 141 additions and 5 deletions.
diff --git a/patches/chromium/.patches b/patches/chromium/.patches
@@ -117,3 +117,4 @@ backport_1074340.patch
 backport_1016278.patch
 backport_1042986.patch
 a11y_axplatformnodebase_getindexinparent_returns_base_optional.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
@@ -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 a94b2b289eb1b5da820dd3bf620911f2edef7c68..e11d29b43553574425c524e722f31bb8c2996fcb 100644
+--- a/content/public/renderer/content_renderer_client.h
++++ b/content/public/renderer/content_renderer_client.h
+@@ -385,6 +385,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 a2fe4cc09ffdcdf93199c01c316a4173a6f84d32..f8360eb68629a5d6b56471ef714312e1c5826025 100644
+--- a/content/renderer/renderer_blink_platform_impl.cc
++++ b/content/renderer/renderer_blink_platform_impl.cc
+@@ -876,6 +876,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 236894dc0dd7242c1f47caeb982c4f24b784ce95..35f229faa607bc3f06c524619e385d9aa356e2f1 100644
+--- a/content/renderer/renderer_blink_platform_impl.h
++++ b/content/renderer/renderer_blink_platform_impl.h
+@@ -182,6 +182,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 3edcb4715c2dd8b2cf33f093710f14816b061117..79336bcad4d18e617677b2fb80d898680618e03b 100644
+--- a/third_party/blink/public/platform/platform.h
++++ b/third_party/blink/public/platform/platform.h
+@@ -615,6 +615,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 83db0d2cba80184ecea9314b5869f836c0e3ea20..a25e2631bfd23bf1c212943d05b5f93a1c0201ed 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
+@@ -323,6 +323,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(
diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc
@@ -1286,6 +1286,18 @@ void ElectronBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
   }
 }
 
+void ElectronBrowserClient::
+    RegisterNonNetworkWorkerMainResourceURLLoaderFactories(
+        content::BrowserContext* browser_context,
+        NonNetworkURLLoaderFactoryMap* factories) {
+  api::Protocol* protocol = api::Protocol::FromWrappedClass(
+      v8::Isolate::GetCurrent(), browser_context);
+  if (protocol) {
+    protocol->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
@@ -175,6 +175,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
@@ -200,11 +200,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);
   }
 }
 

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

diff --git 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<v8::Context> worker_context) {
+void WebWorkerObserver::WorkerScriptReadyForEvaluation(
+    v8::Local<v8::Context> 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
@@ -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:

diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts
@@ -389,6 +389,33 @@ 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,
+          partition: 'sw-file-scheme-worker-spec'
+        }
+      });
+
+      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!');
+          session.fromPartition('sw-file-scheme-worker-spec').clearStorageData({
+            storages: ['serviceworkers']
+          }).then(() => done());
+        }
+      });
+
+      w.webContents.on('crashed', () => done(new Error('WebContents crashed.')));
+      w.loadFile(path.join(fixturesPath, 'pages', 'service-worker', 'index.html'));
+    });
   });
 
   describe('navigator.geolocation', () => {