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

Disable telemetry for known notifications #28677

Conversation

pvollan
Copy link
Contributor

@pvollan pvollan commented May 16, 2024

00575a1

Disable telemetry for known notifications
https://bugs.webkit.org/show_bug.cgi?id=274279
rdar://128213400

Reviewed by Sihui Liu.

Disable telemetry for notifications that we know are being observed in the WebContent process.
For in-process notifications, we do not need to forward them from the UI process.

* Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb.in:
* Source/WebKit/Scripts/process-entitlements.sh:
* Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::registerNotificationObservers):
* Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:

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

632a68e

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2 βœ… πŸ§ͺ wincairo-tests
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ webkitpy βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ›  wpe-cairo
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2 ❌ πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@pvollan pvollan self-assigned this May 16, 2024
@pvollan pvollan added the WebKit Process Model Bugs related to WebKit's multi-process architecture label May 16, 2024
@pvollan pvollan requested review from cdumez, bnham and szewai May 16, 2024 20:49
plistbuddy Add :com.apple.private.darwin-notification.introspect:17 string com.apple.system.lowpowermode
plistbuddy Add :com.apple.private.darwin-notification.introspect:18 string com.apple.system.timezone
plistbuddy Add :com.apple.private.darwin-notification.introspect:19 string com.apple.zoomwindow
plistbuddy Add :com.apple.private.darwin-notification.introspect:8 string com.apple.accessibility.cache.guided.access
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems not in const Vector<ASCIILiteral> notificationMessages, is it forwarded?

@cdumez
Copy link
Contributor

cdumez commented May 20, 2024

@pvollan Any update on this?

@pvollan
Copy link
Contributor Author

pvollan commented May 20, 2024

@pvollan Any update on this?

Ah, sorry for the delayed response. This is actually a notification that is used only in-process, so I don't believe we need to forward this one. And the sandbox change should not be required either, then. I will update the patch.

Thanks for reviewing, all!

@pvollan pvollan force-pushed the eng/Disable-telemetry-for-known-notifications branch from 576baac to 8cd5ddf Compare May 21, 2024 01:21
@pvollan pvollan force-pushed the eng/Disable-telemetry-for-known-notifications branch from 8cd5ddf to 632a68e Compare May 21, 2024 01:35
@pvollan pvollan requested a review from szewai May 21, 2024 01:35
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 21, 2024
@cdumez cdumez added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels May 21, 2024
https://bugs.webkit.org/show_bug.cgi?id=274279
rdar://128213400

Reviewed by Sihui Liu.

Disable telemetry for notifications that we know are being observed in the WebContent process.
For in-process notifications, we do not need to forward them from the UI process.

* Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb.in:
* Source/WebKit/Scripts/process-entitlements.sh:
* Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:
(WebKit::WebProcessPool::registerNotificationObservers):
* Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:

Canonical link: https://commits.webkit.org/279065@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Disable-telemetry-for-known-notifications branch from 632a68e to 00575a1 Compare May 21, 2024 18:04
@webkit-commit-queue
Copy link
Collaborator

Committed 279065@main (00575a1): https://commits.webkit.org/279065@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 21, 2024
@webkit-commit-queue webkit-commit-queue merged commit 00575a1 into WebKit:main May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Process Model Bugs related to WebKit's multi-process architecture
Projects
None yet
6 participants