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: promise support with webFrameMain.executeJavaScript #35292
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.
Good find! I looked into this a couple months ago and I think the promise resolution behavior is relatively new, added after webFrameMain was introduced.
3308846
to
4ff0a02
Compare
Changed from Also removed |
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.
API LGTM
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.
API LGTM
I'd argue that is reasonably expected behavior, a promise should never be assumed to never reject 🤷 |
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.
API LGTM
@MarshallOfSound the previous API always resolved to an empty |
4ff0a02
to
52bef14
Compare
Release Notes Persisted
|
I have automatically backported this PR to "20-x-y", please check out #35358 |
I have automatically backported this PR to "21-x-y", please check out #35359 |
Description of Change
Closes #32756.
Closes #32164.
PR changes the backend for
WebFrameMain::ExecuteJavaScript
to use the version that allows resolving promises before returning the result. Not sure why the previous version which does not support promises https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_host_impl.cc;l=2667-2670 was adopted in 704d69a but this aligns the behavior withWebFrame::ExecuteJavaScript
.Was identified when writing tests for #35281
Checklist
npm test
passesRelease Notes
Notes: fix enable promise support with webFrameMain.executeJavaScript