Make sure calling showNotification will extend the service worker lif…

…etime

https://bugs.webkit.org/show_bug.cgi?id=240273
<rdar://92978482>

Reviewed by Chris Dumez.

Source/WebCore:

Update NotificationClient API so that show is taking a completion handler.
Make use of this completion handler to resolve the promise when the show notification steps are done, as per spec.
Register push event to ServiceWorkerGlobalScope when the event handlers are called.
When ServiceWorkerRegistration::show is called during one of the push event handlers,
extend the push event lifetime by adding the show notification promise to the push event.

Covered by API test.

* Modules/notifications/Notification.cpp:
* Modules/notifications/Notification.h:
* Modules/notifications/NotificationClient.h:
* dom/ScriptExecutionContext.cpp:
* dom/ScriptExecutionContext.h:
* workers/service/ServiceWorkerGlobalScope.cpp:
* workers/service/ServiceWorkerGlobalScope.h:
* workers/service/ServiceWorkerRegistration.cpp:
* workers/service/ServiceWorkerRegistration.h:
* workers/service/context/ServiceWorkerThread.cpp:

Source/WebKit:

On WebProcess side, implement the new NoficationClient::show API that takes a callback.
Delay calling this callback until UIProcess tells us so through async IPC.
On UIProcess side, take a callback and call it when the show notification steps are done.

* NetworkProcess/Notifications/NetworkNotificationManager.cpp:
* NetworkProcess/Notifications/NetworkNotificationManager.h:
* Shared/Notifications/NotificationManagerMessageHandler.h:
* Shared/Notifications/NotificationManagerMessageHandler.messages.in:
* UIProcess/Notifications/ServiceWorkerNotificationHandler.cpp:
* UIProcess/Notifications/ServiceWorkerNotificationHandler.h:
* UIProcess/Notifications/WebNotificationManagerMessageHandler.cpp:
* UIProcess/Notifications/WebNotificationManagerMessageHandler.h:
* WebProcess/GPU/webrtc/RemoteVideoFrameObjectHeapProxyProcessor.cpp:
* WebProcess/Notifications/WebNotificationManager.cpp:
* WebProcess/Notifications/WebNotificationManager.h:
* WebProcess/WebCoreSupport/WebNotificationClient.cpp:
* WebProcess/WebCoreSupport/WebNotificationClient.h:

Source/WebKitLegacy/mac:

* WebCoreSupport/WebNotificationClient.h:
* WebCoreSupport/WebNotificationClient.mm:

Tools:

* TestWebKitAPI/TestNotificationProvider.cpp:
* TestWebKitAPI/TestNotificationProvider.h:
* TestWebKitAPI/Tests/WebKitCocoa/PushAPI.mm:

Canonical link: https://commits.webkit.org/250583@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294225 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,30 @@
+2022-05-16  Youenn Fablet  <youenn@apple.com>
+
+        Make sure calling showNotification will extend the service worker lifetime
+        https://bugs.webkit.org/show_bug.cgi?id=240273
+        <rdar://92978482>
+
+        Reviewed by Chris Dumez.
+
+        Update NotificationClient API so that show is taking a completion handler.
+        Make use of this completion handler to resolve the promise when the show notification steps are done, as per spec.
+        Register push event to ServiceWorkerGlobalScope when the event handlers are called.
+        When ServiceWorkerRegistration::show is called during one of the push event handlers,
+        extend the push event lifetime by adding the show notification promise to the push event.
+
+        Covered by API test.
+
+        * Modules/notifications/Notification.cpp:
+        * Modules/notifications/Notification.h:
+        * Modules/notifications/NotificationClient.h:
+        * dom/ScriptExecutionContext.cpp:
+        * dom/ScriptExecutionContext.h:
+        * workers/service/ServiceWorkerGlobalScope.cpp:
+        * workers/service/ServiceWorkerGlobalScope.h:
+        * workers/service/ServiceWorkerRegistration.cpp:
+        * workers/service/ServiceWorkerRegistration.h:
+        * workers/service/context/ServiceWorkerThread.cpp:
+
 2022-05-16  Martin Robinson  <mrobinson@webkit.org>
 
         Do not allow unitless values for CSS unprefixed perspective property

Source/WebCore/Modules/notifications/Notification.cpp

@@ -49,6 +49,7 @@
 #include "WindowFocusAllowedIndicator.h"
 #include <wtf/CompletionHandler.h>
 #include <wtf/IsoMallocInlines.h>
+#include <wtf/Scope.h>
 
 namespace WebCore {
 
@@ -143,17 +144,23 @@ void Notification::markAsShown()
     m_state = Showing;
 }
 
