Bug 138149 – [EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

[EME][Mac] Adopt new AVStreamSession API: pass storageDirectoryAtURL at creation-time

[Jer Noble]

Created attachment 240571 [details]
Patch

[Brent Fulgham]

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 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]

Committed r175284: <http://trac.webkit.org/changeset/175284>

[Jer Noble]

<rdar://problem/19269057>