[webkit-reviews] review granted: [Bug 226982] Reduce sizeof(NetworkLoadMetrics) : [Attachment 436964] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 226982: Reduce sizeof(NetworkLoadMetrics)
https://bugs.webkit.org/show_bug.cgi?id=226982

Attachment 436964: Patch

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




--- Comment #11 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 436964
  --> https://bugs.webkit.org/attachment.cgi?id=436964
Patch

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

r=me with changes

> Source/WebCore/platform/network/NetworkLoadMetrics.h:67
> +    NetworkLoadMetrics(const NetworkLoadMetrics&) = default;

Why do we need these? What prevents the fault ones from being generated?

> Source/WebCore/platform/network/NetworkLoadMetrics.h:116
> +struct AdditionalNetworkLoadMetricsForWebInspector : public
RefCounted<AdditionalNetworkLoadMetricsForWebInspector> {

Given that this subclasses RefCounted, we should make the constructor private
and have a public create() function. It is too error-prone otherwise.

Alternatively, looking at your patch, I don't think we need to subclass
RefCounted at all as it doesn't seem we ever share ownership of such objects?
If so, I would just not subclass RefCounted and use makeUnique / unique_ptr.