[webkit-reviews] review denied: [Bug 233295] More webpushd architecture work : [Attachment 444641] EWS v5
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Nov 18 11:51:33 PST 2021
Alex Christensen <achristensen at apple.com> has denied Brady Eidson
<beidson at apple.com>'s request for review:
Bug 233295: More webpushd architecture work
https://bugs.webkit.org/show_bug.cgi?id=233295
Attachment 444641: EWS v5
https://bugs.webkit.org/attachment.cgi?id=444641&action=review
--- Comment #7 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 444641
--> https://bugs.webkit.org/attachment.cgi?id=444641
EWS v5
View in context: https://bugs.webkit.org/attachment.cgi?id=444641&action=review
> Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.cpp:45
> + auto cfData =
m_networkSession.networkProcess().sourceApplicationAuditData();
You can probably do this from sourceApplicationAuditToken to be slightly more
efficient.
> Source/WebKit/Shared/Cocoa/CodeSigning.mm:78
> + auto pair = codeSigningIdentifierAndPlatformBinaryStatus(token);
This doesn't need to call SecTaskGetCodeSignStatus.
> Source/WebKit/webpushd/PushClientConnection.mm:42
> +void ClientConnection::setAuditTokenData(const Vector<uint8_t>& tokenData)
This is a little strange. Isn't xpc_connection_get_audit_token enough?
> Source/WebKit/webpushd/PushClientConnection.mm:72
> + m_hostHasPushEntitlement = WTF::hasEntitlement(*m_hostAppAuditToken,
"com.apple.private.webkit.webpush");
This is typically done at the time a connection is being received, then
terminating the connection if it doesn't have an entitlement. That way you
avoid bugs where you forgot to check the entitlement. If you have a
connection, then it is entitled.
> Source/WebKit/webpushd/WebPushDaemon.h:56
> + void echoTwice(ClientConnection*, const String&,
CompletionHandler<void(const String&)>&& replySender);
We can probably remove this at some point.
> Source/WebKit/webpushd/WebPushDaemon.h:77
> + HashMap<xpc_connection_t, std::unique_ptr<ClientConnection>>
m_connectionMap;
It's a little strange that the connection is both in the key and the value.
More information about the webkit-reviews
mailing list