RESOLVED FIXED 198345
Video playback in Safari should continue when CarPlay is plugged in
https://bugs.webkit.org/show_bug.cgi?id=198345
Summary Video playback in Safari should continue when CarPlay is plugged in
Jer Noble
Reported 2019-05-29 12:15:44 PDT
Video playback in Safari should continue when CarPlay is plugged in
Attachments
Patch (20.14 KB, patch)
2019-05-29 12:18 PDT, Jer Noble
no flags
Patch for landing (19.69 KB, patch)
2019-05-30 08:49 PDT, Jer Noble
jer.noble: commit-queue+
Patch for landing (20.52 KB, patch)
2019-05-30 08:50 PDT, Jer Noble
no flags
Jer Noble
Comment 1 2019-05-29 12:17:03 PDT
Jer Noble
Comment 2 2019-05-29 12:18:14 PDT
Eric Carlson
Comment 3 2019-05-29 15:15:05 PDT
Comment on attachment 370875 [details] 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!
Jer Noble
Comment 4 2019-05-29 16:38:26 PDT
Comment on attachment 370875 [details] 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()) Yeah, it probably should be (in the case that we can't load the CarPlayIsConnectedAttribute). >> 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 Will do.
Jer Noble
Comment 5 2019-05-30 08:49:53 PDT
Created attachment 370944 [details] Patch for landing
Jer Noble
Comment 6 2019-05-30 08:50:41 PDT
Created attachment 370945 [details] Patch for landing
WebKit Commit Bot
Comment 7 2019-05-30 09:29:44 PDT
Comment on attachment 370945 [details] Patch for landing Clearing flags on attachment: 370945 Committed r245887: <https://trac.webkit.org/changeset/245887>
WebKit Commit Bot
Comment 8 2019-05-30 09:29:46 PDT
All reviewed patches have been landed. Closing bug.
Truitt Savell
Comment 9 2019-05-30 09:56:41 PDT
Jer Noble
Comment 10 2019-05-30 12:04:46 PDT
(In reply to Truitt Savell from comment #9) > Fixed iOS build in https://trac.webkit.org/changeset/245890/webkit Thanks Truitt!
Ryan Haddad
Comment 11 2019-05-30 20:18:23 PDT
This change broke internal builds, details in email and radar. Reverted in https://trac.webkit.org/r245944
Jer Noble
Comment 12 2019-05-30 21:46:50 PDT
Note You need to log in before you can comment on or make changes to this bug.