[webkit-reviews] review granted: [Bug 221113] PCM: Indicate if load data is user identifiable : [Attachment 418694] Patch

[bugzilla-daemon at webkit.org]

John Wilander <wilander at apple.com> has granted katherine_cheney at apple.com's
request for review:
Bug 221113: PCM: Indicate if load data is user identifiable
https://bugs.webkit.org/show_bug.cgi?id=221113

Attachment 418694: Patch

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




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

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

Looks good but let's work some more on naming and flip the currently used
setting.

> Source/WebCore/ChangeLog:3
> +	   PCM: Indicate if load data is user identifiable

Maybe add "network load"

> Source/WebKit/ChangeLog:3
> +	   PCM: Indicate if load data is user identifiable

Ditto.

> Source/WebKit/NetworkProcess/NetworkDataTask.cpp:57
> +    return NetworkDataTaskCocoa::create(session, client, parameters.request,
parameters.webFrameID, parameters.webPageID,
parameters.storedCredentialsPolicy, parameters.contentSniffingPolicy,
parameters.contentEncodingSniffingPolicy,
parameters.shouldClearReferrerOnHTTPSToHTTPRedirect,
parameters.shouldPreconnectOnly, parameters.isMainFrameNavigation,
parameters.isMainResourceNavigationForAnyFrame,
parameters.networkActivityTracker, parameters.isNavigatingToAppBoundDomain,
parameters.shouldRelaxThirdPartyCookieBlocking,
parameters.carriesUserIdentifiableData);

Hmm. It's unfortunate that we lose scoping here. If something said PCM, it
would be much easier to understand what this parameter is for. I was hoping
PrivateClickMeasurement::CarriesUserIdentifiableData would do it but it doesn't
help on the call site. Ideas? Maybe carriesUserIdentifiableClickData or
carriesUserIdentifiablePCMData?

> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:166
> +    loadParameters.carriesUserIdentifiableData =
WebCore::PrivateClickMeasurement::CarriesUserIdentifiableData::Yes;

It's the opposite. :) This load does not carry user identifying data.