Bug 229887 - Simplify PCM::Client interface
Summary: Simplify PCM::Client interface
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
Keywords: InRadar
Depends on:
Reported: 2021-09-03 12:42 PDT by Alex Christensen
Modified: 2021-09-03 16:15 PDT (History)
3 users (show)

See Also:

Patch (15.83 KB, patch)
2021-09-03 12:57 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2021-09-03 12:42:12 PDT
Simplify PCM::Client interface
Comment 1 Alex Christensen 2021-09-03 12:57:45 PDT
Created attachment 437303 [details]
Comment 2 Kate Cheney 2021-09-03 15:13:34 PDT
Comment on attachment 437303 [details]

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


> Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementNetworkLoader.cpp:50
> +        request.setHTTPBody(WebCore::FormData::create(jsonPayload->toJSONString().utf8()));

You dropped a .data() here, was that intentional?
Comment 3 Alex Christensen 2021-09-03 15:14:02 PDT
Yes, I think that removed an unnecessary CString copy.
Comment 4 EWS 2021-09-03 15:28:01 PDT
Committed r282031 (241335@main): <https://commits.webkit.org/241335@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437303 [details].
Comment 5 Radar WebKit Bug Importer 2021-09-03 15:29:15 PDT
Comment 6 John Wilander 2021-09-03 15:38:06 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=437303&action=review

LGTM. See comment.

> Source/WebKit/NetworkProcess/PrivateClickMeasurement/PrivateClickMeasurementNetworkLoader.cpp:41
> +static NetworkLoadParameters generateNetworkLoadParameters(URL&& url, RefPtr<JSON::Object>&& jsonPayload, PrivateClickMeasurement::PcmDataCarried pcmDataCarried)

This function does the exact same thing as the three deleted ones above, right? Except we lost the UNLIKELY(isDebugModeEnabled) which I don't know how much it gave us.
Comment 7 Alex Christensen 2021-09-03 16:15:32 PDT