Bug 156633 - [Mac][EME] Protected content over HLS is not notified when a HDCP violation occurs.
Summary: [Mac][EME] Protected content over HLS is not notified when a HDCP violation o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-04-15 12:08 PDT by Jer Noble
Modified: 2016-04-18 09:14 PDT (History)
2 users (show)

See Also:


Attachments
Patch (7.95 KB, patch)
2016-04-15 12:18 PDT, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2016-04-15 12:08:28 PDT
[Mac][EME] Protected content over HLS is not notified when a HDCP violation occurs.
Comment 1 Jon Lee 2016-04-15 12:14:35 PDT
rdar://problem/24328552
Comment 2 Jer Noble 2016-04-15 12:18:38 PDT
Created attachment 276492 [details]
Patch
Comment 3 Eric Carlson 2016-04-15 12:25:16 PDT
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.
Comment 4 Jer Noble 2016-04-15 14:49:00 PDT
(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.
Comment 5 Jer Noble 2016-04-15 14:55:44 PDT
(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. :)
Comment 6 Jer Noble 2016-04-18 09:14:24 PDT
Committed r199672: <http://trac.webkit.org/changeset/199672>