WebVideoFullscreenInterfaceAVKit should only call the UI from main thread.
Created attachment 234872 [details] Patch
Comment on attachment 234872 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234872&action=review > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:592 > + if (m_playerController) > + playerController().delegate = m_videoFullscreenModel; Why use both m_playerController and playerController()? > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:623 > + playerController.contentDuration = duration; > + playerController.maxTime = duration; > + playerController.contentDurationWithinEndTimes = duration; > + playerController.loadedTimeRanges = @[@0, @(duration)]; > + > + // FIXME: we take this as an indication that playback is ready. > + playerController.canPlay = YES; > + playerController.canPause = YES; > + playerController.canTogglePlayback = YES; > + playerController.hasEnabledAudio = YES; > + playerController.canSeek = YES; > + playerController.minTime = 0; > + playerController.status = AVPlayerControllerStatusReadyToPlay; In setWebVideoFullscreenModel you test to ensure that m_playerController isn't NULL. Why is it unnecessary here? > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:637 > + NSTimeInterval anchorTimeStamp = ![playerController() rate] ? NAN : anchorTime; > + AVValueTiming *timing = [getAVValueTimingClass() valueTimingWithAnchorValue:currentTime > + anchorTimeStamp:anchorTimeStamp rate:0]; > + playerController().timing = timing; Ditto. Also, why does this code use playerController() when above you use playerController.? > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:648 > + playerController().rate = isPlaying ? playbackRate : 0.; Ditto. > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:660 > + playerController().hasEnabledVideo = hasVideo; > + playerController().contentDimensions = CGSizeMake(width, height); Ditto. > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:682 > + playerController().seekableTimeRanges = seekableRanges; Ditto. > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:707 > + playerController().audioMediaSelectionOptions = webOptions; > + if (selectedIndex < webOptions.count) > + playerController().currentAudioMediaSelectionOption = webOptions[(size_t)selectedIndex]; Ditto. > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:721 > + playerController().legibleMediaSelectionOptions = webOptions; > + if (selectedIndex < webOptions.count) > + playerController().currentLegibleMediaSelectionOption = webOptions[(size_t)selectedIndex]; Ditto. > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:740 > + playerController().externalPlaybackAirPlayDeviceLocalizedName = localizedDeviceName; > + playerController().externalPlaybackType = externalPlaybackType; > + playerController().externalPlaybackActive = enabled; Ditto.
<rdar://problem/16917699>
(In reply to comment #2) > (From update of attachment 234872 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234872&action=review > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:592 > > + if (m_playerController) > > + playerController().delegate = m_videoFullscreenModel; > > Why use both m_playerController and playerController()? > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:623 > > + playerController.contentDuration = duration; > > + playerController.maxTime = duration; > > + playerController.contentDurationWithinEndTimes = duration; > > + playerController.loadedTimeRanges = @[@0, @(duration)]; > > + > > + // FIXME: we take this as an indication that playback is ready. > > + playerController.canPlay = YES; > > + playerController.canPause = YES; > > + playerController.canTogglePlayback = YES; > > + playerController.hasEnabledAudio = YES; > > + playerController.canSeek = YES; > > + playerController.minTime = 0; > > + playerController.status = AVPlayerControllerStatusReadyToPlay; > > In setWebVideoFullscreenModel you test to ensure that m_playerController isn't NULL. Why is it unnecessary here? > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:637 > > + NSTimeInterval anchorTimeStamp = ![playerController() rate] ? NAN : anchorTime; > > + AVValueTiming *timing = [getAVValueTimingClass() valueTimingWithAnchorValue:currentTime > > + anchorTimeStamp:anchorTimeStamp rate:0]; > > + playerController().timing = timing; > > Ditto. Also, why does this code use playerController() when above you use playerController.? > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:648 > > + playerController().rate = isPlaying ? playbackRate : 0.; > > Ditto. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:660 > > + playerController().hasEnabledVideo = hasVideo; > > + playerController().contentDimensions = CGSizeMake(width, height); > > Ditto. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:682 > > + playerController().seekableTimeRanges = seekableRanges; > > Ditto. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:707 > > + playerController().audioMediaSelectionOptions = webOptions; > > + if (selectedIndex < webOptions.count) > > + playerController().currentAudioMediaSelectionOption = webOptions[(size_t)selectedIndex]; > > Ditto. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:721 > > + playerController().legibleMediaSelectionOptions = webOptions; > > + if (selectedIndex < webOptions.count) > > + playerController().currentLegibleMediaSelectionOption = webOptions[(size_t)selectedIndex]; > > Ditto. > > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:740 > > + playerController().externalPlaybackAirPlayDeviceLocalizedName = localizedDeviceName; > > + playerController().externalPlaybackType = externalPlaybackType; > > + playerController().externalPlaybackActive = enabled; > > Ditto. So long as playerController() is called before m_playerController is used, this are fine. Filed https://bugs.webkit.org/show_bug.cgi?id=134895 for consistent use of playerController() and m_playerController.
Comment on attachment 234872 [details] Patch Clearing flags on attachment: 234872 Committed r171089: <http://trac.webkit.org/changeset/171089>
All reviewed patches have been landed. Closing bug.