| Summary: | Remove push subscriptions when associated service worker registrations are removed | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ben Nham <nham> | ||||||
| Component: | New Bugs | Assignee: | Ben Nham <nham> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | beidson, nham, webkit-bug-importer, youennf | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 238068 | ||||||||
| Attachments: |
|
||||||||
|
Description
Ben Nham
2022-03-16 14:48:47 PDT
Created attachment 454899 [details]
Patch
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. Created attachment 455104 [details]
Patch for landing
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]. |