[webkit-reviews] review granted: [Bug 174301] Further WebResourceLoadStatisticsStore / ResourceLoadStatisticsStore clean up : [Attachment 314983] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 174301: Further WebResourceLoadStatisticsStore /
ResourceLoadStatisticsStore clean up
https://bugs.webkit.org/show_bug.cgi?id=174301

Attachment 314983: Patch

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




--- Comment #9 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 314983
  --> https://bugs.webkit.org/attachment.cgi?id=314983
Patch

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

r=me, with some minor comments.

> Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:173
> +	       return WTFMove(statistic);

Very nice!

> Source/WebKit2/UIProcess/Storage/ResourceLoadStatisticsStore.cpp:-322
> -}

I wish the code review tool represented moved code better. It looks like this
was lifted to the WK2 layer.

> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:-156
> -    // Switch to the main thread to get the default website data store

Is this not true anymore? I can't remember if your other changes ensured that
we always were interacting with the right website data store. If you did, then
is it really necessary to move to the main runloop?

If your changes did not tie us to the main data store, I think this comment is
still useful.

> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:291
> +    // FIXME: Decide what to call this file.

I don't think we're going to change the name at this point. Let's just remove
this comment.