Add NetworkProcess stubs for push subscriptions
Created attachment 441659 [details] Patch
Created attachment 441661 [details] Patch
Created attachment 441668 [details] Patch
<rdar://problem/84400696>
Comment on attachment 441668 [details] Patch One note is that the IPC parser didn't seem to handle a return type of Expected<enum:uint8_t PushPermissionState, ExceptionData> properly (its codegen tried to emit an #include<enum:uint8_t ...> in the cpp file). For now that IPC message just returns an Expected<uint8_t, ExceptionData> until I can take a look at that codegen issue in a separate patch.
Comment on attachment 441668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441668&action=review > Source/WebCore/ChangeLog:9 > + via ServiceWorkerRegistration => ServiceWorkerContainer => SWClientConnection. Do the existing tests fully cover this case? Maybe it is too early to do, but maybe there are some things to test (network process crash). > Source/WebCore/Modules/push-api/PushSubscription.h:62 > + WEBCORE_EXPORT PushSubscription(ServiceWorkerRegistration*, String&& endpoint, std::optional<EpochTimeStamp> expirationTime, Ref<PushSubscriptionOptions>&&, Vector<uint8_t>&& clientECDHPublicKey, Vector<uint8_t>&& auth); It would be better to pass a Ref<ServiceWorkerRegistration> if possible. If the issue is PushSubscription creation in Internals, we can always change createPushSubscription to add a ServiceWorkerRegistration as parameter. > Source/WebCore/Modules/push-api/PushSubscription.h:64 > + RefPtr<ServiceWorkerRegistration> m_serviceWorkerRegistration; Would be better as a Ref. > Source/WebCore/Modules/push-api/PushSubscriptionData.h:60 > +bool PushSubscriptionData::decode(Decoder& decoder, PushSubscriptionData& data) Would be better to use the modern form: template<class Decoder> std::optional<PushSubscriptionData> PushSubscriptionData::decode(Decoder& decoder) > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:557 > + auto options = PushSubscriptionOptions::create(true, WTFMove(data.serverVAPIDPublicKey)); true is a bit mysterious. I guess bool userVisibleOnly = true would be clearer. > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:581 > + promise.resolve(result.releaseReturnValue()); Can we use promise.settle(WTFMove(result))? > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:599 > + promise.resolve(createPushSubscriptionFromData(protectedRegistration, WTFMove(*optionalPushSubscriptionData)).ptr()); This triggers some count churn. I would write it as RefPtr<PushSubscription> subscription; if (optionalPushSubscriptionData) subscription = createPushSubscriptionFromData(protectedRegistration, WTFMove(*optionalPushSubscriptionData)); promise.resolve(WTFMove(subscription)); > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:611 > + promise.resolve(result.releaseReturnValue()); promise.settle as well? > Source/WebCore/workers/service/WorkerSWClientConnection.cpp:224 > +void WorkerSWClientConnection::subscribeToPushService(ServiceWorkerRegistrationIdentifier registrationIdentifier, const Vector<uint8_t>& applicationServerKey, SubscribeToPushServiceCallback&& callback) Could we pass a Vector<uint8_t>&& to not copy it here? It seems at the push manager layer, we have a Vector&&.
Created attachment 441788 [details] Patch
Comment on attachment 441788 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441788&action=review > Source/WebCore/Modules/push-api/PushSubscriptionData.cpp:2 > + * Copyright (C) 2017 Apple Inc. All rights reserved. nit: 2021
Created attachment 441824 [details] Patch
Committed r284518 (243261@main): <https://commits.webkit.org/243261@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441824 [details].