- Revision
- 294726
- Author
- [email protected]
- Date
- 2022-05-23 21:00:18 -0700 (Mon, 23 May 2022)
Log Message
Fix webpushd crash when removing notification permissions
https://bugs.webkit.org/show_bug.cgi?id=240737
Reviewed by Chris Dumez and Geoffrey Garen.
When removing notification permissions for an origin, webpushd crashes because of a RELEASE_ASSERT
that fires in AppBundleRequest::start when INSTALL_COORDINATION_BUNDLES isn't enabled and mock
bundles aren't enabled.
Fix this by removing the calls into AppBundleRequest when INSTALL_COORDINATION_BUNDLES isn't enabled.
Those requests are only meant to run when that feature is enabled.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm: Only run the permission management test
when INSTALL_COORDINATION_BUNDLES is enabled (e.g. on iOS); that is the only time it makes sense
to test that codepath.
* Source/WebKit/webpushd/WebPushDaemon.h:
* Source/WebKit/webpushd/WebPushDaemon.mm:
(WebPushD::Daemon::deletePushRegistration):
(WebPushD::Daemon::deletePushAndNotificationRegistration):
Canonical link: https://commits.webkit.org/250904@main
Modified Paths
Diff
Modified: trunk/Source/WebKit/webpushd/WebPushDaemon.h (294725 => 294726)
--- trunk/Source/WebKit/webpushd/WebPushDaemon.h 2022-05-24 03:53:32 UTC (rev 294725)
+++ trunk/Source/WebKit/webpushd/WebPushDaemon.h 2022-05-24 04:00:18 UTC (rev 294726)
@@ -74,6 +74,7 @@
void echoTwice(ClientConnection*, const String&, CompletionHandler<void(const String&)>&& replySender);
void requestSystemNotificationPermission(ClientConnection*, const String&, CompletionHandler<void(bool)>&& replySender);
void getOriginsWithPushAndNotificationPermissions(ClientConnection*, CompletionHandler<void(const Vector<String>&)>&& replySender);
+ void deletePushRegistration(const String&, const String&, CompletionHandler<void()>&&);
void deletePushAndNotificationRegistration(ClientConnection*, const String& originString, CompletionHandler<void(const String&)>&& replySender);
void setDebugModeIsEnabled(ClientConnection*, bool);
void updateConnectionConfiguration(ClientConnection*, const WebPushDaemonConnectionConfiguration&);
Modified: trunk/Source/WebKit/webpushd/WebPushDaemon.mm (294725 => 294726)
--- trunk/Source/WebKit/webpushd/WebPushDaemon.mm 2022-05-24 03:53:32 UTC (rev 294725)
+++ trunk/Source/WebKit/webpushd/WebPushDaemon.mm 2022-05-24 04:00:18 UTC (rev 294726)
@@ -505,6 +505,20 @@
#endif
}
+void Daemon::deletePushRegistration(const String& bundleIdentifier, const String& originString, CompletionHandler<void()>&& callback)
+{
+ runAfterStartingPushService([this, bundleIdentifier, originString, callback = WTFMove(callback)]() mutable {
+ if (!m_pushService) {
+ callback();
+ return;
+ }
+
+ m_pushService->removeRecordsForBundleIdentifierAndOrigin(bundleIdentifier, originString, [callback = WTFMove(callback)](auto&&) mutable {
+ callback();
+ });
+ });
+}
+
void Daemon::deletePushAndNotificationRegistration(ClientConnection* connection, const String& originString, CompletionHandler<void(const String&)>&& replySender)
{
if (!canRegisterForNotifications(*connection)) {
@@ -512,18 +526,17 @@
return;
}
+#if ENABLE(INSTALL_COORDINATION_BUNDLES)
connection->enqueueAppBundleRequest(makeUnique<AppBundleDeletionRequest>(*connection, originString, [this, originString = String { originString }, replySender = WTFMove(replySender), bundleIdentifier = connection->hostAppCodeSigningIdentifier()](auto result) mutable {
- runAfterStartingPushService([this, bundleIdentifier = WTFMove(bundleIdentifier), originString = WTFMove(originString), replySender = WTFMove(replySender), result]() mutable {
- if (!m_pushService) {
- replySender(result);
- return;
- }
-
- m_pushService->removeRecordsForBundleIdentifierAndOrigin(bundleIdentifier, originString, [replySender = WTFMove(replySender), result](auto&&) mutable {
- replySender(result);
- });
+ deletePushRegistration(bundleIdentifier, originString, [replySender = WTFMove(replySender), result]() mutable {
+ replySender(result);
});
}));
+#else
+ deletePushRegistration(connection->hostAppCodeSigningIdentifier(), originString, [replySender = WTFMove(replySender)]() mutable {
+ replySender(emptyString());
+ });
+#endif
}
void Daemon::setDebugModeIsEnabled(ClientConnection* clientConnection, bool enabled)
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm (294725 => 294726)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm 2022-05-24 03:53:32 UTC (rev 294725)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm 2022-05-24 04:00:18 UTC (rev 294726)
@@ -322,67 +322,6 @@
cleanUpTestWebPushD(tempDir);
}
-TEST(WebPushD, PermissionManagement)
-{
- NSURL *tempDirectory = setUpTestWebPushD();
-
- auto dataStoreConfiguration = adoptNS([_WKWebsiteDataStoreConfiguration new]);
- dataStoreConfiguration.get().webPushMachServiceName = @"org.webkit.webpushtestdaemon.service";
- dataStoreConfiguration.get().webPushDaemonUsesMockBundlesForTesting = YES;
- auto dataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:dataStoreConfiguration.get()]);
-
- auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
- configuration.get().websiteDataStore = dataStore.get();
- [configuration.get().preferences _setNotificationsEnabled:YES];
- for (_WKExperimentalFeature *feature in [WKPreferences _experimentalFeatures]) {
- if ([feature.key isEqualToString:@"BuiltInNotificationsEnabled"])
- [[configuration preferences] _setEnabled:YES forFeature:feature];
- }
-
- auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
- auto uiDelegate = adoptNS([[NotificationPermissionDelegate alloc] init]);
- [webView setUIDelegate:uiDelegate.get()];
- [webView synchronouslyLoadHTMLString:@"" baseURL:[NSURL URLWithString:@"https://example.org"]];
- [webView evaluateJavaScript:@"Notification.requestPermission().then(() => { alert('done') })" completionHandler:nil];
- TestWebKitAPI::Util::run(&alertReceived);
-
- static bool originOperationDone = false;
- static RetainPtr<WKSecurityOrigin> origin;
- [dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
- EXPECT_EQ([origins count], 1u);
- origin = [origins anyObject];
- originOperationDone = true;
- }];
-
- TestWebKitAPI::Util::run(&originOperationDone);
-
- EXPECT_WK_STREQ(origin.get().protocol, "https");
- EXPECT_WK_STREQ(origin.get().host, "example.org");
-
- // If we failed to retrieve an expected origin, we will have failed the above checks
- if (!origin) {
- cleanUpTestWebPushD(tempDirectory);
- return;
- }
-
- originOperationDone = false;
- [dataStore _deletePushAndNotificationRegistration:origin.get() completionHandler:^(NSError *error) {
- EXPECT_FALSE(!!error);
- originOperationDone = true;
- }];
-
- TestWebKitAPI::Util::run(&originOperationDone);
-
- originOperationDone = false;
- [dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
- EXPECT_EQ([origins count], 0u);
- originOperationDone = true;
- }];
- TestWebKitAPI::Util::run(&originOperationDone);
-
- cleanUpTestWebPushD(tempDirectory);
-}
-
static void clearWebsiteDataStore(WKWebsiteDataStore *store)
{
__block bool clearedStore = false;
@@ -938,6 +877,67 @@
#if ENABLE(INSTALL_COORDINATION_BUNDLES)
#if USE(APPLE_INTERNAL_SDK)
+TEST(WebPushD, PermissionManagement)
+{
+ NSURL *tempDirectory = setUpTestWebPushD();
+
+ auto dataStoreConfiguration = adoptNS([_WKWebsiteDataStoreConfiguration new]);
+ dataStoreConfiguration.get().webPushMachServiceName = @"org.webkit.webpushtestdaemon.service";
+ dataStoreConfiguration.get().webPushDaemonUsesMockBundlesForTesting = YES;
+ auto dataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:dataStoreConfiguration.get()]);
+
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ configuration.get().websiteDataStore = dataStore.get();
+ [configuration.get().preferences _setNotificationsEnabled:YES];
+ for (_WKExperimentalFeature *feature in [WKPreferences _experimentalFeatures]) {
+ if ([feature.key isEqualToString:@"BuiltInNotificationsEnabled"])
+ [[configuration preferences] _setEnabled:YES forFeature:feature];
+ }
+
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
+ auto uiDelegate = adoptNS([[NotificationPermissionDelegate alloc] init]);
+ [webView setUIDelegate:uiDelegate.get()];
+ [webView synchronouslyLoadHTMLString:@"" baseURL:[NSURL URLWithString:@"https://example.org"]];
+ [webView evaluateJavaScript:@"Notification.requestPermission().then(() => { alert('done') })" completionHandler:nil];
+ TestWebKitAPI::Util::run(&alertReceived);
+
+ static bool originOperationDone = false;
+ static RetainPtr<WKSecurityOrigin> origin;
+ [dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
+ EXPECT_EQ([origins count], 1u);
+ origin = [origins anyObject];
+ originOperationDone = true;
+ }];
+
+ TestWebKitAPI::Util::run(&originOperationDone);
+
+ EXPECT_WK_STREQ(origin.get().protocol, "https");
+ EXPECT_WK_STREQ(origin.get().host, "example.org");
+
+ // If we failed to retrieve an expected origin, we will have failed the above checks
+ if (!origin) {
+ cleanUpTestWebPushD(tempDirectory);
+ return;
+ }
+
+ originOperationDone = false;
+ [dataStore _deletePushAndNotificationRegistration:origin.get() completionHandler:^(NSError *error) {
+ EXPECT_FALSE(!!error);
+ originOperationDone = true;
+ }];
+
+ TestWebKitAPI::Util::run(&originOperationDone);
+
+ originOperationDone = false;
+ [dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
+ EXPECT_EQ([origins count], 0u);
+ originOperationDone = true;
+ }];
+ TestWebKitAPI::Util::run(&originOperationDone);
+
+ cleanUpTestWebPushD(tempDirectory);
+}
+
static void deleteAllRegistrationsForDataStore(WKWebsiteDataStore *dataStore)
{
__block bool originOperationDone = false;
