We should add a way to determine if a load is carrying user-identifiable data.
rdar://problem/71348187
Created attachment 418694 [details] Patch
Comment on attachment 418694 [details] 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.
Comment on attachment 418694 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418694&action=review I'll make these changes, thanks John! >> Source/WebCore/ChangeLog:3 >> + PCM: Indicate if load data is user identifiable > > Maybe add "network load" Will fix. >> 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? Yes I suppose this is still vague. I'm leaning toward carriesUserIdentifiablePCMData because I could see 'clickData' being confused with things unrelated to PCM. >> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:166 >> + loadParameters.carriesUserIdentifiableData = WebCore::PrivateClickMeasurement::CarriesUserIdentifiableData::Yes; > > It's the opposite. :) This load does not carry user identifying data. Oops, I was thinking backwards after our conversation on naming -- good catch.
Created attachment 418912 [details] Patch
Comment on attachment 418912 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418912&action=review > 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.carriesUserIdentifiablePCMData); Good choice.
Created attachment 418940 [details] Patch
(In reply to katherine_cheney from comment #7) > Created attachment 418940 [details] > Patch Name change based on offline conversation.
Comment on attachment 418940 [details] Patch LGTM. I don't know why cq has a minus.
Created attachment 419055 [details] Patch for landing
Committed r272239: <https://trac.webkit.org/changeset/272239> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419055 [details].