New issue
Advanced search Search tips

Issue 660750 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Do not call closeTask of mediakeysession when player performs the cleanup

Reported by dongheun...@lge.com, Oct 31 2016

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36

Steps to reproduce the problem:
1. open test page(shaka-test.html need to modify iframe's url for testing)
2. select a "Car ClearKey" in iframe.
3. click the "remove iframe" button.

What is the expected behavior?
When call The MediaKeySession's close, call WebContentDecryptionModuleSession's call.

What went wrong?
can't call WebContentDecryptionModuleSession's call because clear pendingActions by calling contextDestroyed.

[debug log]
chrome --enable-logging  --v=1 --vmodule=HTMLMediaElement*=3,MediaKey*=3,*=0

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 56.0.2903.0  Channel: canary
OS Version: ubuntu 14.04 LTS
Flash Version: 

Consequently, there occurs a problem that can not be cleaned up resources for key system.
 
shaka-test.html
541 bytes View Download
debug-log
1.5 KB View Download
Components: -Blink>Media Internals>Media>Encrypted
Status: Available (was: Unconfirmed)
Looking at the test page, Shaka unload() returns a promise. So if the test is updated to

  myiframe_window.shakaDemo.player_.unload().then(() => myiframe.remove());

MediaKeySession::close() has a chance to complete before the context is destroyed.

However, there is still the issue of what to do with any open sessions remaining when the context is destroyed.
Chromium change:
https://codereview.chromium.org/2454333002
I tried the test included as part of the CL. I added additional logging to several functions, and changed the MediaKeySession::contextDestroyed() method to simply log if close was present (instead of calling closeTask()). The log:

    1: AesDecryptor::AesDecryptor()
    2: MediaKeys::MediaKeys()
    3: MediaKeys::createSession() "temporary"
    4: MediaKeySession::MediaKeySession()
    5: MediaKeySession::generateRequest() "webm"
    6: MediaKeySession::message()
    7: MediaKeySession::close()
    8: MediaKeySession::contextDestroyed() skipping close()
    9: MediaKeys::contextDestroyed()
   10: AesDecryptor::~AesDecryptor()

Line 1 is creating ClearKey, while lines 2-7 is basically the code in the test up to the point where the iframe is destroyed. Once the iframe is destroyed, I see contextDestroyed() called on both the MediaKeys and MediaKeySession objects. They free the corresponding Chromium objects (WebContentDecryptionModuleImpl and WebContentDecryptionModuleSessionImpl, respectively), and once that is done the CDM (AesDecryptor in this case) is freed. ~AesDecryptor() should free up any resources used by the CDM.

So this leaves a bunch of questions:
1) If you really want close() to complete, why does the app not wait for the promise to be fulfilled?
2) What should happen to sessions where close() was not called? Don't they leak resources too?
3) As the CDM can do whatever it wants to free up resources in the destructor, why is it important that close() is actually called first? What does close() do that the destructor can't do?
Dear jrummell

Thank you for reviewing this issue.

This is answer about your questions.
1) If you really want close() to complete, why does the app not wait for the promise to be fulfilled?
-> I agree. but I think app wait promise of close. but we can't guide about web page. I think need to implement exceptional case.
2) What should happen to sessions where close() was not called? Don't they leak resources too? 
-> If Key System implement exception code about this case, we don't need to call callTask. Embedded platform don't consider various case. (TV use various embedded chip and platform driver.) We can demand to improve about this issues. But I think need to improve web platform.
3) As the CDM can do whatever it wants to free up resources in the destructor, why is it important that close() is actually called first? What does close() do that the destructor can't do?
-> DRM resource on Embedded Platform is limited. So If don't properly close session, can't acquire resource(call "key slot" by drm engineer) for decryption.


I looked at the current EME spec. Section 6 [1] in describing MediaKeySession states "If a MediaKeySession object becomes inaccessible to the page and is not closed, the User Agent must run the MediaKeySession destroyed algorithm before User Agent state associated with the session is deleted."

Currently we don't do this, so it should be done for both cases 1) and 2). I'll comment in the CL.


[1] http://w3c.github.io/encrypted-media/#mediakeysession-interface
This text was mostly added for the steps related to persistent-usage-record sessions. Destroying the keys is not visible and mostly a Key System issue.

Note that even implementing the missing steps above, there isn't likely to be much difference in practice because the MediaKeySession remains accessible as long as it is not closed and the MediaKeys object that created it remains accessible:
>"A MediaKeySession object shall not be destroyed and shall continue to receive events if it is not closed and the MediaKeys object that created it remains accessible. Otherwise, a MediaKeySession object that is no longer accessible to JavaScript shall not receive further events and may be destroyed."

Thus, this will only be called when the MediaKeys becomes inaccessible, in which case all resources, including sessions and in-memory keys, related to the CDM instance should be destroyed.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 12 2016

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

commit 266df595a25c40f0ddf6876742149559ff42040b
Author: jrummell <jrummell@chromium.org>
Date: Sat Nov 12 00:26:39 2016

EME: Make sure sessions are closed before they are destroyed

EME spec notes that "the User Agent must use the CDM to close the
session before User Agent state associated with the session is
deleted", so close the session if the session hasn't been closed.

BUG= 660750 
TEST=EME layout tests pass

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

[modify] https://crrev.com/266df595a25c40f0ddf6876742149559ff42040b/media/blink/webcontentdecryptionmodulesession_impl.cc
[modify] https://crrev.com/266df595a25c40f0ddf6876742149559ff42040b/media/blink/webcontentdecryptionmodulesession_impl.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 23 2016

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

commit f6f57160426b548a43d8ebc1b1c70c3796dc7923
Author: dongheun.kang <dongheun.kang@lge.com>
Date: Wed Nov 23 02:30:20 2016

Add EME Layout test for unclosed key session

EME spec notes that "the User Agent must use the CDM to close the
session before User Agent state associated with the session is
deleted"
So add test close the key session associated with the object
when the page is inaccessible.

R=jrummell@chromium.org, ddorwin@chromium.org, haraken@chromium.org
BUG= 660750 
TEST=EME layout tests pass

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

[add] https://crrev.com/f6f57160426b548a43d8ebc1b1c70c3796dc7923/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-session-close-and-context-destroyed.html
[add] https://crrev.com/f6f57160426b548a43d8ebc1b1c70c3796dc7923/third_party/WebKit/LayoutTests/media/resources/encrypted-media-session-close-and-context-destroyed-iframe.html

Status: Fixed (was: Available)

Sign in to add a comment