Skip to content

Conversation

@brentfulgham
Copy link
Contributor

@brentfulgham brentfulgham commented May 24, 2023

fe4af8b

[Cocoa] Call [AVCaptureDevice ensureServerConnection] when new extensions are made to the GPU Process
https://bugs.webkit.org/show_bug.cgi?id=257241
<rdar://109380900>

Reviewed by Eric Carlson.

Our current code only calls [AVCaptureDevice ensureServerConnection] the first time we extend the sandbox.
This can lead to problems if the user visits a site that only needs Microphone access (for example), and
then visits a site that needs Camera access. Because the camera-related extensions are only recognized by
the video capture system when [AVCaptureDevice ensureServiceConnection] is called, the fact that we called
it before the new sandbox extensions were in place mean we never activate the relevant features.

To fix this, we should simply call [AVCaptureDevice ensureServerConnection] any time we extend the sandbox.
I have confirmed with the relevant teams that this call is low-cost (performance wise), and is not harmful
to call repeatedly for things that have already been activated.

This patch also adds new logging to help debug similar problems in the future.

* Source/WebKit/GPUProcess/GPUProcess.cpp:
(WebKit::GPUProcess::updateSandboxAccess): Add logging.
(WebKit::GPUProcess::updateCaptureAccess): Ditto.
* Source/WebKit/GPUProcess/cocoa/GPUProcessCocoa.mm:
(WebKit::GPUProcess::ensureAVCaptureServerConnection): Add new logging, no longer make this a 'std::call_once'.

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

f552b6c

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
💥 🧪 ios-wk2-wpt ✅ 🧪 gtk-wk2
🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@brentfulgham brentfulgham self-assigned this May 24, 2023
@brentfulgham brentfulgham added the WebKit2 Bugs relating to the WebKit2 API layer label May 24, 2023
@brentfulgham brentfulgham added the merge-queue Applied to send a pull request to merge-queue label May 24, 2023
…ions are made to the GPU Process

https://bugs.webkit.org/show_bug.cgi?id=257241
<rdar://109380900>

Reviewed by Eric Carlson.

Our current code only calls [AVCaptureDevice ensureServerConnection] the first time we extend the sandbox.
This can lead to problems if the user visits a site that only needs Microphone access (for example), and
then visits a site that needs Camera access. Because the camera-related extensions are only recognized by
the video capture system when [AVCaptureDevice ensureServiceConnection] is called, the fact that we called
it before the new sandbox extensions were in place mean we never activate the relevant features.

To fix this, we should simply call [AVCaptureDevice ensureServerConnection] any time we extend the sandbox.
I have confirmed with the relevant teams that this call is low-cost (performance wise), and is not harmful
to call repeatedly for things that have already been activated.

This patch also adds new logging to help debug similar problems in the future.

* Source/WebKit/GPUProcess/GPUProcess.cpp:
(WebKit::GPUProcess::updateSandboxAccess): Add logging.
(WebKit::GPUProcess::updateCaptureAccess): Ditto.
* Source/WebKit/GPUProcess/cocoa/GPUProcessCocoa.mm:
(WebKit::GPUProcess::ensureAVCaptureServerConnection): Add new logging, no longer make this a 'std::call_once'.

Canonical link: https://commits.webkit.org/264460@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Cocoa-Call-AVCaptureDevice-ensureServerConnection-when-new-extensions-are-made-to-the-GPU-Process branch from f552b6c to fe4af8b Compare May 24, 2023 06:06
@webkit-commit-queue
Copy link
Collaborator

Committed 264460@main (fe4af8b): https://commits.webkit.org/264460@main

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

@webkit-commit-queue webkit-commit-queue merged commit fe4af8b into WebKit:main May 24, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit2 Bugs relating to the WebKit2 API layer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants