Bug 237983 – Remove push subscriptions when associated service worker registrations are removed

• Component: New Bugs
• Status: RESOLVED FIXED

[Ben Nham]

Remove push subscriptions when associated service worker registrations are removed

[Ben Nham]

Created attachment 454899 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/90394173>

[youenn fablet]

Comment on attachment 454899 [details]
Patch

LGTM.
I am wondering whether there is a chance that things get dysinchronized, say the SQL service worker is damaged so now we loose all registrations but keep some push subscriptions. Maybe there should be something ensuring everything is fine.
For instance, if we push a message and there is no corresponding service worker registration, we should probably remove the corresponding push subscription.

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

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2335
> +        session->notificationManager().getPushSubscription(WTFMove(scopeURL), [callback = WTFMove(callback)](auto &&result) mutable {

Preexisting but I would not expect getPushSubscription to require a URL&&.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:2341
> +            callback(true);

Could be written as callback(result && !!result.value()) ?

> Source/WebKit/webpushd/PushService.mm:394
> +                    RELEASE_LOG(Push, "PushSubscription.unsubscribe(bundleID: %{public}s, scope: %{sensitive}s) failed with domain: %{public}s code: %lld)", m_bundleIdentifier.utf8().data(), m_scope.utf8().data(), error.domain.UTF8String ?: "none", static_cast<int64_t>(error.code));

RELEASE_LOG_IF which will not require the RELEASE_LOG_DISABLED around the if.

[Ben Nham]

Created attachment 455104 [details]
Patch for landing

[EWS]

Committed r291492 (248604@main): <https://commits.webkit.org/248604@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455104 [details].