[webkit-reviews] review granted: [Bug 231278] Add PushSubscription stubs : [Attachment 440330] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 5 22:48:48 PDT 2021


Alex Christensen <achristensen at apple.com> has granted Ben Nham
<nham at apple.com>'s request for review:
Bug 231278: Add PushSubscription stubs
https://bugs.webkit.org/show_bug.cgi?id=231278

Attachment 440330: Patch

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




--- Comment #3 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 440330
  --> https://bugs.webkit.org/attachment.cgi?id=440330
Patch

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

> Source/WebCore/Modules/push-api/PushEncryptionKeyName.h:32
> +enum class PushEncryptionKeyName {

: bool

> Source/WebCore/Modules/push-api/PushSubscription.cpp:103
> +	       { "p256dh", base64URLEncodeToString(m_clientECDHPublicKey) },
> +	       { "auth", base64URLEncodeToString(m_auth) }

_s

> Source/WebCore/Modules/push-api/PushSubscription.h:48
> +    WEBCORE_EXPORT static Ref<PushSubscription> create(String&& endpoint,
std::optional<DOMTimeStamp> expirationTime, Ref<PushSubscriptionOptions>&&,
Vector<uint8_t>&& clientECDHPublicKey, Vector<uint8_t>&& auth);

This might be a good candidate for something like this:
template<typename... Args> static Ref<PushSubscription> create(Args&&... args)
{ return adoptRef(*new PushSubscription(std::forward<Args>(args)...)); }

> Source/WebCore/Modules/push-api/PushSubscription.h:66
> +    Vector<uint8_t> m_auth;

Could you use a whole word to describe what this is instead of an abbreviation?

> Source/WebCore/Modules/push-api/PushSubscriptionOptions.cpp:40
> +

extra space.


More information about the webkit-reviews mailing list