-
Notifications
You must be signed in to change notification settings - Fork 15k
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: nativeImage remote serialization #23543
Conversation
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.
Looks good but needs a test!
lib/browser/remote/server.ts
Outdated
@@ -259,6 +262,8 @@ const fakeConstructor = (constructor: Function, name: string) => | |||
const unwrapArgs = function (sender: electron.WebContents, frameId: number, contextId: string, args: any[]) { | |||
const metaToValue = function (meta: MetaTypeFromRenderer): any { | |||
switch (meta.type) { | |||
case 'nativeimage': | |||
return electron.nativeImage.createFromBitmap(meta.value.buffer, meta.value.size as any); |
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.
The scaleFactor
option should also be passed, and we should add a way to pass the isTemplateImage
information.
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.
If we're going to do a different serialization of native images here we should also update type-utils
.
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.
@nornagon i was planning to handle that in a subsequent pr to prevent too much scope creep 🤔 i can do it here if preferable though.
f073717
to
500160a
Compare
dc3795f
to
136177b
Compare
136177b
to
d5b088b
Compare
d5b088b
to
17ebafa
Compare
ae666e8
to
7d5e375
Compare
I was unable to backport this PR to "8-x-y" cleanly; |
I have automatically backported this PR to "9-x-y", please check out #23635 |
We weren't serializing nativeImages properly in the remote module, leading to gin conversion errors when trying to, for example, create a new context menu in the renderer with icons using nativeImage. This fixes that by adding a new special case to handle them.
@codebytere has manually backported this PR to "9-x-y", please check out #23796 |
We weren't serializing nativeImages properly in the remote module, leading to gin conversion errors when trying to, for example, create a new context menu in the renderer with icons using nativeImage. This fixes that by adding a new special case to handle them.
@codebytere has manually backported this PR to "8-x-y", please check out #23797 |
* fix: nativeImage remote serialization (#23543) We weren't serializing nativeImages properly in the remote module, leading to gin conversion errors when trying to, for example, create a new context menu in the renderer with icons using nativeImage. This fixes that by adding a new special case to handle them. * refactor: correctly serialize nativeImage/buffer with typeUtils (#23666) * refactor: correctly serialize nativeImage/buffer with typeUtils * test: add serialization specs * fix: construct from dataURL * test: test for dataURL specificity * refactor: use typeutils for nativeImage serialization * fix: ensure nativeImage serialization main->renderer * chore: fix FTBFS Co-authored-by: Charles Kerr <charles@charleskerr.com>
Description of Change
Closes #23444.
We weren't serializing
nativeImages
properly in the remote module, leading to gin conversion errors when trying to, for example, create a new context menu in the renderer with icons usingnativeImage
. This fixes that by adding a new special case to handle them.Tested with https://gist.github.com/d8f1bcee2aa65c85b9cf3c8e4d34ad82.
cc @nornagon @MarshallOfSound
Checklist
npm test
passesRelease Notes
Notes: Fixed an issue where
nativeImage
s might throw conversion errors in the renderer process.