Skip to content

Commit 6e7d7b2

Browse files
committed
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. 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
1 parent 103005c commit 6e7d7b2

31 files changed

+221
-55
lines changed

Source/WebCore/ChangeLog

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,30 @@
1+
2022-05-16 Youenn Fablet <[email protected]>
2+
3+
Make sure calling showNotification will extend the service worker lifetime
4+
https://bugs.webkit.org/show_bug.cgi?id=240273
5+
<rdar://92978482>
6+
7+
Reviewed by Chris Dumez.
8+
9+
Update NotificationClient API so that show is taking a completion handler.
10+
Make use of this completion handler to resolve the promise when the show notification steps are done, as per spec.
11+
Register push event to ServiceWorkerGlobalScope when the event handlers are called.
12+
When ServiceWorkerRegistration::show is called during one of the push event handlers,
13+
extend the push event lifetime by adding the show notification promise to the push event.
14+
15+
Covered by API test.
16+
17+
* Modules/notifications/Notification.cpp:
18+
* Modules/notifications/Notification.h:
19+
* Modules/notifications/NotificationClient.h:
20+
* dom/ScriptExecutionContext.cpp:
21+
* dom/ScriptExecutionContext.h:
22+
* workers/service/ServiceWorkerGlobalScope.cpp:
23+
* workers/service/ServiceWorkerGlobalScope.h:
24+
* workers/service/ServiceWorkerRegistration.cpp:
25+
* workers/service/ServiceWorkerRegistration.h:
26+
* workers/service/context/ServiceWorkerThread.cpp:
27+
128
2022-05-16 Martin Robinson <[email protected]>
229

330
Do not allow unitless values for CSS unprefixed perspective property

Source/WebCore/Modules/notifications/Notification.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include "WindowFocusAllowedIndicator.h"
5050
#include <wtf/CompletionHandler.h>
5151
#include <wtf/IsoMallocInlines.h>
52+
#include <wtf/Scope.h>
5253

5354
namespace WebCore {
5455

@@ -143,17 +144,23 @@ void Notification::markAsShown()
143144
m_state = Showing;
144145
}
145146

146-
void Notification::show()
147+
void Notification::show(CompletionHandler<void()>&& callback)
147148
{
149+
CompletionHandlerCallingScope scope { WTFMove(callback) };
150+
148151
// prevent double-showing
149152
if (m_state != Idle)
150153
return;
151154

152-
auto* client = clientFromContext();
155+
auto* context = scriptExecutionContext();
156+
if (!context)
157+
return;
158+
159+
auto* client = context->notificationClient();
153160
if (!client)
154161
return;
155162

156-
if (client->checkPermission(scriptExecutionContext()) != Permission::Granted) {
163+
if (client->checkPermission(context) != Permission::Granted) {
157164
switch (m_notificationSource) {
158165
case NotificationSource::Document:
159166
dispatchErrorEvent();
@@ -163,11 +170,10 @@ void Notification::show()
163170
// If permission has since been revoked, then silently failing here is expected behavior.
164171
break;
165172
}
166-
167173
return;
168174
}
169175

170-
if (client->show(*this))
176+
if (client->show(*this, scope.release()))
171177
m_state = Showing;
172178
}
173179

Source/WebCore/Modules/notifications/Notification.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include "NotificationPermission.h"
4040
#include "ScriptExecutionContextIdentifier.h"
4141
#include "SerializedScriptValue.h"
42+
#include <wtf/CompletionHandler.h>
4243
#include <wtf/URL.h>
4344
#include <wtf/UUID.h>
4445
#include "WritingMode.h"
@@ -74,7 +75,7 @@ class Notification final : public ThreadSafeRefCounted<Notification>, public Act
7475

7576
WEBCORE_EXPORT virtual ~Notification();
7677

77-
void show();
78+
void show(CompletionHandler<void()>&& = [] { });
7879
void close();
7980

8081
const String& title() const { return m_title; }

Source/WebCore/Modules/notifications/NotificationClient.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class NotificationClient {
4747
using PermissionHandler = CompletionHandler<void(Permission)>;
4848

4949
// Requests that a notification be shown.
50-
virtual bool show(Notification&) = 0;
50+
virtual bool show(Notification&, CompletionHandler<void()>&&) = 0;
5151

5252
// Requests that a notification that has already been shown be canceled.
5353
virtual void cancel(Notification&) = 0;

Source/WebCore/dom/ScriptExecutionContext.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ ScriptExecutionContext::~ScriptExecutionContext()
176176
m_inScriptExecutionContextDestructor = true;
177177
#endif // ASSERT_ENABLED
178178

179+
auto callbacks = WTFMove(m_notificationCallbacks);
180+
for (auto& callback : callbacks.values())
181+
callback();
182+
179183
#if ENABLE(SERVICE_WORKER)
180184
setActiveServiceWorker(nullptr);
181185
#endif
@@ -767,4 +771,16 @@ ScriptExecutionContext::HasResourceAccess ScriptExecutionContext::canAccessResou
767771
RELEASE_ASSERT_NOT_REACHED();
768772
}
769773

774+
ScriptExecutionContext::NotificationCallbackIdentifier ScriptExecutionContext::addNotificationCallback(CompletionHandler<void()>&& callback)
775+
{
776+
auto identifier = NotificationCallbackIdentifier::generateThreadSafe();
777+
m_notificationCallbacks.add(identifier, WTFMove(callback));
778+
return identifier;
779+
}
780+
781+
CompletionHandler<void()> ScriptExecutionContext::takeNotificationCallback(NotificationCallbackIdentifier identifier)
782+
{
783+
return m_notificationCallbacks.take(identifier);
784+
}
785+
770786
} // namespace WebCore

