Bug 156633 – [Mac][EME] Protected content over HLS is not notified when a HDCP violation occurs.

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

[Mac][EME] Protected content over HLS is not notified when a HDCP violation occurs.

[Jon Lee]

rdar://problem/24328552

[Jer Noble]

Created attachment 276492 [details]
Patch

[Eric Carlson]

Comment on attachment 276492 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:62
> +        return nullptr;

Nit: nil

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:66
> +    [player addObserver:self forKeyPath:@"outputObscuredDueToInsufficientExternalProtection" options:NSKeyValueObservingOptionNew context:nullptr];

Ditto

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:89
> +                strongSelf->_parent->playerDidReceiveError(error.get());

_parent is a raw pointer, should probably NULL-check it

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:100
> +    : m_parent(parent->createWeakPtr())

Why is this change necessary, it doesn't seem to be used.

[Jer Noble]

(In reply to comment #3)
> Comment on attachment 276492 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276492&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:62
> > +        return nullptr;
> 
> Nit: nil

Ok.

> > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:66
> > +    [player addObserver:self forKeyPath:@"outputObscuredDueToInsufficientExternalProtection" options:NSKeyValueObservingOptionNew context:nullptr];
> 
> Ditto

Ok.

> > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:89
> > +                strongSelf->_parent->playerDidReceiveError(error.get());
> 
> _parent is a raw pointer, should probably NULL-check it

Ok.

> > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:100
> > +    : m_parent(parent->createWeakPtr())
> 
> Why is this change necessary, it doesn't seem to be used.

It actually is used in a lot of places, everywhere that currently uses m_parent will be impacted by this change.  But if the CDM outlives its associated MediaPlayer (which can happen), it needs to be notified and disconnect it's back reference.  The easiest way to do this is with a weak pointer.

[Jer Noble]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 276492 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=276492&action=review
> > 
> > > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:62
> > > +        return nullptr;
> > 
> > Nit: nil
> 
> Ok.
> 
> > > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:66
> > > +    [player addObserver:self forKeyPath:@"outputObscuredDueToInsufficientExternalProtection" options:NSKeyValueObservingOptionNew context:nullptr];
> > 
> > Ditto
> 
> Ok.
> 
> > > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:89
> > > +                strongSelf->_parent->playerDidReceiveError(error.get());
> > 
> > _parent is a raw pointer, should probably NULL-check it
> 
> Ok.
> 
> > > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionAVFoundationObjC.mm:100
> > > +    : m_parent(parent->createWeakPtr())
> > 
> > Why is this change necessary, it doesn't seem to be used.
> 
> It actually is used in a lot of places, everywhere that currently uses
> m_parent will be impacted by this change.  But if the CDM outlives its
> associated MediaPlayer (which can happen), it needs to be notified and
> disconnect it's back reference.  The easiest way to do this is with a weak
> pointer.

Actually, when I say "lots of places" I mean "that one place I added a null check in generateKeyRequest()".  But it _is_ used. :)

[Jer Noble]

Committed r199672: <http://trac.webkit.org/changeset/199672>