[webkit-reviews] review granted: [Bug 198227] Resource Load Statistics: Downgrade document.referrer to the referrer's eTLD+1 if the page was navigated to with a prevalent resource referrer containing link decoration : [Attachment 370605] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted John Wilander
<wilander at apple.com>'s request for review:
Bug 198227: Resource Load Statistics: Downgrade document.referrer to the
referrer's eTLD+1 if the page was navigated to with a prevalent resource
referrer containing link decoration
https://bugs.webkit.org/show_bug.cgi?id=198227

Attachment 370605: Patch

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




--- Comment #5 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 370605
  --> https://bugs.webkit.org/attachment.cgi?id=370605
Patch

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

r=me with a few comments.

> Source/WebCore/page/CrossSiteNavigationDataTransfer.h:34
> +    enum Flag {

enum class Flag : uint8_t

> Source/WebKit/UIProcess/WebProcessPool.cpp:2528
> +    m_networkProcess->didCommitCrossSiteLoadWithDataTransfer(sessionID,
fromDomain, toDomain, navigationDataTransfer, pageID, [] { });

I don't understand why this completion handler is necessary.  Could we remove
it and have this message just be a notification?  That would make this code a
lot simpler.

>
LayoutTests/http/tests/resourceLoadStatistics/downgraded-referrer-for-navigatio
n-with-link-query-from-prevalent-resource.html:14
> +	   if (document.referrer === prevalentResourceOrigin + "/")

If this test is flaky, we should spin until this is true.  I hope it's not
flaky, but I think it's currently based on a race condition.