New issue
Advanced search Search tips

Issue 616166 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 714344



Sign in to add a comment

EME: Allow remove() of temporary sessions

Project Member Reported by ddorwin@chromium.org, May 31 2016

Issue description

https://github.com/w3c/encrypted-media/issues/180#issuecomment-222194463 changes the definition of remove() to "Removes all license(s) and key(s)..." and makes it apply to "temporary" sessions.

For "temporary" sessions, it is a bit redundant with close for most purposes.
 
Labels: M-56 Pri-1

Comment 2 by xhw...@chromium.org, Mar 24 2017

Cc: yucliu@chromium.org xhw...@chromium.org
Owner: jrumm...@chromium.org
yucliu: FYI. For MediaDrm, this can be implemented using MediaDrm::removeKeys(). This is not directly related to  issue 493521  and we can fix it separately.

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aab90f001ac39ed2fdc19fd81d004d278160db2e

commit aab90f001ac39ed2fdc19fd81d004d278160db2e
Author: xhwang <xhwang@chromium.org>
Date: Sat Apr 22 00:30:06 2017

media: Update comment on SessionExpirationUpdateCB

Clarify what the base::Time value should be to signal NaN in JS.

BUG= 616166 
TEST=Comment change only.
R=jrummell@chromium.org

Review-Url: https://codereview.chromium.org/2830343003
Cr-Commit-Position: refs/heads/master@{#466510}

[modify] https://crrev.com/aab90f001ac39ed2fdc19fd81d004d278160db2e/media/base/content_decryption_module.h

Blockedon: 714344
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/74fbc3d0e83f922543913db7b8b86a65c9d176d7

commit 74fbc3d0e83f922543913db7b8b86a65c9d176d7
Author: jrummell <jrummell@chromium.org>
Date: Tue Apr 25 20:54:19 2017

EME: Allow temporary sessions to be removed for ClearKey only.

The EME spec allows MediaKeySession::remove() to be called on temporary
sessions, so update the code to allow it. Doing this for ClearKey
only as other CDMs may not support it.

BUG= 616166 
TEST=updated tests pass

Review-Url: https://codereview.chromium.org/2831963003
Cr-Commit-Position: refs/heads/master@{#467106}

[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/content/renderer/media/cdm/render_cdm_factory.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/base/android/android_cdm_factory.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/blink/webcontentdecryptionmodulesession_impl.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/blink/webcontentdecryptionmodulesession_impl.h
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/cdm/aes_decryptor.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/cdm/aes_decryptor.h
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/cdm/aes_decryptor_unittest.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/cdm/default_cdm_factory.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/cdm/ppapi/external_clear_key/clear_key_cdm.h
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/mojo/clients/mojo_cdm_factory.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/test/fake_encrypted_media.cc
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/test/fake_encrypted_media.h
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/media/test/pipeline_integration_test.cc
[add] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-session-remove-temporary.html
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-syntax.html
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js
[modify] https://crrev.com/74fbc3d0e83f922543913db7b8b86a65c9d176d7/third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42f79db5eabcab9856321dcbdec9953c156784a6

commit 42f79db5eabcab9856321dcbdec9953c156784a6
Author: jrummell <jrummell@chromium.org>
Date: Fri Apr 28 20:12:03 2017

Cleanup verifyKeyStatuses() to take KeyId/status pairs

As suggested in a previous CL, the status for each keyId should have it's own
status value rather than relying on all the KeyIds to have the same status.

No longer passing |unexpected|, as the size check verifies that the correct
number of expected KeyIds matches.

BUG= 616166 
TEST=modified tests pass

Review-Url: https://codereview.chromium.org/2847803005
Cr-Commit-Position: refs/heads/master@{#468105}

[modify] https://crrev.com/42f79db5eabcab9856321dcbdec9953c156784a6/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-keystatuses-multiple-keys.html
[modify] https://crrev.com/42f79db5eabcab9856321dcbdec9953c156784a6/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-keystatuses-multiple-sessions.html
[modify] https://crrev.com/42f79db5eabcab9856321dcbdec9953c156784a6/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-keystatuses-multiple-updates.html
[modify] https://crrev.com/42f79db5eabcab9856321dcbdec9953c156784a6/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-session-remove-temporary.html
[modify] https://crrev.com/42f79db5eabcab9856321dcbdec9953c156784a6/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-utils.js

Comment 7 by xhw...@chromium.org, May 12 2017

Is this fixed now?
#5 fixed it for ClearKey only (with a key_system check in webcontentdecryptionmodulesession_impl.cc).
Labels: -Pri-1 -M-56 Pri-2
Removing milestone as issue 714344 needs to be fixed before this can be enabled for all key systems.
Status: Assigned (was: Available)
Now fixed for Widevine as well, will add a test and enable in Chrome.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5abec6ca0ec9fca31dca9893cc9ae251d242f71

commit b5abec6ca0ec9fca31dca9893cc9ae251d242f71
Author: John Rummell <jrummell@chromium.org>
Date: Thu Oct 11 01:45:45 2018

EME: Allow remove() on temporary sessions

Now that all the supported CDMs support this, don't fail the call early
but instead allow the CDM to process it.

BUG= 616166 
TEST=new browser_tests pass

Change-Id: I67bb775b71f0f8db4e62462f4a288527e86a78e0
Reviewed-on: https://chromium-review.googlesource.com/c/1269915
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598626}
[modify] https://crrev.com/b5abec6ca0ec9fca31dca9893cc9ae251d242f71/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/b5abec6ca0ec9fca31dca9893cc9ae251d242f71/media/blink/webcontentdecryptionmodulesession_impl.cc
[modify] https://crrev.com/b5abec6ca0ec9fca31dca9893cc9ae251d242f71/media/test/data/eme_player_js/utils.js
[add] https://crrev.com/b5abec6ca0ec9fca31dca9893cc9ae251d242f71/media/test/data/eme_remove_session_test.html

Status: Fixed (was: Assigned)

Sign in to add a comment