[Webkit-unassigned] [Bug 220764] [SOUP] Stop using SoupRequest API in preparation for libsoup3

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=220764

--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 417962
  --> https://bugs.webkit.org/attachment.cgi?id=417962
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

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210126/32adb7f3/attachment.htm>