Bug 133747

Summary: [EME][Mac] Propagate errors from AVSampleBufferDisplayLayer through to MSE
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, glenn, philipj, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch eric.carlson: review+

Description Jer Noble 2014-06-11 10:35:57 PDT
[EME][Mac] Propagate errors from AVSampleBufferDisplayLayer through to MSE
Comment 1 Jer Noble 2014-06-11 12:19:30 PDT
Created attachment 232885 [details]
Patch
Comment 2 Eric Carlson 2014-06-11 12:38:05 PDT
Comment on attachment 232885 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232885&action=review

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionMediaSourceAVFObjC.mm:68
> +{
> +    m_parent->unregisterForErrorNotifications(this);
>  }

Is it worth NULLing m_client here?
Comment 3 Jer Noble 2014-06-11 13:52:09 PDT
(In reply to comment #2)
> (From update of attachment 232885 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232885&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionMediaSourceAVFObjC.mm:68
> > +{
> > +    m_parent->unregisterForErrorNotifications(this);
> >  }
> 
> Is it worth NULLing m_client here?

Probably not, since the object itself is about to get destroyed.
Comment 4 Jer Noble 2014-06-11 14:02:55 PDT
Committed r169830: <http://trac.webkit.org/changeset/169830>
Comment 5 Darin Adler 2014-06-16 10:24:41 PDT
Comment on attachment 232885 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232885&action=review

> Source/WebCore/Modules/encryptedmedia/MediaKeySession.h:48
> +class MediaKeySession final : public RefCounted<MediaKeySession>, public EventTargetWithInlineData, public ActiveDOMObject, public CDMSessionClient {

Why public instead of private inheritance?

> Source/WebCore/Modules/encryptedmedia/MediaKeySession.h:90
> +    virtual void sendMessage(Uint8Array*, String destinationURL);
> +    virtual void sendError(MediaKeyErrorCode, unsigned long systemCode);

Why protected instead of private? Why not marked override?

> Source/WebCore/platform/graphics/SourceBufferPrivateClient.h:87
> +    virtual void sourceBufferPrivateDidReceiveRenderingError(SourceBufferPrivate*, int errocCode) = 0;

Typo here: "errocCode".

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionMediaSourceAVFObjC.h:38
> +class CDMSessionMediaSourceAVFObjC : public CDMSession, public SourceBufferPrivateAVFObjCErrorClient {

Why public instead of private inheritance?

> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:828
> +    int errorCode = [[[error userInfo] valueForKey:@"OSStatus"] intValue];

Seems like this should be inside the "if (m_client)".