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
[GLib] New API to get the request body of WebKitURISchemeRequest #10714
Conversation
EWS run on previous version of this PR (hash e3384ed) |
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.
Thanks for the patch @wusyong, this looks neat ποΈ
We have just branched for 2.40 recently, but IMO this is quite nice to have and should be fine to add into the release branch if @carlosgcampos or @mcatanzaro agreeβthe patch is self-contained, only adds one public method, and comes with a test. WDYT?
(As with any API change, we need a second approval before landing, so let's wait and see what the other reviewers think.)
@wusyong I have changed the Bugzilla issue title to β[GLib] New API to get the request body of WebKitURISchemeRequestβ because to better reflect what the feature is about, and also because it also bring the new API to the WPE port (GLib = GTK + WPE). Could you change the first line of the commit log accordingly? Thanks! ππΌ |
EWS run on previous version of this PR (hash b017a81) |
EWS run on previous version of this PR (hash 978112b) |
Looks good to me, but I wonder if it would be easier to expose the body as a GBytes instead. In this case, the body is always serialized over IPC, so it fits in memory, and it's not expected to be processed as a stream, I guess. |
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 work.
GBytes is fine. Or simply |
EWS run on previous version of this PR (hash 5fa5f51) |
I'll stick to GBytes since many APIs also return this type for bytes. I've updated the return type to GBytes. Please let me know if I missed anything. |
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.
By the way: it's very unusual for your first contribution to be new public API, so congrats on that. This is simple, adequately tested, and looks good.
EWS run on previous version of this PR (hash de12c04) |
One last thing: please squash your two commits into one. We can't land pull requests with multiple commits for WebKit-specific reasons. So please fix that, and make the one-line change that I proposed above, and then this is good to go. And we'll backport to 2.40 as well so you can use it sooner rather than later. |
EWS run on previous version of this PR (hash cf8fdad) |
EWS run on previous version of this PR (hash 3f5fcd4) |
EWS run on previous version of this PR (hash cb97834) |
EWS run on previous version of this PR (hash 9b57d27) |
EWS run on current version of this PR (hash 1d216ea) |
https://bugs.webkit.org/show_bug.cgi?id=252564 Reviewed by Carlos Garcia Campos, Michael Catanzaro and Adrian Perez de Castro. * Source/WebCore/platform/network/soup/ResourceRequest.h: * Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp: (WebCore::ResourceRequest::createInputStream const): Added new function API * Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.h.in: * Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp: (webkit_uri_scheme_request_get_http_body): Added new function API * Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp: (testWebContextURIScheme): Expanded 'post' URI scheme to test webkit_uri_scheme_request_get_http_body Canonical link: https://commits.webkit.org/260949@main
1d216ea
to
5fe923a
Compare
Committed 260949@main (5fe923a): https://commits.webkit.org/260949@main Reviewed commits have been landed. Closing PR #10714 and removing active labels. |
5fe923a
1d216ea
π§ͺ ios-wk2π§ͺ api-macπ§ͺ gtk-wk2π§ͺ api-iosπ§ͺ api-gtkπ§ͺ mac-AS-debug-wk2