Source/WebCore/dom/ScriptExecutionContext.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,12 @@ class ScriptExecutionContext : public SecurityContext, public CanMakeWeakPtr<Scr
314314
enum class HasResourceAccess : uint8_t { No, Yes, DefaultForThirdParty };
315315
WEBCORE_EXPORT HasResourceAccess canAccessResource(ResourceType) const;
316316

317+
enum NotificationCallbackIdentifierType { };
318+
using NotificationCallbackIdentifier = ObjectIdentifier<NotificationCallbackIdentifierType>;
319+
320+
WEBCORE_EXPORT NotificationCallbackIdentifier addNotificationCallback(CompletionHandler<void()>&&);
321+
WEBCORE_EXPORT CompletionHandler<void()> takeNotificationCallback(NotificationCallbackIdentifier);
322+
317323
protected:
318324
class AddConsoleMessageTask : public Task {
319325
public:
@@ -396,6 +402,8 @@ class ScriptExecutionContext : public SecurityContext, public CanMakeWeakPtr<Scr
396402

397403
bool m_hasLoggedAuthenticatedEncryptionWarning { false };
398404
StorageBlockingPolicy m_storageBlockingPolicy { StorageBlockingPolicy::AllowAll };
405+
406+
HashMap<NotificationCallbackIdentifier, CompletionHandler<void()>> m_notificationCallbacks;
399407
};
400408

401409
} // namespace WebCore

Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "FrameLoaderClient.h"
3838
#include "JSDOMPromiseDeferred.h"
3939
#include "NotificationEvent.h"
40+
#include "PushEvent.h"
4041
#include "SWContextManager.h"
4142
#include "ServiceWorker.h"
4243
#include "ServiceWorkerClient.h"
@@ -78,6 +79,14 @@ ServiceWorkerGlobalScope::~ServiceWorkerGlobalScope()
7879
callOnMainThread([notificationClient = WTFMove(m_notificationClient)] { });
7980
}
8081

