Bug 133747 - [EME][Mac] Propagate errors from AVSampleBufferDisplayLayer through to MSE
Summary: [EME][Mac] Propagate errors from AVSampleBufferDisplayLayer through to MSE
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:
Depends on:
Blocks:
 
Reported: 2014-06-11 10:35 PDT by Jer Noble
Modified: 2014-06-16 10:24 PDT (History)
5 users (show)

See Also:

Attachments
Patch (25.51 KB, patch)
2014-06-11 12:19 PDT, Jer Noble 		eric.carlson: 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-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)".