[webkit-reviews] review denied: [Bug 195301] Resource Load Statistics: Log first-party navigations with link decoration : [Attachment 363671] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied John Wilander
<wilander at apple.com>'s request for review:
Bug 195301: Resource Load Statistics: Log first-party navigations with link
decoration
https://bugs.webkit.org/show_bug.cgi?id=195301

Attachment 363671: Patch

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




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

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

Looks good, with a couple of database issues. If you fix those, I think this
will be good to go!

>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:78
> +constexpr auto topFrameDecoratedNavigationsFromQuery = "INSERT INTO
TopFrameDecoratedNavigationsFrom (fromDomainID, toDomainID) "

You also need to create the table. I think you should use a shorter name: maybe
NavigationThroughDecoratedLink, with the from and toDomainID as you showed.

See the section with CREATE TABLE, below.

>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:82
> +constexpr auto
updateWasNavigatedToWithLinkDecorationByPrevalentResourceStatementQuery =
"UPDATE ObservedDomains SET wasNavigatedToWithLinkDecorationByPrevalentResource
= ? WHERE registrableDomain = ?"_s;

These names are way too long!