RESOLVED FIXED 138149
[EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time
https://bugs.webkit.org/show_bug.cgi?id=138149
Summary [EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creat...
Jer Noble
Reported 2014-10-28 14:22:24 PDT
[EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time
Attachments
Patch (5.91 KB, patch)
2014-10-28 14:33 PDT, Jer Noble
bfulgham: review+
Jer Noble
Comment 1 2014-10-28 14:33:22 PDT
Brent Fulgham
Comment 2 2014-10-28 16:06:46 PDT
Comment on attachment 240571 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240571&action=review Looks good. I had a couple of minor questions about some failure cases. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:695 > + if (!getAVStreamSessionClass() || ![getAVStreamSessionClass() instancesRespondToSelector:@selector(initWithStorageDirectoryAtURL:)]) Did we used to handle AVF media streams that did not support initWithStorageDirectoryAtURL? It looks like the answer is no, but just to be careful I'll ask! :-) > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:699 > + m_streamSession = adoptNS([[getAVStreamSessionClass() alloc] initWithStorageDirectoryAtURL:[NSURL fileURLWithPath:sessionStorageDirectory()]]); Does anything bad happen if we call NSURL fileURLWithPath: <empty string>?
Jer Noble
Comment 3 2014-10-28 16:13:15 PDT
Comment on attachment 240571 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240571&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:695 >> + if (!getAVStreamSessionClass() || ![getAVStreamSessionClass() instancesRespondToSelector:@selector(initWithStorageDirectoryAtURL:)]) > > Did we used to handle AVF media streams that did not support initWithStorageDirectoryAtURL? It looks like the answer is no, but just to be careful I'll ask! :-) AVMediaStream didn't used to support initWithStorageDirectoryAtURL, instead it had a setStorageDirectoryAtURL:error: method, but we did check that they responder to that selector for before calling. >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:699 >> + m_streamSession = adoptNS([[getAVStreamSessionClass() alloc] initWithStorageDirectoryAtURL:[NSURL fileURLWithPath:sessionStorageDirectory()]]); > > Does anything bad happen if we call NSURL fileURLWithPath: <empty string>? Interesting. I don't think so, but for that to happen, confstr() would have to return NULL, which is itself exceptional. I'll address this in a follow-up patch, where we pull the storage location out of ChromeClient rather than using _CS_DARWIN_USER_CACHE_DIR.
Jer Noble
Comment 4 2014-10-28 16:51:43 PDT
Jer Noble
Comment 5 2014-12-16 13:28:26 PST
Note You need to log in before you can comment on or make changes to this bug.