-void Notification::show()
+void Notification::show(CompletionHandler<void()>&& callback)
 {
+    CompletionHandlerCallingScope scope { WTFMove(callback) };
+
     // prevent double-showing
     if (m_state != Idle)
         return;
 
-    auto* client = clientFromContext();
+    auto* context = scriptExecutionContext();
+    if (!context)
+        return;
+
+    auto* client = context->notificationClient();
     if (!client)
         return;
 
-    if (client->checkPermission(scriptExecutionContext()) != Permission::Granted) {
+    if (client->checkPermission(context) != Permission::Granted) {
         switch (m_notificationSource) {
         case NotificationSource::Document:
             dispatchErrorEvent();
@@ -163,11 +170,10 @@ void Notification::show()
             // If permission has since been revoked, then silently failing here is expected behavior.
             break;
         }
-
         return;
     }
 
-    if (client->show(*this))
+    if (client->show(*this, scope.release()))
         m_state = Showing;
 }
 

Source/WebCore/Modules/notifications/Notification.h

@@ -39,6 +39,7 @@
 #include "NotificationPermission.h"
 #include "ScriptExecutionContextIdentifier.h"
 #include "SerializedScriptValue.h"
+#include <wtf/CompletionHandler.h>
 #include <wtf/URL.h>
 #include <wtf/UUID.h>
 #include "WritingMode.h"
@@ -74,7 +75,7 @@ class Notification final : public ThreadSafeRefCounted<Notification>, public Act
 
     WEBCORE_EXPORT virtual ~Notification();
 
-    void show();
+    void show(CompletionHandler<void()>&& = [] { });
     void close();
 
     const String& title() const { return m_title; }

Source/WebCore/Modules/notifications/NotificationClient.h

@@ -47,7 +47,7 @@ class NotificationClient {
     using PermissionHandler = CompletionHandler<void(Permission)>;
 
     // Requests that a notification be shown.
-    virtual bool show(Notification&) = 0;
+    virtual bool show(Notification&, CompletionHandler<void()>&&) = 0;
 
     // Requests that a notification that has already been shown be canceled.
     virtual void cancel(Notification&) = 0;

Source/WebCore/dom/ScriptExecutionContext.cpp

@@ -176,6 +176,10 @@ ScriptExecutionContext::~ScriptExecutionContext()
     m_inScriptExecutionContextDestructor = true;
 #endif // ASSERT_ENABLED
 
+    auto callbacks = WTFMove(m_notificationCallbacks);
+    for (auto& callback : callbacks.values())
+        callback();
+
 #if ENABLE(SERVICE_WORKER)
     setActiveServiceWorker(nullptr);
 #endif
@@ -767,4 +771,16 @@ ScriptExecutionContext::HasResourceAccess ScriptExecutionContext::canAccessResou
     RELEASE_ASSERT_NOT_REACHED();
 }
 
+ScriptExecutionContext::NotificationCallbackIdentifier ScriptExecutionContext::addNotificationCallback(CompletionHandler<void()>&& callback)
+{
+    auto identifier = NotificationCallbackIdentifier::generateThreadSafe();
+    m_notificationCallbacks.add(identifier, WTFMove(callback));
+    return identifier;
+}
+
+CompletionHandler<void()> ScriptExecutionContext::takeNotificationCallback(NotificationCallbackIdentifier identifier)
+{
+    return m_notificationCallbacks.take(identifier);
+}
+
 } // namespace WebCore

Source/WebCore/dom/ScriptExecutionContext.h

@@ -314,6 +314,12 @@ class ScriptExecutionContext : public SecurityContext, public CanMakeWeakPtr<Scr
     enum class HasResourceAccess : uint8_t { No, Yes, DefaultForThirdParty };
     WEBCORE_EXPORT HasResourceAccess canAccessResource(ResourceType) const;
 
+    enum NotificationCallbackIdentifierType { };
+    using NotificationCallbackIdentifier = ObjectIdentifier<NotificationCallbackIdentifierType>;
+
+    WEBCORE_EXPORT NotificationCallbackIdentifier addNotificationCallback(CompletionHandler<void()>&&);
+    WEBCORE_EXPORT CompletionHandler<void()> takeNotificationCallback(NotificationCallbackIdentifier);
+
 protected:
     class AddConsoleMessageTask : public Task {
     public:
@@ -396,6 +402,8 @@ class ScriptExecutionContext : public SecurityContext, public CanMakeWeakPtr<Scr
 
     bool m_hasLoggedAuthenticatedEncryptionWarning { false };
     StorageBlockingPolicy m_storageBlockingPolicy { StorageBlockingPolicy::AllowAll };
+
+    HashMap<NotificationCallbackIdentifier, CompletionHandler<void()>> m_notificationCallbacks;
 };
 
 } // namespace WebCore

Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp

