Hook up PushManager to notification permission state
Created attachment 452257 [details] Patch
<rdar://problem/89056721>
Created attachment 452302 [details] Patch
Comment on attachment 452302 [details] Patch This seems fine. Ping me when the bots are green and I'll give it the nit-pick look, but it'll be r+
Created attachment 452334 [details] Patch
Created attachment 452335 [details] Patch
Created attachment 452336 [details] Patch
Created attachment 452366 [details] Patch
Created attachment 452486 [details] Patch
Comment on attachment 452486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452486&action=review > Source/WebCore/Modules/push-api/PushManager.cpp:134 > + promise.reject(Exception { NotAllowedError, "User denied push permission"_s }); We may want to provide a clearer explanation here of WHY this is failing. > Source/WebCore/Modules/push-api/PushManager.cpp:139 > + // Ref { *this } triggers a compiler bug on wincairo where it tries to ref the lambda instead of PushManager. MSVC has trouble with nested lambdas. I think the trick to get this building in MSVC is to capture `protectedThis = WTFMove(protectedThis)` below instead of `protectedThis = Ref { *this }`. > Source/WebCore/Modules/push-api/PushManager.cpp:170 > + if (permission == NotificationPermission::Default) This screams for a switch statement IMO.
Comment on attachment 452486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452486&action=review R+ with review feedback addressed >> Source/WebCore/Modules/push-api/PushManager.cpp:139 >> + // Ref { *this } triggers a compiler bug on wincairo where it tries to ref the lambda instead of PushManager. > > MSVC has trouble with nested lambdas. I think the trick to get this building in MSVC is to capture `protectedThis = WTFMove(protectedThis)` below instead of `protectedThis = Ref { *this }`. I think the bot bubbles are all green, which is good. >> Source/WebCore/Modules/push-api/PushManager.cpp:170 >> + if (permission == NotificationPermission::Default) > > This screams for a switch statement IMO. Agreed: Switch plz
Comment on attachment 452486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452486&action=review >>> Source/WebCore/Modules/push-api/PushManager.cpp:139 >>> + // Ref { *this } triggers a compiler bug on wincairo where it tries to ref the lambda instead of PushManager. >> >> MSVC has trouble with nested lambdas. I think the trick to get this building in MSVC is to capture `protectedThis = WTFMove(protectedThis)` below instead of `protectedThis = Ref { *this }`. > > I think the bot bubbles are all green, which is good. Well, they are all green because of an ugly #if !PLATFORM(WIN_CAIRO)
Created attachment 452590 [details] Patch for landing
Found 1 new test failure: http/wpt/push-api/onpush-disabled.html
Created attachment 452617 [details] Patch for landing
Committed r290198 (247526@main): <https://commits.webkit.org/247526@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452617 [details].