Notification.requestPermission() should return a Promise.
Created attachment 428416 [details] Patch
Created attachment 428418 [details] Patch
Created attachment 428439 [details] Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Created attachment 428440 [details] Patch
Created attachment 428443 [details] Patch
Comment on attachment 428443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428443&action=review > Source/WebCore/Modules/notifications/NotificationClient.h:69 > + virtual void requestPermission(ScriptExecutionContext*, CompletionHandler<void(NotificationClient::Permission)>&&) = 0; This should take a ScriptExecutionContext&, not a pointer, since it’s required to be non-null. > Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.cpp:68 > + for (auto& completionHandler : completionHandlers) > + completionHandler(NotificationClient::Permission::Denied); Seems like we could use a helper function for this loop since it’s done twice. > Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.cpp:90 > + for (auto& completionHandler : completionHandlers) > + completionHandler(allowed ? NotificationClient::Permission::Granted : NotificationClient::Permission::Denied); Here is the second time that loop is done. > Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.h:52 > + void startRequest(const WebCore::SecurityOriginData&, CompletionHandler<void(WebCore::NotificationClient::Permission)>&&); A named type for the completion handler could possibly make the code less verbose. > Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.h:65 > + HashMap<WebCore::SecurityOriginData, Vector<CompletionHandler<void(WebCore::NotificationClient::Permission)>>> m_requestsPerOrigin; A named type for the vector could possibly make the code less verbose.
Comment on attachment 428443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428443&action=review >> Source/WebCore/Modules/notifications/NotificationClient.h:69 >> + virtual void requestPermission(ScriptExecutionContext*, CompletionHandler<void(NotificationClient::Permission)>&&) = 0; > > This should take a ScriptExecutionContext&, not a pointer, since it’s required to be non-null. Would really like you to do this one if you’re willing. >> Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.cpp:68 >> + completionHandler(NotificationClient::Permission::Denied); > > Seems like we could use a helper function for this loop since it’s done twice. Not sure this is such a great idea; I think you’ll have to decide if it’s really helpful. >> Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.cpp:90 >> + completionHandler(allowed ? NotificationClient::Permission::Granted : NotificationClient::Permission::Denied); > > Here is the second time that loop is done. Ditto. >> Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.h:52 >> + void startRequest(const WebCore::SecurityOriginData&, CompletionHandler<void(WebCore::NotificationClient::Permission)>&&); > > A named type for the completion handler could possibly make the code less verbose. Ditto. >> Source/WebKit/WebProcess/Notifications/NotificationPermissionRequestManager.h:65 >> + HashMap<WebCore::SecurityOriginData, Vector<CompletionHandler<void(WebCore::NotificationClient::Permission)>>> m_requestsPerOrigin; > > A named type for the vector could possibly make the code less verbose. Ditto.
Created attachment 428456 [details] Patch
Committed r277420 (237670@main): <https://commits.webkit.org/237670@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428456 [details].
<rdar://problem/77951307>