fix: loading dedicated/shared worker scripts over custom protocol by deepak1556 · Pull Request #20625 · electron/electron

deepak1556 · 2019-10-18T01:27:29Z

Description of Change

Closes #20557.
Closes #24715.

This is a two part fix,

Part I: Fix crash when loading over data: protocol mentioned in the above issue

This issue will appear only with nodeintegration enabled in the worker. Adds patch worker_feat_add_hook_to_notify_script_ready.patch that explains the root cause of this issue, I am gonna try upstreaming this, will see how it goes. The access to global properties from Node.js happens in node::MaybeInitializeContext which modifies the global object for primordials.

Part II: Cannot load worker scripts over custom protocol

After https://chromium-review.googlesource.com/c/chromium/src/+/1798250 we need to explicitly register custom URLLoaderFactories for dedicated/shared workers.

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
relevant documentation is changed or added
PR title follows semantic commit guidelines
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes:

fix loading shared worker scripts over custom protocol
fix crash when loading worker scripts with nodeIntegration enabled

deepak1556 · 2019-10-18T01:28:09Z

WIP to add some tests.

Dan-Shields · 2020-05-28T22:54:43Z

Wouldn't know where to begin with writing the tests, but would love to see this merged, especially as v6 is now not supported and was the last version where shared workers worked!

krisdages · 2020-06-12T04:39:40Z

I thought this had been ready to go for a while and just needed to be merged... I assumed there were tests in the last commit since it was approved. (Should probably have looked to confirm before writing this comment, but maybe they are already done?)

[worker_global_scope.cc(111)] Check failed: url_.IsValid().

Backports https://chromium-review.googlesource.com/c/chromium/src/+/1798250 that distinguishes loading the main script resource of dedicated/shared worker, this allows us to register a custom URLLoaderFactory.

codebytere

Added a test and updated patches - this should be good to merge. We can add additional tests as a follow-up but for now let's mitigate the crash.

codebytere · 2020-07-27T23:27:40Z

macOS failures are: #24748. Will merge when woa finishes.

trop · 2020-07-28T01:49:06Z

I have automatically backported this PR to "10-x-y", please check out #24749

trop · 2020-07-28T01:49:13Z

I have automatically backported this PR to "9-x-y", please check out #24750

…0625) Co-authored-by: Shelley Vohr <[email protected]>

trop · 2020-10-23T22:42:27Z

@miniak has manually backported this PR to "8-x-y", please check out #26142

…0625)