82+
void ServiceWorkerGlobalScope::dispatchPushEvent(PushEvent& pushEvent)
83+
{
84+
ASSERT(!m_pushEvent);
85+
m_pushEvent = &pushEvent;
86+
dispatchEvent(pushEvent);
87+
m_pushEvent = nullptr;
88+
}
89+
8190
void ServiceWorkerGlobalScope::notifyServiceWorkerPageOfCreationIfNecessary()
8291
{
8392
auto serviceWorkerPage = this->serviceWorkerPage();

Source/WebCore/workers/service/ServiceWorkerGlobalScope.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ namespace WebCore {
4040
class DeferredPromise;
4141
class ExtendableEvent;
4242
class Page;
43+
class PushEvent;
4344
class ServiceWorkerClient;
4445
class ServiceWorkerClients;
4546
class ServiceWorkerThread;
@@ -81,6 +82,9 @@ class ServiceWorkerGlobalScope final : public WorkerGlobalScope {
8182

8283
WEBCORE_EXPORT Page* serviceWorkerPage();
8384

85+
void dispatchPushEvent(PushEvent&);
86+
PushEvent* pushEvent() { return m_pushEvent.get(); }
87+
8488
bool hasPendingSilentPushEvent() const { return m_hasPendingSilentPushEvent; }
8589
void setHasPendingSilentPushEvent(bool value) { m_hasPendingSilentPushEvent = value; }
8690

@@ -112,6 +116,7 @@ class ServiceWorkerGlobalScope final : public WorkerGlobalScope {
112116
bool m_hasPendingSilentPushEvent { false };
113117
bool m_isProcessingUserGesture { false };
114118
Timer m_userGestureTimer;
119+
RefPtr<PushEvent> m_pushEvent;
115120
};
116121

117122
} // namespace WebCore

Source/WebCore/workers/service/ServiceWorkerRegistration.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,14 @@
3232
#include "Event.h"
3333
#include "EventLoop.h"
3434
#include "EventNames.h"
35+
#include "JSDOMPromise.h"
3536
#include "JSDOMPromiseDeferred.h"
3637
#include "JSNotification.h"
3738
#include "Logging.h"
3839
#include "NavigationPreloadManager.h"
3940
#include "NotificationClient.h"
4041
#include "NotificationPermission.h"
42+
#include "PushEvent.h"
4143
#include "ServiceWorker.h"
4244
#include "ServiceWorkerContainer.h"
4345
#include "ServiceWorkerGlobalScope.h"
@@ -274,40 +276,44 @@ NavigationPreloadManager& ServiceWorkerRegistration::navigationPreload()
274276
}
275277

276278
#if ENABLE(NOTIFICATION_EVENT)
277-
void ServiceWorkerRegistration::showNotification(ScriptExecutionContext& context, String&& title, NotificationOptions&& options, DOMPromiseDeferred<void>&& promise)
279+
void ServiceWorkerRegistration::showNotification(ScriptExecutionContext& context, String&& title, NotificationOptions&& options, Ref<DeferredPromise>&& promise)
278280
{
279281
if (!m_activeWorker) {
280-
promise.reject(Exception { TypeError, "Registration does not have an active worker"_s });
282+
promise->reject(Exception { TypeError, "Registration does not have an active worker"_s });
281283
return;
282284
}
283285

284286
auto* client = context.notificationClient();
285287
if (!client) {
286-
promise.reject(Exception { TypeError, "Registration not active"_s });
288+
promise->reject(Exception { TypeError, "Registration not active"_s });
287289
return;
288290
}
289291

290292
if (client->checkPermission(&context) != NotificationPermission::Granted) {
291-
promise.reject(Exception { TypeError, "Registration does not have permission to show notifications"_s });
293+
promise->reject(Exception { TypeError, "Registration does not have permission to show notifications"_s });
292294
return;
293295
}
294296

295297
if (context.isServiceWorkerGlobalScope())
296298
downcast<ServiceWorkerGlobalScope>(context).setHasPendingSilentPushEvent(false);
297299

298-
// The Notification is kept alive by virtue of being show()'n soon.
299-
// FIXME: When implementing getNotifications(), store this Notification in the registration's notification list.
300300
auto notificationResult = Notification::createForServiceWorker(context, WTFMove(title), WTFMove(options), m_registrationData.scopeURL);
301301
if (notificationResult.hasException()) {
302-
promise.reject(notificationResult.releaseException());
302+
promise->reject(notificationResult.releaseException());
303303
return;
304304
}
305305

306-
auto notification = notificationResult.releaseReturnValue();
307-
notification->showSoon();
306+
if (auto* serviceWorkerGlobalScope = dynamicDowncast<ServiceWorkerGlobalScope>(context)) {
307+
if (auto* pushEvent = serviceWorkerGlobalScope->pushEvent()) {
308+
auto& globalObject = *JSC::jsCast<JSDOMGlobalObject*>(promise->globalObject());
309+
auto& jsPromise = *JSC::jsCast<JSC::JSPromise*>(promise->promise());
310+
pushEvent->waitUntil(DOMPromise::create(globalObject, jsPromise));
311+
}
312+
}
308313

309-
context.eventLoop().queueTask(TaskSource::DOMManipulation, [promise = WTFMove(promise)]() mutable {
310-
promise.resolve();
314+
auto notification = notificationResult.releaseReturnValue();
315+
notification->show([promise = WTFMove(promise)]() mutable {
316+
promise->resolve();
311317
});
312318
}
313319

Source/WebCore/workers/service/ServiceWorkerRegistration.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ class ServiceWorkerRegistration final : public RefCounted<ServiceWorkerRegistrat
9999
String tag;
100100
};
101101

102-
void showNotification(ScriptExecutionContext&, String&& title, NotificationOptions&&, DOMPromiseDeferred<void>&&);
102+
void showNotification(ScriptExecutionContext&, String&& title, NotificationOptions&&, Ref<DeferredPromise>&&);
103103
void getNotifications(const GetNotificationOptions& filter, DOMPromiseDeferred<IDLSequence<IDLInterface<Notification>>>);
104104
#endif
105105

0 commit comments

Comments
 (0)