[webkit-reviews] review granted: [Bug 219763] Accept click measurement data from hosting application : [Attachment 415945] Patch

[bugzilla-daemon at webkit.org]

John Wilander <wilander at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 219763: Accept click measurement data from hosting application
https://bugs.webkit.org/show_bug.cgi?id=219763

Attachment 415945: Patch

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




--- Comment #3 from John Wilander <wilander at apple.com> ---
Comment on attachment 415945
  --> https://bugs.webkit.org/attachment.cgi?id=415945
Patch

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

r+ with comments.

> Source/WebCore/loader/PrivateClickMeasurement.h:243
> +	   , m_purchaser { WTFMove(purchaser) }

These will be in-memory-only for now since we're not updating the persistence
layer. That's perfectly OK. However, it would be nice to restrict their size. I
believe we've said 100 characters max. Not a deal breaker.

> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:79
> +// FIXME: Move this to UIKitSPI.h

Do we need a bug tracking this?

> Source/WebKit/UIProcess/WebPageProxy.h:2909
> +    Optional<WebCore::PrivateClickMeasurement> m_privateClickMeasurement;

I wonder if this is too generic of a member name. Isn't
m_newPageNavigationPrivateClickMeasurement a pretty good description even with
your patch?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/EventAttribution.mm:56
> +    _sourceIdentifier = 42;

Could we add a test with a too large sourceIdentifier? That's the most
important input validation here.