…0625) (#26142) Co-authored-by: Robo <[email protected]>

deepak1556 added the target/7-0-x label Oct 18, 2019

deepak1556 requested review from a team and zcbenz October 18, 2019 01:27

electron-cation Bot added the new-pr 🌱 PR opened recently label Oct 18, 2019

deepak1556 added the wip ⚒ label Oct 18, 2019

deepak1556 force-pushed the robo/fix_worker_custom_protocol branch from 1e20e47 to 467e29f Compare October 18, 2019 02:21

jkleinsc approved these changes Oct 18, 2019

View reviewed changes

electron-cation Bot removed the new-pr 🌱 PR opened recently label Oct 19, 2019

deepak1556 force-pushed the robo/fix_worker_custom_protocol branch from 467e29f to d5db43a Compare March 27, 2020 10:43

deepak1556 added target/8-x-y and removed wip ⚒ labels Mar 27, 2020

deepak1556 mentioned this pull request Mar 27, 2020

fix: ensure standard schemes are registered in nw service process #22867

Merged

6 tasks

zcbenz approved these changes Mar 30, 2020

View reviewed changes

deepak1556 force-pushed the robo/fix_worker_custom_protocol branch from d5db43a to a8cbcfe Compare April 1, 2020 04:09

deepak1556 added target/10-x-y and removed target/7-3-x labels Jun 18, 2020

deepak1556 mentioned this pull request Jul 27, 2020

fix: service worker crash with Node.js #24736

Closed

6 tasks

deepak1556 and others added 3 commits July 27, 2020 14:36

fix: add patch to avoid crash in worker with nodeintegration enabled

68167b9

[worker_global_scope.cc(111)] Check failed: url_.IsValid().

spec: add crash test for worker with nodeIntegrationInWorker

644c751

codebytere force-pushed the robo/fix_worker_custom_protocol branch from a8cbcfe to 644c751 Compare July 27, 2020 21:36

update patches

14f35c0

codebytere force-pushed the robo/fix_worker_custom_protocol branch from d7b03dc to 14f35c0 Compare July 27, 2020 21:52

codebytere approved these changes Jul 27, 2020

View reviewed changes

MarshallOfSound approved these changes Jul 27, 2020

View reviewed changes

trop Bot added needs-manual-bp/8-x-y and removed target/8-x-y labels Jul 28, 2020

This was referenced Jul 28, 2020

fix: loading dedicated/shared worker scripts over custom protocol #24749

Merged

fix: loading dedicated/shared worker scripts over custom protocol #24750

Merged

trop Bot added in-flight/10-x-y and removed target/10-x-y labels Jul 28, 2020

trop Bot added in-flight/9-x-y and removed target/9-x-y labels Jul 28, 2020

trop Bot added merged/10-x-y and removed in-flight/10-x-y labels Aug 6, 2020

Maxime117 mentioned this pull request Aug 10, 2020

SharedWorker crash on init — Custom Protocol #24917

Closed

trop Bot added merged/9-x-y and removed in-flight/9-x-y labels Aug 17, 2020

codebytere mentioned this pull request Aug 17, 2020

test: use custom partition for sw tests #24999

Merged

6 tasks

pfrazee mentioned this pull request Aug 31, 2020

SharedWorker not working with hyper:// beakerbrowser/beaker#1761

Closed

miniak pushed a commit that referenced this pull request Oct 23, 2020

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

7a80be3

…0625) Co-authored-by: Shelley Vohr <[email protected]>

miniak mentioned this pull request Oct 23, 2020

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

Merged

trop Bot added in-flight/8-x-y and removed needs-manual-bp/8-x-y labels Oct 23, 2020

miniak pushed a commit that referenced this pull request Oct 23, 2020

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

be2d0e6

…0625)

miniak pushed a commit that referenced this pull request Oct 24, 2020

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

cccc9b0

…0625)

miniak pushed a commit that referenced this pull request Oct 28, 2020

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

2ef211c

…0625)

miniak pushed a commit that referenced this pull request Oct 28, 2020

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

70ce0ce

…0625)

trop Bot added merged/8-x-y and removed in-flight/8-x-y labels Oct 29, 2020

deepak1556 added a commit that referenced this pull request Oct 29, 2020

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

86e4cce

…0625) (#26142) Co-authored-by: Robo <[email protected]>

codebytere mentioned this pull request May 23, 2022

feat: note when unmerged PRs are drafts electron/unreleased#66

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix: loading dedicated/shared worker scripts over custom protocol#20625
codebytere merged 4 commits into
masterfrom
robo/fix_worker_custom_protocol

deepak1556 commented Oct 18, 2019 •

edited by codebytere

Loading

Uh oh!

deepak1556 commented Oct 18, 2019

Uh oh!

Dan-Shields commented May 28, 2020

Uh oh!

krisdages commented Jun 12, 2020 •

edited

Loading

Uh oh!

codebytere left a comment •

edited

Loading

Uh oh!

codebytere commented Jul 27, 2020

Uh oh!

trop Bot commented Jul 28, 2020

Uh oh!

trop Bot commented Jul 28, 2020

Uh oh!

trop Bot commented Oct 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

deepak1556 commented Oct 18, 2019 • edited by codebytere Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

Release Notes

Uh oh!

deepak1556 commented Oct 18, 2019

Uh oh!

Dan-Shields commented May 28, 2020

Uh oh!

krisdages commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codebytere left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codebytere commented Jul 27, 2020

Uh oh!

trop Bot commented Jul 28, 2020

Uh oh!

trop Bot commented Jul 28, 2020

Uh oh!

trop Bot commented Oct 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

deepak1556 commented Oct 18, 2019 •

edited by codebytere

Loading

krisdages commented Jun 12, 2020 •

edited

Loading

codebytere left a comment •

edited

Loading