Only use CFNetwork SPI for metrics where needed
Created attachment 380685 [details] Patch
Comment on attachment 380685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380685&action=review r=me The first time we tried this (bug 198762) got rolled out in r247095. Just make sure this test continues to pass! http/tests/inspector/network/har/har-page.html > Source/WTF/wtf/Platform.h:1521 > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101600) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) > +#define HAVE_CFNETWORK_METRICS 1 > +#endif Nit: I believe Keith Rollins was just investigating if we should enable these for tvOS / watchOS. The APIs do exist there, so I suggest we do that. Maybe you can sync up with him on how best to do that, but that doesn't block this. I'd probably have named this positive for "APIs" since older builds have the "SPIs". So maybe something like HAVE_CFNETWORK_METRICS_APIS_V4? I'd leave that up to you. > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:710 > + networkLoadMetrics.tlsCipher = stringForSSLCipher((SSLCipherSuite)[m.negotiatedTLSCipherSuite unsignedShortValue]); Shouldn't this be a `tls_ciphersuite_t` not a `SSLCipherSuite`? Or are they interchangeable? > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:734 > for (NSURLSessionTaskTransactionMetrics *transactionMetrics in metrics.transactionMetrics) { > +#if HAVE(CFNETWORK_METRICS) > + requestHeaderBytesSent += transactionMetrics.countOfRequestHeaderBytesSent; > + responseHeaderBytesReceived += transactionMetrics.countOfResponseHeaderBytesReceived; > + responseBodyBytesReceived += transactionMetrics.countOfResponseBodyBytesReceived; > + responseBodyDecodedSize += transactionMetrics.countOfResponseBodyBytesAfterDecoding ? transactionMetrics.countOfResponseBodyBytesAfterDecoding : transactionMetrics.countOfResponseBodyBytesReceived; > +#else Not significant but we should probably update NetworkLoadMetrics's values here from uint32_ts to uint64_ts. This was done in the first attempt to use the CFNetwork APIs: https://bugs.webkit.org/attachment.cgi?id=371875&action=review ``` uint32_t requestHeaderBytesSent; uint32_t responseHeaderBytesReceived; ``` The CFNetwork APIs here are `int64_t` and the SPIs here were `NSInteger`. So it seems a >32bit number could happen.
Created attachment 380769 [details] Patch
Created attachment 380772 [details] Patch
*** Bug 198762 has been marked as a duplicate of this bug. ***
Comment on attachment 380772 [details] Patch Clearing flags on attachment: 380772 Committed r251021: <https://trac.webkit.org/changeset/251021>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56313694>
Comment on attachment 380772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380772&action=review > Source/WTF/wtf/Platform.h:1519 > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101600) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) FWIW "PLATFORM(IOS_FAMILY) && only testing an IPHONE_OS version" is always wrong. This is disabled on all non-iOS iOS-family platforms.