Repository navigation
Conversation
pasrom
left a comment
pasrom
left a comment
There was a problem hiding this comment.
Thanks, this is careful work. I read through the detection and tap paths and it holds up: only com.apple.WebKit.GPU processes are matched, and a hit counts only when macOS holds Safari (or STP) responsible for it, so Mail, other WKWebView hosts and Chrome fall through. requiresRecordingConsent is carried over from browserMeetings, so the consent prompt, cooldown and deny list apply as for Chromium. The meeting carries Safari's main PID, so the existing ProcessResponsibility.tapPIDs widening picks up the WebKit services. End detection follows the GPU process's input. The checkOnce / isMeetingActive rewrite also looks behaviour-preserving for the existing mic apps, and the tests pin the ownership check, the consent flag, the tap PID and the reset-before-pick ordering. It's also good that the STP bundle ID is labelled as assumed rather than measured.
One change I'd like before merge: the App Store build. Details are inline on patterns(watching:); in short, Safari detection can never fire there, but turning the browser toggle on now makes the detector enumerate Core Audio processes every poll, and the Settings caption promises Safari detection.
Smaller points:
- Users who already have Browser Web Meetings on get Safari detection on upgrade, plus the per-poll Core Audio enumeration. That's fine since consent gates recording, but worth a line in the release notes.
- Not covered and not stated anywhere: a Safari web app ("Add to Dock", its own
com.apple.Safari.WebApp.*bundle that owns its GPU process), other WebKit browsers such as Orion, and a Safari launched from a shell, which gets attributed to the launcher. A sentence indetection.mdwould set expectations. - The PR body says 17 tests;
MicInputDetectorSafariTestshas 16. Anddocs/plans/2026-08-11-generic-browser-meeting-detection.mdstill says "No Firefox or Safari meeting detection"; it's a historical RFC, but a one-line note would avoid confusion. - Housekeeping, not your fault since both rules landed on
mainafter you opened this: CONTRIBUTING now asks for noCo-Authored-By:trailer from AI tools (a new CI check flags it), with the model named in the PR description instead, and the PR template gained a Tests section (the line you reverted and the test that went red). It would be great to pick both up when you rebase.
CI on this PR has so far only run the title check; the build and tests will run once the workflows are approved.
| static func patterns(watching watchedAppNames: [String], customBundleIDs: [String] = []) -> [MicPattern] { | ||
| let watched = Set(watchedAppNames) | ||
| let builtIns = defaultPatterns.filter { watched.contains($0.appName) } | ||
| let browsers = watched.contains(AppMeetingPattern.browserMeetings.appName) ? safariPatterns : [] |
There was a problem hiding this comment.
In the App Store build this can never produce a meeting: ProcessResponsibility.responsibleFn is nil under APPSTORE, so responsiblePID returns 0, systemResponsibleApp returns nil, and claim() never accepts a GPU process. But patterns is now non-empty whenever Browser Web Meetings is on, so checkOnce no longer takes the patterns.isEmpty early return. It enumerates Core Audio processes every poll and logs each unmatched mic-holding bundle once per session, all for nothing.
Suggest compiling the Safari patterns out there, e.g.
#if APPSTORE
let browsers: [MicPattern] = []
#else
let browsers = watched.contains(AppMeetingPattern.browserMeetings.appName) ? safariPatterns : []
#endifThe Build Variants table in the root CLAUDE.md could then gain a "Safari meeting detection" row next to "Safari call audio".
| Detects web meetings (Google Meet, Whereby, web Zoom/Teams) by the WebRTC \ | ||
| signal, so any browser works. Other apps that place calls can trigger it too; \ | ||
| it always asks before recording, and "Never for this app" stops one for good. | ||
| signal in Chromium browsers and by microphone use in Safari. Other apps that \ |
There was a problem hiding this comment.
Related to the App Store point on patterns(watching:): this caption, and the README line, promise Safari detection unconditionally. With the patterns compiled out under APPSTORE, a caption variant there would keep the sandboxed build from advertising something it can't do.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #920 +/- ##
=====================================
Coverage 91.6% 91.6%
=====================================
Files 304 304
Lines 23674 23730 +56
Branches 6 6
=====================================
+ Hits 21694 21748 +54
- Misses 1976 1978 +2
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Browser meetings were detected only by the "WebRTC has active
PeerConnections" power assertion, which comes from Chromium's content
layer. WebKit never creates it, so a Google Meet call in Safari went
unseen. Measured during a Meet call in Safari on macOS 27.0.1:
com.apple.WebKit.GPU runs mic input for the whole call, the same with
one participant or two, keeps running it while muted, and stops when the
user leaves with the tab still open. Safari's only power assertions
("WebKit Media Playback") look the same as YouTube's, so the mic is the
signal.
MicInputDetector gains Safari and Safari Technology Preview patterns,
selected by the existing Browser Web Meetings toggle rather than a new
setting. The GPU service is shared with every app that embeds a
WKWebView, so a hit counts only when ProcessResponsibility attributes
the GPU process to Safari. The meeting carries Safari's main PID, which
the recorder already widens to the WebKit services carrying the audio.
The identity is "Safari" and requires consent, so the prompt, its
cooldown and "Never for this app" work as they do for a Chromium fork.
The deny list is now wired into MicInputDetector too.
A parked Safari prompt re-confirms on every poll, so checkOnce now
follows the assertion channel's rule (issue pasrom#814): counters of apps with
no hit are reset before the pick, and a meeting that needs no consent
wins the poll over one that waits for it.
The App Store build cannot resolve responsibility (private symbol), so
the Safari patterns are compiled out of patterns(watching:) there:
they could never match, and with them in, the browser toggle alone would
start a Core Audio enumeration every poll. The Settings caption names
Safari only in the Homebrew build.
Not done: gating on the coreaudiod VPAUAggregateAudioDevice assertion
(voice processing on). It would sharpen the signal, but the consent
prompt is already the filter, as it is for Chromium. Safari web apps,
other WebKit browsers such as Orion, and a Safari launched from a shell
are not covered (detection.md says so).
e6f913d to
8029f20
Compare
What changed and why
Follow-up to #503: browser-meeting detection now covers Safari in the Homebrew build. WebKit never creates the "WebRTC has active PeerConnections" assertion that path keys on, so Safari calls are detected by the WebKit GPU process holding the mic, behind the existing Browser Web Meetings toggle and the same consent prompt.
Measured during a Meet call in Safari on macOS 27.0.1:
com.apple.WebKit.GPUruns mic input for the whole call, the same with one participant or two, keeps it running while muted (20 s+ observed), and stops when the user leaves with the tab still open. Safari's only power assertions ("WebKit Media Playback") look the same as YouTube's.Change
MicInputDetectorgains Safari and Safari Technology Preview patterns, selected by the Browser Web Meetings toggle; no new setting.ProcessResponsibilityattributes the GPU process to Safari.MicInputDetectortoo.checkOncefollows the assertion detector's rule ([Bug] Google Meet in Chrome not detected: WebRTC assertion logged as "did not match", no consent prompt #814): counters of apps with no hit are reset before the pick, and a meeting that needs no consent wins the poll.patterns(watching:), since responsibility can't be resolved there (private symbol); the browser toggle alone no longer starts a per-poll Core Audio enumeration, and the Settings caption names Safari only in the Homebrew build.CLAUDE.md's Build Variants table gains a row.Release notes: users who already have Browser Web Meetings on get Safari detection on upgrade, plus a Core Audio process enumeration on every poll. Recording stays consent-gated.
Not covered (stated in
detection.md): Safari web apps ("Add to Dock"), other WebKit browsers such as Orion, a Safari launched from a shell (attributed to the launcher), and thecoreaudiodVPAUAggregateAudioDevicegate that would sharpen the signal. Meet's pre-join screen and any page using the mic prompt too; the prompt is the filter, as for Chromium. The 2026-08-11 browser-detection RFC gets a one-line pointer.Tests
MicInputDetector.swift, the Homebrewlet browsers = …inpatterns(watching:)→[]WatchingController+Detectors.swift,mic.isIdentityDenied = isDenied→{ _ in false }MicInputDetector.swift, the counter reset before the pick →where falselet browsers: [MicPattern] = []→ the Homebrew expressionMicInputDetectorSafariTests, includingtestBrowserToggleSelectsTheSafariPatternsandtestDefaultDetectorWatchesSafariAndHonoursTheDenyListtestDefaultDetectorWatchesSafariAndHonoursTheDenyListtestStaleHitIsResetWhileSafariIsReturnedMicInputDetectorSafariAppStoreTests.testBrowserToggleAddsNoSafariPatternsand.testBrowserToggleAloneDoesNotEnumerateAudioProcesses[Safari, com.apple.WebKit.GPU]; leaving the call with the tab open started the end grace at once and ended the recording 15 s later.AI assistance
Checklist
main, no merge commits./scripts/lint.shis clean, and./scripts/lint.sh --analyzetoocd app/MeetingTranscriber && swift test