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 serialization of <webview>.capturePage() result #20825
Conversation
0388424
to
872ead0
Compare
@nornagon, @MarshallOfSound can you please review? |
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.
Let's apply the serialize
/ deserialize
operation only where it's needed, i.e. this one capturePage
operation. Otherwise we add a bunch of extra useless CPU work for no reason.
872ead0
to
1dfe5ee
Compare
@nornagon done, I've created a separate IPC for |
f8799ee
to
e6a1912
Compare
e6a1912
to
526485a
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.
isPromise
looks weird though.
@alexeykuzmin i agree, but it was there before, so 🤷♂ i guess? |
@nornagon yeah, I was going to write a comment about it but then checked contents of the removed file =) The PR looks good otherwise. |
The win-x64-testing failure is not related to the change. Merging. |
Description of Change
Fixes #8151
Fixes #16241
Checklist
npm test
passesRelease Notes
Notes: Fixed
<webview>.capturePage()
resolving with an empty object instead ofNativeImage
instance.