@@ -37,6 +37,7 @@
 #include "FrameLoaderClient.h"
 #include "JSDOMPromiseDeferred.h"
 #include "NotificationEvent.h"
+#include "PushEvent.h"
 #include "SWContextManager.h"
 #include "ServiceWorker.h"
 #include "ServiceWorkerClient.h"
@@ -78,6 +79,14 @@ ServiceWorkerGlobalScope::~ServiceWorkerGlobalScope()
     callOnMainThread([notificationClient = WTFMove(m_notificationClient)] { });
 }
 
+void ServiceWorkerGlobalScope::dispatchPushEvent(PushEvent& pushEvent)
+{
+    ASSERT(!m_pushEvent);
+    m_pushEvent = &pushEvent;
+    dispatchEvent(pushEvent);
+    m_pushEvent = nullptr;
+}
+
 void ServiceWorkerGlobalScope::notifyServiceWorkerPageOfCreationIfNecessary()
 {
     auto serviceWorkerPage = this->serviceWorkerPage();

Source/WebCore/workers/service/ServiceWorkerGlobalScope.h

@@ -40,6 +40,7 @@ namespace WebCore {
 class DeferredPromise;
 class ExtendableEvent;
 class Page;
+class PushEvent;
 class ServiceWorkerClient;
 class ServiceWorkerClients;
 class ServiceWorkerThread;
@@ -81,6 +82,9 @@ class ServiceWorkerGlobalScope final : public WorkerGlobalScope {
 
     WEBCORE_EXPORT Page* serviceWorkerPage();
 
+    void dispatchPushEvent(PushEvent&);
+    PushEvent* pushEvent() { return m_pushEvent.get(); }
+
     bool hasPendingSilentPushEvent() const { return m_hasPendingSilentPushEvent; }
     void setHasPendingSilentPushEvent(bool value) { m_hasPendingSilentPushEvent = value; }
 
@@ -112,6 +116,7 @@ class ServiceWorkerGlobalScope final : public WorkerGlobalScope {
     bool m_hasPendingSilentPushEvent { false };
     bool m_isProcessingUserGesture { false };
     Timer m_userGestureTimer;
+    RefPtr<PushEvent> m_pushEvent;
 };
 
 } // namespace WebCore

Source/WebCore/workers/service/ServiceWorkerRegistration.cpp

@@ -32,12 +32,14 @@
 #include "Event.h"
 #include "EventLoop.h"
 #include "EventNames.h"
+#include "JSDOMPromise.h"
 #include "JSDOMPromiseDeferred.h"
 #include "JSNotification.h"
 #include "Logging.h"
 #include "NavigationPreloadManager.h"
 #include "NotificationClient.h"
 #include "NotificationPermission.h"
+#include "PushEvent.h"
 #include "ServiceWorker.h"
 #include "ServiceWorkerContainer.h"
 #include "ServiceWorkerGlobalScope.h"
@@ -274,40 +276,44 @@ NavigationPreloadManager& ServiceWorkerRegistration::navigationPreload()
 }
 
 #if ENABLE(NOTIFICATION_EVENT)
-void ServiceWorkerRegistration::showNotification(ScriptExecutionContext& context, String&& title, NotificationOptions&& options, DOMPromiseDeferred<void>&& promise)
+void ServiceWorkerRegistration::showNotification(ScriptExecutionContext& context, String&& title, NotificationOptions&& options, Ref<DeferredPromise>&& promise)
 {
     if (!m_activeWorker) {
-        promise.reject(Exception { TypeError, "Registration does not have an active worker"_s });
+        promise->reject(Exception { TypeError, "Registration does not have an active worker"_s });
         return;
     }
 
     auto* client = context.notificationClient();
     if (!client) {
-        promise.reject(Exception { TypeError, "Registration not active"_s });
+        promise->reject(Exception { TypeError, "Registration not active"_s });
         return;
     }
 
     if (client->checkPermission(&context) != NotificationPermission::Granted) {
-        promise.reject(Exception { TypeError, "Registration does not have permission to show notifications"_s });
+        promise->reject(Exception { TypeError, "Registration does not have permission to show notifications"_s });
         return;
     }
 
     if (context.isServiceWorkerGlobalScope())
         downcast<ServiceWorkerGlobalScope>(context).setHasPendingSilentPushEvent(false);
 
-    // The Notification is kept alive by virtue of being show()'n soon.
-    // FIXME: When implementing getNotifications(), store this Notification in the registration's notification list.
     auto notificationResult = Notification::createForServiceWorker(context, WTFMove(title), WTFMove(options), m_registrationData.scopeURL);
     if (notificationResult.hasException()) {
-        promise.reject(notificationResult.releaseException());
+        promise->reject(notificationResult.releaseException());
         return;
     }
 
-    auto notification = notificationResult.releaseReturnValue();
-    notification->showSoon();
+    if (auto* serviceWorkerGlobalScope = dynamicDowncast<ServiceWorkerGlobalScope>(context)) {
+        if (auto* pushEvent = serviceWorkerGlobalScope->pushEvent()) {
+            auto& globalObject = *JSC::jsCast<JSDOMGlobalObject*>(promise->globalObject());
+            auto& jsPromise = *JSC::jsCast<JSC::JSPromise*>(promise->promise());
+            pushEvent->waitUntil(DOMPromise::create(globalObject, jsPromise));
+        }
+    }
 
-    context.eventLoop().queueTask(TaskSource::DOMManipulation, [promise = WTFMove(promise)]() mutable {
-        promise.resolve();
+    auto notification = notificationResult.releaseReturnValue();
+    notification->show([promise = WTFMove(promise)]() mutable {
+        promise->resolve();
     });
 }
 

Source/WebCore/workers/service/ServiceWorkerRegistration.h

@@ -99,7 +99,7 @@ class ServiceWorkerRegistration final : public RefCounted<ServiceWorkerRegistrat
         String tag;
     };
 
-    void showNotification(ScriptExecutionContext&, String&& title, NotificationOptions&&, DOMPromiseDeferred<void>&&);
+    void showNotification(ScriptExecutionContext&, String&& title, NotificationOptions&&, Ref<DeferredPromise>&&);
     void getNotifications(const GetNotificationOptions& filter, DOMPromiseDeferred<IDLSequence<IDLInterface<Notification>>>);
 #endif
 

Source/WebCore/workers/service/context/ServiceWorkerThread.cpp

@@ -231,7 +231,7 @@ void ServiceWorkerThread::queueTaskToFirePushEvent(std::optional<Vector<uint8_t>
         serviceWorkerGlobalScope->setHasPendingSilentPushEvent(true);
 
         auto pushEvent = PushEvent::create(eventNames().pushEvent, { }, WTFMove(data), ExtendableEvent::IsTrusted::Yes);
-        serviceWorkerGlobalScope->dispatchEvent(pushEvent);
+        serviceWorkerGlobalScope->dispatchPushEvent(pushEvent);
 
         pushEvent->whenAllExtendLifetimePromisesAreSettled([serviceWorkerGlobalScope, eventCreationTime = pushEvent->timeStamp(), callback = WTFMove(callback)](auto&& extendLifetimePromises) mutable {
             bool hasRejectedAnyPromise = false;

Source/WebKit/ChangeLog

@@ -1,3 +1,29 @@
+2022-05-16  Youenn Fablet  <youenn@apple.com>
+
+        Make sure calling showNotification will extend the service worker lifetime
+        https://bugs.webkit.org/show_bug.cgi?id=240273
+        <rdar://92978482>
+
+        Reviewed by Chris Dumez.
+
+        On WebProcess side, implement the new NoficationClient::show API that takes a callback.
+        Delay calling this callback until UIProcess tells us so through async IPC.
+        On UIProcess side, take a callback and call it when the show notification steps are done.
+
+        * NetworkProcess/Notifications/NetworkNotificationManager.cpp:
+        * NetworkProcess/Notifications/NetworkNotificationManager.h:
+        * Shared/Notifications/NotificationManagerMessageHandler.h:
+        * Shared/Notifications/NotificationManagerMessageHandler.messages.in:
+        * UIProcess/Notifications/ServiceWorkerNotificationHandler.cpp:
+        * UIProcess/Notifications/ServiceWorkerNotificationHandler.h:
+        * UIProcess/Notifications/WebNotificationManagerMessageHandler.cpp:
+        * UIProcess/Notifications/WebNotificationManagerMessageHandler.h:
+        * WebProcess/GPU/webrtc/RemoteVideoFrameObjectHeapProxyProcessor.cpp:
+        * WebProcess/Notifications/WebNotificationManager.cpp:
+        * WebProcess/Notifications/WebNotificationManager.h:
+        * WebProcess/WebCoreSupport/WebNotificationClient.cpp:
+        * WebProcess/WebCoreSupport/WebNotificationClient.h:
+
 2022-05-16  Youenn Fablet  <youenn@apple.com>
 
         Add logging when taking a process assertion synchronously

Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.cpp

@@ -90,10 +90,9 @@ void NetworkNotificationManager::getPendingPushMessages(CompletionHandler<void(c
     sendMessageWithReply<WebPushD::MessageType::GetPendingPushMessages>(WTFMove(replyHandler));
 }
 
-void NetworkNotificationManager::showNotification(IPC::Connection&, const WebCore::NotificationData&)
+void NetworkNotificationManager::showNotification(IPC::Connection&, const WebCore::NotificationData&, CompletionHandler<void()>&& callback)
 {
-    if (!m_connection)
-        return;
+    callback();
 
 //     FIXME: While we don't normally land commented-out code in the tree,
 //     this is a nice bookmark for a development milestone; Roundtrip communication with webpushd

Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.h

@@ -70,7 +70,7 @@ class NetworkNotificationManager : public NotificationManagerMessageHandler {
     NetworkNotificationManager(NetworkSession&, const String& webPushMachServiceName, WebPushD::WebPushDaemonConnectionConfiguration&&);
 
     void requestSystemNotificationPermission(const String& originString, CompletionHandler<void(bool)>&&) final;
-    void showNotification(IPC::Connection&, const WebCore::NotificationData&) final;
+    void showNotification(IPC::Connection&, const WebCore::NotificationData&, CompletionHandler<void()>&&) final;
     void cancelNotification(const UUID& notificationID) final;
     void clearNotifications(const Vector<UUID>& notificationIDs) final;
     void didDestroyNotification(const UUID& notificationID) final;

Source/WebKit/Shared/Notifications/NotificationManagerMessageHandler.h

@@ -41,7 +41,7 @@ class NotificationManagerMessageHandler : public IPC::MessageReceiver {
     virtual ~NotificationManagerMessageHandler() = default;
 
     virtual void requestSystemNotificationPermission(const String& securityOrigin, CompletionHandler<void(bool)>&&) = 0;
-    virtual void showNotification(IPC::Connection&, const WebCore::NotificationData&) = 0;
+    virtual void showNotification(IPC::Connection&, const WebCore::NotificationData&, CompletionHandler<void()>&&) = 0;
     virtual void cancelNotification(const UUID& notificationID) = 0;
     virtual void clearNotifications(const Vector<UUID>& notificationIDs) = 0;
     virtual void didDestroyNotification(const UUID& notificationID) = 0;

Source/WebKit/Shared/Notifications/NotificationManagerMessageHandler.messages.in

@@ -22,7 +22,7 @@
 
 messages -> NotificationManagerMessageHandler NotRefCounted {
     RequestSystemNotificationPermission(String originIdentifier) -> (bool allowed)
-    ShowNotification(struct WebCore::NotificationData notificationData) WantsConnection
+    ShowNotification(struct WebCore::NotificationData notificationData) -> () WantsConnection
     CancelNotification(UUID notificationID)
     ClearNotifications(Vector<UUID> notificationIDs)
     DidDestroyNotification(UUID notificationID)

Source/WebKit/UIProcess/Notifications/ServiceWorkerNotificationHandler.cpp

@@ -28,6 +28,7 @@
 
 #include "WebsiteDataStore.h"
 #include <WebCore/NotificationData.h>
+#include <wtf/Scope.h>
 
 namespace WebKit {
 
@@ -52,8 +53,10 @@ WebsiteDataStore* ServiceWorkerNotificationHandler::dataStoreForNotificationID(c
     return WebsiteDataStore::existingDataStoreForSessionID(iterator->value);
 }
 
-void ServiceWorkerNotificationHandler::showNotification(IPC::Connection& connection, const WebCore::NotificationData& data)
+void ServiceWorkerNotificationHandler::showNotification(IPC::Connection& connection, const WebCore::NotificationData& data, CompletionHandler<void()>&& callback)
 {
+    auto scope = makeScopeExit([&callback] { callback(); });
+
     auto* dataStore = WebsiteDataStore::existingDataStoreForSessionID(data.sourceSession);
     if (!dataStore)
         return;

Source/WebKit/UIProcess/Notifications/ServiceWorkerNotificationHandler.h

@@ -38,7 +38,7 @@ class ServiceWorkerNotificationHandler final : public NotificationManagerMessage
 public:
     static ServiceWorkerNotificationHandler& singleton();
 
-    void showNotification(IPC::Connection&, const WebCore::NotificationData&) final;
+    void showNotification(IPC::Connection&, const WebCore::NotificationData&, CompletionHandler<void()>&&) final;
     void cancelNotification(const UUID& notificationID) final;
     void clearNotifications(const Vector<UUID>& notificationIDs) final;
     void didDestroyNotification(const UUID& notificationID) final;