Bug 179627 – Replace dispatch_async with callOnMainThread

• Status: NEW
• Component: New Bugs

[Alex Christensen]

Replace dispatch_async with callOnMainThread

[Alex Christensen]

Created attachment 326784 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 326784 [details]
Patch

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

I think this blanket replacement is dangerous. Some of those call sites almost certainly want the actual main thread. I think you need to consult file owners for each area.

> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:318
> -    RetainPtr<WebMediaSessionHelper> strongSelf = self;
> -    dispatch_async(dispatch_get_main_queue(), [strongSelf]() {
> +    callOnMainThread([strongSelf = retainPtr(self)]() {
>          BEGIN_BLOCK_OBJC_EXCEPTIONS
>          RetainPtr<MPVolumeView> volumeView = adoptNS([allocMPVolumeViewInstance() init]);
>          callOnWebThreadOrDispatchAsyncOnMainThread([strongSelf, volumeView]() {

I think this one was correct.

> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:364
>          [[UIApplication sharedApplication] beginReceivingRemoteControlEvents];

This probably really should be the main thread, so undo this part.

> Source/WebCore/platform/audio/mac/AudioSampleDataSource.mm:185
> -    dispatch_async(dispatch_get_main_queue(), [sampleCount, sampleTime, presentationTime, absoluteTime = mach_absolute_time(), startFrame1, endFrame1, startFrame2, endFrame2] {
> +    callOnMainThread([sampleCount, sampleTime, presentationTime, absoluteTime = mach_absolute_time(), startFrame1, endFrame1, startFrame2, endFrame2] {
>          LOG(MediaCaptureSamples, "@@ pushSamples: added %ld samples for time = %s (was %s), mach time = %lld", sampleCount, toString(sampleTime).utf8().data(), toString(presentationTime).utf8().data(), absoluteTime);
>          LOG(MediaCaptureSamples, "@@ pushSamples: buffered range was [%lld .. %lld], is [%lld .. %lld]", startFrame1, endFrame1, startFrame2, endFrame2);

Why would we do anything here if we're not logging?