Bug 167870 – [EME] Implement MediaKeySession::remove()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Zan Dobersek]

SSIA.

[Zan Dobersek]

Created attachment 300698 [details]
Patch

[Xabier Rodríguez Calvar]

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.

[Jer Noble]

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.

[Zan Dobersek]

(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.

[Xabier Rodríguez Calvar]

(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.

[Zan Dobersek]

Created attachment 300886 [details]
Patch for landing

[Zan Dobersek]

Comment on attachment 300886 [details]
Patch for landing

Clearing flags on attachment: 300886

Committed r211857: <http://trac.webkit.org/changeset/211857>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.