Bug 188391 – [Curl] Surface additional NetworkLoadMetrics

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Don Olmstead]

Currently a few network load metrics are missing in the curl port.

[Don Olmstead]

Created attachment 346737 [details]
Patch

[Don Olmstead]

Created attachment 346738 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 346738 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346738&action=review

Nice! r=me

You may be able to enable a few of the Inspector LayoutTests. Namely:
LayoutTests/http/tests/inspector/network/resource-sizes-network.html

> Source/WebCore/platform/network/curl/CurlContext.cpp:739
> +    long protocol;

This appears to be unused and can be removed.

> Source/WebCore/platform/network/curl/CurlContext.cpp:761
> +    errorCode = curl_easy_getinfo(m_handle, CURLINFO_REQUEST_SIZE, &requestHeaderSize);

Can you subtract out the requestBodySize to get requestHeaderSize closer to the actual value? You'd want to do some sanity checking to see if that makes sense.

> Source/WebCore/platform/network/curl/CurlContext.cpp:773
> +    errorCode = curl_easy_getinfo(m_handle, CURLINFO_SIZE_DOWNLOAD_T, &responseBodySize);

Same here, does this include the Header Size or not? The curl docs did not seem clear to me.

[Basuke Suzuki]

Comment on attachment 346738 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346738&action=review

> Source/WebCore/platform/network/curl/CurlContext.cpp:82
> +static const ASCIILiteral http20Protocol { "h2"_s };

constant should be named like kHttpProtocol10, kHttpProtocol11, kHttpProtocol2. Also version number of HTTP/2 is just 2, not 2.0. https://http2.github.io/faq/#is-it-http20-or-http2

>> Source/WebCore/platform/network/curl/CurlContext.cpp:761
>> +    errorCode = curl_easy_getinfo(m_handle, CURLINFO_REQUEST_SIZE, &requestHeaderSize);
> 
> Can you subtract out the requestBodySize to get requestHeaderSize closer to the actual value? You'd want to do some sanity checking to see if that makes sense.

Write. Request size includes entire size (header + body). I confirmed that by reading code for libcurl.

>> Source/WebCore/platform/network/curl/CurlContext.cpp:773
>> +    errorCode = curl_easy_getinfo(m_handle, CURLINFO_SIZE_DOWNLOAD_T, &responseBodySize);
> 
> Same here, does this include the Header Size or not? The curl docs did not seem clear to me.

This doesn't include the header size.

[Basuke Suzuki]

> Write.

I mean "Right". :(

[Don Olmstead]

Created attachment 346805 [details]
Patch

Updated based on review feedback.

[WebKit Commit Bot]

Comment on attachment 346805 [details]
Patch

Clearing flags on attachment: 346805

Committed r234715: <https://trac.webkit.org/changeset/234715>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43073623>