Bug 236531 – [macOS] Use system window and screen picker when available

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[Eric Carlson]

Use system window and screen picker when available

[Eric Carlson]

rdar://87111816

[Eric Carlson]

Created attachment 451757 [details]
Patch

[Jer Noble]

Comment on attachment 451757 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=451757&action=review

r=me with nit

> Source/WebCore/platform/mediastream/mac/ScreenCaptureKitCaptureSource.mm:151
> +    RunLoop::main().dispatch([self, strongSelf = RetainPtr { self }, session = RetainPtr { session }]() mutable {

We shouldn't use RunLoop::main().dispatch() from WebCore code, because it will dispatch to the wrong thread on iOS/WK1. Just use runOnMainThread(). It's a known problem that this pattern is super confusing; apologies!

> Source/WebCore/platform/mediastream/mac/ScreenCaptureKitCaptureSource.mm:159
> +    RunLoop::main().dispatch([self, strongSelf = RetainPtr { self }, session = RetainPtr { session }]() mutable {

Ditto.

[EWS]

Committed r289696 (247181@main): <https://commits.webkit.org/247181@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451757 [details].

[Eric Carlson]

Reopening to attach new patch.

[Eric Carlson]

Created attachment 451793 [details]
Patch

[EWS]

Committed r289701 (247186@main): <https://commits.webkit.org/247186@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451793 [details].

[Geoffrey Garen]

> > Source/WebCore/platform/mediastream/mac/ScreenCaptureKitCaptureSource.mm:151
> > +    RunLoop::main().dispatch([self, strongSelf = RetainPtr { self }, session = RetainPtr { session }]() mutable {
> 
> We shouldn't use RunLoop::main().dispatch() from WebCore code, because it
> will dispatch to the wrong thread on iOS/WK1. Just use runOnMainThread().
> It's a known problem that this pattern is super confusing; apologies!

Would this review comment be easier to understand if we renamed callOnMainThread to callOnWebCoreThread (or similar)? (I have been low-key wanting to rename that function for a few years, but not sure if the proposed new name would help most people or not.)