New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure standard schemes are registered in nw service process #22867
Conversation
dd6c8f7
to
ff670d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get a test?
@nornagon thoughts on how to test this ? when this crash happens it doesn't seem to affect the resource loading on those custom schemes, so thats why existing test suites where passing. Only the service process goes on a loop restarting. |
Maybe I'm confused then... how is this change visible to the user? |
Just infinite logs from network service
The registered scheme itself works fine, files are served over the protocol, renderer window.location object parses the host and other properties. And just observed that shutdown of app isn't graceful. Let me see if I can write a test that verifies app shutdown when using this api. |
ff670d1
to
cb91e5c
Compare
@nornagon added simple test, PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems weird to me that the network service would function fine but continually crash..? but OK.
Co-Authored-By: Jeremy Apthorp <jeremya@chromium.org>
4209b9a
to
de656d6
Compare
Failing test is unrelated, merging. |
Release Notes Persisted
|
I have automatically backported this PR to "9-x-y", please check out #22917 |
…ectron#22867) * fix: ensure standard schemes are registered in nw service process Refs electron#20546 * chore: add test * chore: apply suggestions from code review Co-Authored-By: Jeremy Apthorp <jeremya@chromium.org> Co-authored-by: Jeremy Apthorp <jeremya@chromium.org>
Description of Change
Crash in Network service process due to #20546 regressed after f5b9e49 .
This PR fixes by making a clear indication of how the schemes are registered in respective Browser, Renderer and Network process.
Identified while testing #20625 but ci logs in master also show the crash.
Checklist
npm test
passesRelease Notes
Notes: fix crash in network service process when using protocol.registerSchemeAsPrivileged api