Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PushManager in ephemeral sessions #1744

Merged

Conversation

bnham
Copy link
Contributor

@bnham bnham commented Jun 23, 2022

0570dfb

Fix PushManager in ephemeral sessions
https://bugs.webkit.org/show_bug.cgi?id=241934

Reviewed by Geoffrey Garen.

PushManager does not behave correctly when invoked in an ephemeral session. For instance:

 - PushManager.subscribe and PushManager.getSubscription fail with an AbortError.
 - PushManager.permissionState returns the persistent session's notification permissions for the
   given origin.

This patch makes PushManager in ephemeral sessions behave as if a user denied notification
permissions for all origins (which users can already do in persistent sessions via the appropriate
checkbox). This also prevents these APIs from being used to detect private browsing mode.

* LayoutTests/http/tests/push-api/permissions-ephemeral-expected.txt: Added.
* LayoutTests/http/tests/push-api/permissions-ephemeral.html: Added.
* LayoutTests/http/tests/push-api/resources/subscribe-tests.js
* LayoutTests/http/tests/push-api/resources/subscribe-worker.js
* LayoutTests/platform/gtk/TestExpectations:
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/platform/win/TestExpectations:
* Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.cpp:
(WebKit::NetworkNotificationManager::getPushSubscription):
(WebKit::NetworkNotificationManager::getPushPermissionState):
* Source/WebKit/WebProcess/WebCoreSupport/WebNotificationClient.cpp:
(WebKit::WebNotificationClient::requestPermission):
(WebKit::WebNotificationClient::checkPermission):

Canonical link: https://commits.webkit.org/251831@main

@bnham bnham self-assigned this Jun 23, 2022
@bnham bnham added WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). WebKit Nightly Build labels Jun 23, 2022
Copy link
Contributor

@geoffreygaren geoffreygaren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jun 23, 2022
@bnham bnham removed the merging-blocked Applied to prevent a change from being merged label Jun 23, 2022
@bnham bnham force-pushed the eng/pushmanager-ephemeral branch from c1d1be7 to ed3444d Compare June 23, 2022 23:23
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jun 24, 2022
@bnham bnham removed the merging-blocked Applied to prevent a change from being merged label Jun 24, 2022
@bnham bnham force-pushed the eng/pushmanager-ephemeral branch 2 times, most recently from 6cc8e54 to c23a0eb Compare June 24, 2022 17:33
@bnham bnham added the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2022
https://bugs.webkit.org/show_bug.cgi?id=241934

Reviewed by Geoffrey Garen.

PushManager does not behave correctly when invoked in an ephemeral session. For instance:

 - PushManager.subscribe and PushManager.getSubscription fail with an AbortError.
 - PushManager.permissionState returns the persistent session's notification permissions for the
   given origin.

This patch makes PushManager in ephemeral sessions behave as if a user denied notification
permissions for all origins (which users can already do in persistent sessions via the appropriate
checkbox). This also prevents these APIs from being used to detect private browsing mode.

* LayoutTests/http/tests/push-api/permissions-ephemeral-expected.txt: Added.
* LayoutTests/http/tests/push-api/permissions-ephemeral.html: Added.
* LayoutTests/http/tests/push-api/resources/subscribe-tests.js
* LayoutTests/http/tests/push-api/resources/subscribe-worker.js
* LayoutTests/platform/gtk/TestExpectations:
* LayoutTests/platform/ios/TestExpectations:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/platform/win/TestExpectations:
* Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.cpp:
(WebKit::NetworkNotificationManager::getPushSubscription):
(WebKit::NetworkNotificationManager::getPushPermissionState):
* Source/WebKit/WebProcess/WebCoreSupport/WebNotificationClient.cpp:
(WebKit::WebNotificationClient::requestPermission):
(WebKit::WebNotificationClient::checkPermission):

Canonical link: https://commits.webkit.org/251831@main
@webkit-commit-queue
Copy link
Collaborator

Committed 251831@main (0570dfb): https://commits.webkit.org/251831@main

Reviewed commits have been landed. Closing PR #1744 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).
Projects
None yet
4 participants