Bug 176985 - [EME] ClearKey: list 'persistent-license' sessions as supported
Summary: [EME] ClearKey: list 'persistent-license' sessions as supported
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-15 00:05 PDT by Zan Dobersek
Modified: 2017-09-27 12:26 PDT (History)
4 users (show)

See Also:


Attachments
Patch (9.18 KB, patch)
2017-09-15 00:13 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (9.04 KB, patch)
2017-09-15 02:02 PDT, Zan Dobersek
calvaris: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2017-09-15 00:05:05 PDT
[EME] ClearKey: list 'persistent-license' sessions as supported
Comment 1 Zan Dobersek 2017-09-15 00:13:18 PDT
Created attachment 320873 [details]
Patch
Comment 2 Build Bot 2017-09-15 00:14:46 PDT
Attachment 320873 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:186:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:210:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Xabier Rodríguez Calvar 2017-09-15 01:29:16 PDT
Comment on attachment 320873 [details]
Patch

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

> Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:212
> +    // Reject any configuration that marks persistent state as required, unless
> +    // the 'persistent-license' session type has to be supported.
> +    auto& sessionTypes = configuration.sessionTypes;
> +    bool supportsPersistentLicenseSession = std::any_of(sessionTypes.begin(), sessionTypes.end(),
> +        [] (auto& sessionType) { return sessionType == CDMSessionType::PersistentLicense; });
> +    if (configuration.persistentState == CDMRequirement::Required && !supportsPersistentLicenseSession)
>          return false;

This piece of code is the same as in the other function, right? It could make sense to refactor it into a private method.
Comment 4 Zan Dobersek 2017-09-15 02:02:02 PDT
Created attachment 320882 [details]
Patch
Comment 5 Build Bot 2017-09-15 02:03:51 PDT
Attachment 320882 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:179:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Xabier Rodríguez Calvar 2017-09-15 05:27:14 PDT
Comment on attachment 320882 [details]
Patch

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

> Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:190
> +    // Reject any configuration that marks persistent state as required, unless
> +    // the 'persistent-license' session type has to be supported.
> +    if (configuration.persistentState == CDMRequirement::Required && !containsPersistentLicenseType(configuration.sessionTypes))

You could have gone further, pass the configuration and check the persistent state to be required and if the vector contains a persistent session.
Comment 7 Zan Dobersek 2017-09-15 06:21:44 PDT
Comment on attachment 320882 [details]
Patch

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

>> Source/WebCore/platform/encryptedmedia/clearkey/CDMClearKey.cpp:190
>> +    if (configuration.persistentState == CDMRequirement::Required && !containsPersistentLicenseType(configuration.sessionTypes))
> 
> You could have gone further, pass the configuration and check the persistent state to be required and if the vector contains a persistent session.

IMO that would reduce clarity, and it would remove the similarity with the distinctiveIdentifier check.
Comment 8 Zan Dobersek 2017-09-15 06:24:50 PDT
Committed r222085: <http://trac.webkit.org/changeset/222085>
Comment 9 Radar WebKit Bug Importer 2017-09-27 12:26:59 PDT
<rdar://problem/34693298>