If it is dead, no need to do it.
rdar://problem/33803695
Created attachment 320933 [details] Patch
Comment on attachment 320933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320933&action=review > Source/WebCore/ChangeLog:11 > + (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::requestNotificationWhenReadyForVideoData): exciting early in block to prevent enqueueing. Nit: "exiting". > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:401 > + auto weakThis = createWeakPtr(); > [m_sampleBufferDisplayLayer requestMediaDataWhenReadyOnQueue:dispatch_get_main_queue() usingBlock:^ { > + if (!weakThis) > + return; > + Note that this won't necessarily be correct on iOS WK1, as the "dispatch_get_main_queue()" won't necessarily be the WebThread, in which case, you'll assert in the weakThis getter. This isn't a problem in the MSE code, since that's not enabled on iOS, but it may be a problem here, since media streams are.
> Note that this won't necessarily be correct on iOS WK1, as the > "dispatch_get_main_queue()" won't necessarily be the WebThread, in which > case, you'll assert in the weakThis getter. This isn't a problem in the MSE > code, since that's not enabled on iOS, but it may be a problem here, since > media streams are. I am not familiar with WebThread. I guess that if we are not in the WebThread, we could hop to it, through callOnMainThread probably?. Or is there a way to dispatch it to the WebThread directly?
(In reply to youenn fablet from comment #4) > > Note that this won't necessarily be correct on iOS WK1, as the > > "dispatch_get_main_queue()" won't necessarily be the WebThread, in which > > case, you'll assert in the weakThis getter. This isn't a problem in the MSE > > code, since that's not enabled on iOS, but it may be a problem here, since > > media streams are. > > I am not familiar with WebThread. The basic gist is that, to get smooth scrolling on iOS in WK1, all the WebCore "main thread" functions run off the process's main thread, an on a named thread called "WebThread". > I guess that if we are not in the WebThread, we could hop to it, through > callOnMainThread probably?. Yes. > Or is there a way to dispatch it to the WebThread directly? No, there's not (but that would be great!).
Comment on attachment 320933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320933&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:401 >> + > > Note that this won't necessarily be correct on iOS WK1, as the "dispatch_get_main_queue()" won't necessarily be the WebThread, in which case, you'll assert in the weakThis getter. This isn't a problem in the MSE code, since that's not enabled on iOS, but it may be a problem here, since media streams are. We don't support media streams in WK1 so this should be OK.
(In reply to Eric Carlson from comment #6) > Comment on attachment 320933 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=320933&action=review > > >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:401 > >> + > > > > Note that this won't necessarily be correct on iOS WK1, as the "dispatch_get_main_queue()" won't necessarily be the WebThread, in which case, you'll assert in the weakThis getter. This isn't a problem in the MSE code, since that's not enabled on iOS, but it may be a problem here, since media streams are. > > We don't support media streams in WK1 so this should be OK. Oh, ok, never mind then. :)
Comment on attachment 320933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320933&action=review > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:400 > + if (!weakThis) > + return; What if the object gets destroyed after this check, but before the rest of the code in the block runs?
I think there is a plan to try removing the WebThread.
Comment on attachment 320933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=320933&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:400 >> + return; > > What if the object gets destroyed after this check, but before the rest of the code in the block runs? As I read it, the object destruction and this code block should both happen on the same thread.
Comment on attachment 320933 [details] Patch Have we converged to an r+ then?
(In reply to Geoffrey Garen from comment #11) > Comment on attachment 320933 [details] > Patch > > Have we converged to an r+ then? Yep.
Comment on attachment 320933 [details] Patch Clearing flags on attachment: 320933 Committed r222105: <http://trac.webkit.org/changeset/222105>
All reviewed patches have been landed. Closing bug.