[webkit-reviews] review granted: [Bug 187671] Reduce size of NetworkLoadMetrics and therefore ResourceResponse : [Attachment 345008] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 187671: Reduce size of NetworkLoadMetrics and therefore ResourceResponse
https://bugs.webkit.org/show_bug.cgi?id=187671

Attachment 345008: Patch

https://bugs.webkit.org/attachment.cgi?id=345008&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 345008
  --> https://bugs.webkit.org/attachment.cgi?id=345008
Patch

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

Concept here is fine but std::numeric_limits<unit64_t>::max() is probably
better than writing -1.

Also if we really want to optimize the size of this it seems strange we really
need 64 bits for something like header size. There must be much smaller
practical limits for the sizes of various things here. And we could store
headers in a sorted vector instead of a map and save a *lot* more space.

I am also concerned that this leaves nothing behind to remind us to keep this
small. I could easily imagine someone showing up soon with a “cleanup” patch to
make this use std::optional and we are not even leaving a comment behind to
prevent that.

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:246
> +    if (networkLoadMetrics.requestHeaderBytesSent != -1ull)

-1ull does not make sense. -1 is not an unsigned number. Can we find a correct
way to write this?