It's gone in libsoup3, we can just use soup_session_send_async() instead, which is what SoupRequest does internally.
Created attachment 417962 [details] Patch
Comment on attachment 417962 [details] Patch Patch LGTM with a couple of nits. Please check them out before landing, in particular I would like to see the suggested simplification for ”m_acceptEncoding” in the final version that lands; the other two suggestions I will leave them up to you =) View in context: https://bugs.webkit.org/attachment.cgi?id=417962&action=review > Source/WebCore/platform/network/soup/ResourceRequest.h:85 > + bool m_acceptEncoding : 1; It does not make sense to specify a bit width of “1” here. This is the only member for which the bit width is indicated, which will result in the compiler adding padding anyway—which defeats the intent of saving some bytes of memory. We may as well do: bool m_acceptEncoding { true }; …and skip setting the initial value in the constructors above. Just tested: % cat > t.cc <<EOF #include <cstdio> enum Flag { FLAG_FOO, FLAG_BAR, FLAG_BAZ }; class X { bool bit: 1; enum Flag f; }; class Y { bool bit; enum Flag f; }; int main() { std::printf("X: %zu bytes, Y: %zu bytes\n", sizeof(X), sizeof(Y)); } EOF % g++ -o t t.cc && ./t X: 8 bytes, Y: 8 bytes % > Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:-219 > - Nice to see this bit gone \o/ > Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:333 > + g_object_set_data_full(G_OBJECT(task->m_pendingResult.get()), "wk-send-request-data", protectedData.release(), [](gpointer data) { Is there any reason not to use “g_object_set_qdata_full()”? That would avoid the string→GQuark conversion on each usage. Maybe we should open a new bug to convert all the uses of “g_object_{g,s}et_data” to use the “qdata” variants, WDYT? > Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:477 > +} The “didSniffContent()” member function is private and only ever called from the “didSniffContentCallback()” function right above. I think it's may not be worth it to have this function containing a single assignment, it could be done just directly inside “didSniffContentCallback()” — though I have no strong preference, feel free to leave this as-is.
Comment on attachment 417962 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417962&action=review >> Source/WebCore/platform/network/soup/ResourceRequest.h:85 >> + bool m_acceptEncoding : 1; > > It does not make sense to specify a bit width of “1” here. This is the > only member for which the bit width is indicated, which will result in > the compiler adding padding anyway—which defeats the intent of saving > some bytes of memory. We may as well do: > > bool m_acceptEncoding { true }; > > …and skip setting the initial value in the constructors above. > > Just tested: > > % cat > t.cc <<EOF > #include <cstdio> > enum Flag { FLAG_FOO, FLAG_BAR, FLAG_BAZ }; > class X { bool bit: 1; enum Flag f; }; > class Y { bool bit; enum Flag f; }; > int main() { > std::printf("X: %zu bytes, Y: %zu bytes\n", sizeof(X), sizeof(Y)); > } > EOF > % g++ -o t t.cc && ./t > X: 8 bytes, Y: 8 bytes > % Sure. >> Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:333 >> + g_object_set_data_full(G_OBJECT(task->m_pendingResult.get()), "wk-send-request-data", protectedData.release(), [](gpointer data) { > > Is there any reason not to use “g_object_set_qdata_full()”? That would > avoid the string→GQuark conversion on each usage. Maybe we should open > a new bug to convert all the uses of “g_object_{g,s}et_data” to use the > “qdata” variants, WDYT? I don't know if it really has any impact. But it's probably better to do that in a separate commit for all the set_data cases instead. >> Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:477 >> +} > > The “didSniffContent()” member function is private and only ever called from > the “didSniffContentCallback()” function right above. I think it's may not be > worth it to have this function containing a single assignment, it could be done > just directly inside “didSniffContentCallback()” — though I have no strong > preference, feel free to leave this as-is. Yes, this is just to follow the pattern for all other async ops in this file, didFooCallback -> didFoo
Created attachment 418390 [details] Patch for landing
Created attachment 418394 [details] Patch for landing
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Linking the bug with the regressions from r271879. (Should have gardened with this one instead).
Committed r272010: <https://trac.webkit.org/changeset/272010>