[webkit-reviews] review granted: [Bug 198345] Video playback in Safari should continue when CarPlay is plugged in : [Attachment 370875] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 198345: Video playback in Safari should continue when CarPlay is plugged in
https://bugs.webkit.org/show_bug.cgi?id=198345

Attachment 370875: Patch

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




--- Comment #3 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 370875
  --> https://bugs.webkit.org/attachment.cgi?id=370875
Patch

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

> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:213
> +    setIsPlayingToAutomotiveHeadUnit(carPlayIsConnected.value());

Shouldn't this be: setIsPlayingToAutomotiveHeadUnit(carPlayIsConnected &&
carPlayIsConnected.value())

> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:312
> +    callOnWebThreadOrDispatchAsyncOnMainThread([protectedSelf =
retainPtr(self)]() mutable {

:-/

> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:450
> +	   protectedSelf->_callback->carPlayServerDied();

_callback should be NULL-checked, it may have been clear by the time this runs

> Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:467
> +	  
protectedSelf->_callback->updateCarPlayIsConnected(WTFMove(carPlayIsConnected))
;

Ditto.

> LayoutTests/media/video-isplayingtoautomotiveheadunit.html:11
> +		findMediaElement();
> +
> +		run('video.src = findMediaFile("video", "content/test")');

:-O TABS!