[webkit-reviews] review granted: [Bug 223073] Replace some usage of dispatch_queue with WorkQueue now that it supports sync dispatching : [Attachment 422941] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 223073: Replace some usage of dispatch_queue with WorkQueue now that it
supports sync dispatching
https://bugs.webkit.org/show_bug.cgi?id=223073

Attachment 422941: Patch

https://bugs.webkit.org/attachment.cgi?id=422941&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 422941
  --> https://bugs.webkit.org/attachment.cgi?id=422941
Patch

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

> Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:107
> -    OSObjectPtr<dispatch_queue_t> m_dispatchQueue;
> +    RefPtr<WorkQueue> m_workQueue;
>      RetainPtr<WebInterruptionObserverHelper> m_interruptionObserverHelper;

This now adds a non-thread-safe reference count to this object. I’m assuming
that didn’t matter.

Also, given that everything here is public, this seems more like a struct than
a class.

> Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:271
> +	   NSError* error = nil;

We normally format this as "NSError *". The old code above didn’t do that, but
the code below did.

> Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:272
> +	   m_private->m_workQueue->dispatchSync([&error] {

Can we make a bool come out of this thing instead of a
possibly-already-deallocated NSError that we check against nil?