Bug 167870 - [EME] Implement MediaKeySession::remove()
Summary: [EME] Implement MediaKeySession::remove()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks: 158841
  Show dependency tree / graph
 
Reported: 2017-02-05 23:36 PST by Zan Dobersek
Modified: 2017-02-08 02:41 PST (History)
2 users (show)

See Also:

Attachments
Patch (21.20 KB, patch)
2017-02-06 01:19 PST, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Patch for landing (20.22 KB, patch)
2017-02-08 02:39 PST, Zan Dobersek 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Zan Dobersek 2017-02-05 23:36:30 PST 
SSIA.
Comment 1 Zan Dobersek 2017-02-06 01:19:55 PST 
Created attachment 300698 [details]
Patch
Comment 2 Xabier Rodríguez Calvar 2017-02-06 02:18:02 PST 
Comment on attachment 300698 [details]
Patch

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

> Source/WebCore/testing/MockCDMFactory.h:75
> +    Vector<Ref<SharedBuffer>> removeKeysFromSessionWithID(const String& id);

You don't need the parameter name here.
Comment 3 Jer Noble 2017-02-06 08:47:52 PST 
Comment on attachment 300698 [details]
Patch

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

> Source/WebCore/Modules/encryptedmedia/MediaKeySession.cpp:422
> +        CDMInstance::LicenseType sessionType;
> +        switch (m_sessionType) {
> +        case MediaKeySessionType::Temporary:
> +            sessionType = CDMInstance::LicenseType::Temporary;
> +            break;
> +        case MediaKeySessionType::PersistentLicense:
> +            sessionType = CDMInstance::LicenseType::Persistable;
> +            break;
> +        case MediaKeySessionType::PersistentUsageRecord:
> +            sessionType = CDMInstance::LicenseType::UsageRecord;
> +            break;
> +        };

This pattern happens enough in the new MSE code that we should have a conversion routine to cast between MediaKeySessionType <-> LicenseType.
Comment 4 Zan Dobersek 2017-02-06 11:45:33 PST 
(In reply to comment #3)
> This pattern happens enough in the new MSE code that we should have a
> conversion routine to cast between MediaKeySessionType <-> LicenseType.

Posted a patch in bug #167896.
Comment 5 Xabier Rodríguez Calvar 2017-02-07 01:11:49 PST 
(In reply to comment #3)
> > Source/WebCore/Modules/encryptedmedia/MediaKeySession.cpp:422
> > +        CDMInstance::LicenseType sessionType;
> > +        switch (m_sessionType) {
> > +        case MediaKeySessionType::Temporary:
> > +            sessionType = CDMInstance::LicenseType::Temporary;
> > +            break;
> > +        case MediaKeySessionType::PersistentLicense:
> > +            sessionType = CDMInstance::LicenseType::Persistable;
> > +            break;
> > +        case MediaKeySessionType::PersistentUsageRecord:
> > +            sessionType = CDMInstance::LicenseType::UsageRecord;
> > +            break;
> > +        };
> 
> This pattern happens enough in the new MSE code that we should have a
> conversion routine to cast between MediaKeySessionType <-> LicenseType.

I thought of that too but I thought it might be left for a rework later.

Same goes for several functions that we have on the tests and I am seeing repeated during the latest patches review processes.
Comment 6 Zan Dobersek 2017-02-08 02:39:19 PST 
Created attachment 300886 [details]
Patch for landing
Comment 7 Zan Dobersek 2017-02-08 02:41:22 PST 
Comment on attachment 300886 [details]
Patch for landing

Clearing flags on attachment: 300886

Committed r211857: <http://trac.webkit.org/changeset/211857>
Comment 8 Zan Dobersek 2017-02-08 02:41:30 PST 
All reviewed patches have been landed.  Closing bug.