| Summary: | [macOS] Use system window and screen picker when available | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||
| Component: | WebRTC | Assignee: | Eric Carlson <eric.carlson> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, ews-watchlist, ggaren, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Eric Carlson
2022-02-11 15:24:30 PST
Created attachment 451757 [details]
Patch
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. 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]. Reopening to attach new patch. Created attachment 451793 [details]
Patch
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]. > > 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.)
|