[webkit-reviews] review granted: [Bug 226672] Reduce use of legacy MainThreadTaskQueue in media code : [Attachment 430639] 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 226672: Reduce use of legacy MainThreadTaskQueue in media code
https://bugs.webkit.org/show_bug.cgi?id=226672

Attachment 430639: Patch

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




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

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

> Source/WebCore/platform/graphics/cocoa/TextTrackRepresentationCocoa.mm:155
> +	   if (weakThis)
> +	      
weakThis->client().textTrackRepresentationBoundsChanged(weakThis->bounds());

Is bounds() guaranteed to not do any work that could have the side effect of
destroying this?

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:182
> +    callOnMainThread([this] {

How did you decide a smart pointer (weak pointer or strong pointer) was not
needed here?

> Source/WebCore/platform/mock/MediaPlaybackTargetPickerMock.cpp:79
> +    callOnMainThread([this, weakThis = makeWeakPtr(*this)] {
> +	   if (!weakThis)
> +	       return;

I prefer the style where we turn weakThis into a RefPtr for the duration of the
lambda over the more tricky style where we capture this twice, for both weak
and strong pointers. And this is test code so it’s not performance-sensitive.