| Summary: | [EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||
| Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, eric.carlson, glenn, philipj, sergio | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Jer Noble
2014-10-28 14:22:24 PDT
Created attachment 240571 [details]
Patch
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>? 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. Committed r175284: <http://trac.webkit.org/changeset/175284> |