Do not call closeTask of mediakeysession when player performs the cleanup
Reported by
dongheun...@lge.com,
Oct 31 2016
|
|||
Issue descriptionUserAgent: 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.
,
Nov 1 2016
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.
,
Nov 1 2016
Chromium change: https://codereview.chromium.org/2454333002
,
Nov 1 2016
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?
,
Nov 1 2016
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.
,
Nov 3 2016
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
,
Nov 7 2016
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.
,
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
,
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
,
Feb 13 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by jrumm...@chromium.org
, Oct 31 2016