Bug 138149 - [EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time
Summary: [EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creat...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-28 14:22 PDT by Jer Noble
Modified: 2014-12-16 13:28 PST (History)
5 users (show)

See Also:

Attachments
Patch (5.91 KB, patch)
2014-10-28 14:33 PDT, Jer Noble 		bfulgham: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2014-10-28 14:22:24 PDT 
[EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time
Comment 1 Jer Noble 2014-10-28 14:33:22 PDT 
Created attachment 240571 [details]
Patch
Comment 2 Brent Fulgham 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>?
Comment 3 Jer Noble 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.
Comment 4 Jer Noble 2014-10-28 16:51:43 PDT 
Committed r175284: <http://trac.webkit.org/changeset/175284>
Comment 5 Jer Noble 2014-12-16 13:28:26 PST 
<rdar://problem/19269057>