[webkit-reviews] review granted: [Bug 172519] [WK2] Address thread safety issues with ResourceLoadStatistics : [Attachment 311375] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 172519: [WK2] Address thread safety issues with ResourceLoadStatistics
https://bugs.webkit.org/show_bug.cgi?id=172519

Attachment 311375: Patch

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




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

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

r=me if the bots are happy. But let's file a bug about simplifying the design.
This is too error prone.

> Source/WebCore/loader/ResourceLoadObserver.cpp:209
> +	   m_store->setResourceStatisticsForPrimaryDomain(targetPrimaryDomain,
WTFMove(targetStatistics));

I think this setResourceStatisticsForPrimaryDomain() is no longer needed now
that you are modifying a reference to the value in the hash map.

> Source/WebCore/loader/ResourceLoadStatisticsStore.cpp:206
> +void
ResourceLoadStatisticsStore::setNotificationCallback(WTF::Function<void()>
handler)

WTF::Function<void()>&& would be clearer since a WTF::Function has to be moved
around.