In the object-oriented API (bug 16613), the issue is now the clearing of the |keys| attribute and the close()'ing of a MediaKeySession. These operations are not defined yet, so this bug should track the definition of them.

Assigning to David.

Comment moved/paraphrased from duplicate bug 19785 --

There is some discussion on this email thread (http://lists.w3.org/Archives/Public/public-html-media/2012Oct/0066.html) about when keys are cleared. After re-reading the spec it does not appear clear that keys or licenses can be retained in a persistent cache by the CDM between sessions. 

I don't believe the intent of the spec is to prevent the CDM from retaining keys or licenses across sessions, but I think that needs to be spelled out in the spec a little more explicitly. Specifically I think this section (http://dvcs.w3.org/hg/html-media/raw-file/tip/encrypted-media/encrypted-media.html#dom-close) needs to be clarified to say that only keys which are not intended to be retained across sessions should be cleared.

I mean across sessions created using createSession(). I also mean across browser instantiations -- e.g. if I close my browser I may not want to throw away all of my cached licenses. This has implications for when the browser is in privacy mode and when this type of data would be cleared, but all have pretty reasonable answers.

It might be useful to add a definition of transient keys versus persistent keys and use that as a reference point where key caching is discussed in the spec as well.

*** Bug 19785 has been marked as a duplicate of this bug. ***

We discussed this bug on the last telcon.

I think there are two parts to this bug. One is how close() might relate to secure key release. I believe that this should be handled as part of bug 17199, which should be used to solve the problems for and define secure key release.

Here I think the question is what effect should close() have on a session. For that I think we have to consider what scenarios close would be used in. close() is only necessary if we think there is a situation where the JavaScript application will have a better idea of the desired key lifetime than the CDM. It would use close() to signal some extra hint that would be used to release the resources associated with keys. It is not clear that this situation exists and if we don't have evidence for it then it would be better to leave close() out until we understand what scenarios it needs to solve.

With our proposal for the MediaKeySession state model, two different MediaKeySessions could be responsible for acquiring the same key. This means that calling close() on one session doesn't offer any guarantee that a particular key is released. In addition, the way many CDMs are likely to be implemented, keys will be passed to the media engine and their lifetime managed there.

I propose that we remove the close() method from the MediaKeySession in v1 unless we have a concrete example from on implementer of how they will use it. In that case, the language of the spec should make close only a hint to the CDM that may be ignored.

I vote that we retain close() but indicate in the spec that it is a hint. In limited resource situations the CDM may need to release unused keys as quickly as possible to allow new keys to be acquired. I don't see another specified mechanism to let the CDM know that the app is done with the key.

In case it was not clear from my comment - the concrete example I am thinking of is switching from one app to another, both using EME. I would like to not retain the keys any longer than required and my feeling is than an explicit close() message will be faster than waiting for the JS engine to garbage collect the closed tab. It will also allow for apps that detect when they are not in the foreground and release resources they don't need any longer (e.g. keys).

My proposal for close is as follows (I think this also solves for Mark's comments in bug 17199):

We should add a new state to the state diagram CLOSED. It is possible to get to CLOSED from CREATED, PENDING, and READY. A keyclosed event is fired when this transition occurs.

There are two ways to end up in the CLOSED state:

1) The CDM may have information to know that a session is no longer needed for anything and can fire keyclosed to notify the application.

2) An application can call the close() method. This may move the session directly to CLOSED (by causing keyclosed event to fire). In the secure key release case, a move to PENDING and then CLOSED occurs (with corresponding keymessage and keyclosed events).

When a session is in the CLOSED state, no further events will be fired. Calling close() in the CLOSED state is a no-op. CDMs are not required to take any action related to keys but they may do whatever they want.

I think the CLOSED state makes sense, and there are other use cases for it (e.g. the platform forcefully closed the session for some reason).

I don't object to close(), but I'd like to make sure we understand the use cases we're targeting and that it really solves them.

(In reply to Adrian Bateman [MSFT] from comment #5)
> We discussed this bug on the last telcon.
> 
> I think there are two parts to this bug. One is how close() might relate to
> secure key release. I believe that this should be handled as part of bug
> 17199, which should be used to solve the problems for and define secure key
> release.
> 
> Here I think the question is what effect should close() have on a session.
> For that I think we have to consider what scenarios close would be used in.
> close() is only necessary if we think there is a situation where the
> JavaScript application will have a better idea of the desired key lifetime
> than the CDM. It would use close() to signal some extra hint that would be
> used to release the resources associated with keys. It is not clear that
> this situation exists and if we don't have evidence for it then it would be
> better to leave close() out until we understand what scenarios it needs to
> solve.

What changed between comment #5 and comment #8? What scenarios/concrete examples are we trying to address? (Also, see the reply to comment #7 below.)

> With our proposal for the MediaKeySession state model, two different
> MediaKeySessions could be responsible for acquiring the same key. This means
> that calling close() on one session doesn't offer any guarantee that a
> particular key is released. In addition, the way many CDMs are likely to be
> implemented, keys will be passed to the media engine and their lifetime
> managed there.

We need to address the behavior around such "shared" sessions if we choose to move forward with the proposal in comment #8. If close() is just a hint (see below), is it up to the CDM to decide what to do? Whenever "shared" sessions are closed by the CDM, both should probably receive the CLOSED event.

> I propose that we remove the close() method from the MediaKeySession in v1
> unless we have a concrete example from on implementer of how they will use
> it. In that case, the language of the spec should make close only a hint to
> the CDM that may be ignored.

Should we be more explicit about close() being a hint in the proposal in comment #8?


(In reply to Joe Steele from comment #7)
> In case it was not clear from my comment - the concrete example I am
> thinking of is switching from one app to another, both using EME. I would
> like to not retain the keys any longer than required and my feeling is than
> an explicit close() message will be faster than waiting for the JS engine to
> garbage collect the closed tab. It will also allow for apps that detect when
> they are not in the foreground and release resources they don't need any
> longer (e.g. keys).

Does "switching from one app to another" mean navigating away from the page? I don't think there's any guarantee that you'll be able to call close() or that it will take effect when navigating away or closing the tab.

Maybe we need the browser to "stop" the session and/or MediaKeys (CDM instance) in these cases. (This would be similar to how ActiveDOMObjects in WebKit/Blink are stop()'d, but I don't know if there is a way to specify such behavior.)

(In reply to David Dorwin from comment #9)
> What changed between comment #5 and comment #8? What scenarios/concrete
> examples are we trying to address? (Also, see the reply to comment #7 below.)

What changed was thinking about Joe's comments and also reading Mark's comments in bug 17199.

> > With our proposal for the MediaKeySession state model, two different
> > MediaKeySessions could be responsible for acquiring the same key. This means
> > that calling close() on one session doesn't offer any guarantee that a
> > particular key is released. In addition, the way many CDMs are likely to be
> > implemented, keys will be passed to the media engine and their lifetime
> > managed there.
> 
> We need to address the behavior around such "shared" sessions if we choose
> to move forward with the proposal in comment #8. If close() is just a hint
> (see below), is it up to the CDM to decide what to do? Whenever "shared"
> sessions are closed by the CDM, both should probably receive the CLOSED
> event.

Since I wrote that I've proposed that there is no explicit sharing - just that you don't necessarily need to go through PENDING to get to READY.

> > I propose that we remove the close() method from the MediaKeySession in v1
> > unless we have a concrete example from on implementer of how they will use
> > it. In that case, the language of the spec should make close only a hint to
> > the CDM that may be ignored.
> 
> Should we be more explicit about close() being a hint in the proposal in
> comment #8?

I don't think we can be. I wonder if we can start with saying there are no guarantees and see if we run into implementation problems? Are there specific guarantees you are looking for?

(In reply to Adrian Bateman [MSFT] from comment #10)
> Since I wrote that I've proposed that there is no explicit sharing - just
> that you don't necessarily need to go through PENDING to get to READY.

Thanks. I moved this discussion to bug 21855.
 
(In reply to Adrian Bateman [MSFT] from comment #10)
> (In reply to David Dorwin from comment #9)
> > Should we be more explicit about close() being a hint in the proposal in
> > comment #8?
> 
> I don't think we can be. I wonder if we can start with saying there are no
> guarantees and see if we run into implementation problems? Are there
> specific guarantees you are looking for?

I meant, should we explicitly note that close() is just a hint - that is, it will not necessarily destroy keys or result in a closed event? (I think we're in agreement that this is the intended/possible behavior.) This might be helpful for readers who might otherwise assume that close() will destroy keys immediately.

(In reply to David Dorwin from comment #11)
> (In reply to Adrian Bateman [MSFT] from comment #10)
> > (In reply to David Dorwin from comment #9)
> > > Should we be more explicit about close() being a hint in the proposal in
> > > comment #8?
> > 
> > I don't think we can be. I wonder if we can start with saying there are no
> > guarantees and see if we run into implementation problems? Are there
> > specific guarantees you are looking for?
> 
> I meant, should we explicitly note that close() is just a hint - that is, it
> will not necessarily destroy keys or result in a closed event? (I think
> we're in agreement that this is the intended/possible behavior.) This might
> be helpful for readers who might otherwise assume that close() will destroy
> keys immediately.

Got it. Yes, we should.

(In reply to Adrian Bateman [MSFT] from comment #8)
This proposal was discussed on the telcon 10/15. This will be added to the spec.

Immediately before the current definition of close() [1] is the following text:

"The key acquisition process may involve the web page handling keymessage events, sending the message to a Key System-specific service, and calling update with the response message. update calls may generate keymessage events. During the process, the web page may wish to cancel the acquisition process. For example, if the page cannot contact the license service because of network issues it may wish to fallback to an alternative key system. The page calls close() to cancel the a key acquisition session."

This text is no longer accurate/complete and may be unnecessary. We should update or remove it when updating the spec to address this bug.

[1] https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-media.html#dom-close

Assigning to Adrian per Action 41.

It seems likely that use cases involving stored sessions will want to use close() to indicate that the stored session should be removed from local storage.

In that case, close() is more than a hint to destroy in-memory keys and move the session to the CLOSED state. I think it's fine for close() to be a "hint", but we might want the name to give an indication that it is more than just a quick way to move a MediaKeySession to the CLOSED state. It's is really permanently releasing the backing resources for the session.

Maybe it should be called release() or something similar (free() and destroy() come to mind but don't seem to fit all cases as well).

Note: When using stored session data, apps would not be able to use close() just to accelerate closing of a MediaKeySession. That scenario probably doesn't make much sense anyway since the session is stored locally.

An alternative is to add an explicit method (e.g. release()) in addition to close(). This has the advantage of there being no ambiguity in the behavior, but it adds another method to EME that may not always be useful.

Note: The spec currently says "When destroying a MediaKeySession object, follow the steps in close()." This may not longer be true if we made changes similar to those mentioned above.

Note: The text used to say "When destroying a __MediaKeys__ object, follow the steps in close().", but I don't think that is correct. A MediaKeys object should probably not be destroyed until all of its MediaKeySessions have been destroyed, but I'm not sure how to specify that.

Looks good to me. This addresses the use-case and part of bug 17199. I think release() is a good name, with the semantics that (shared keys aside) all copies of the key (in memory and in storage) are destroyed.

For shared keys, we could be explicit that the key is destroyed when all sessions associated with the key are released.

The remaining issue in 17199 is how to deal with sessions that were destroyed due to windows closing etc. before having the chance to go through the release() -> PENDING -> update() -> CLOSED sequence.

I've made an initial attempt to add the CLOSED state approach:
https://dvcs.w3.org/hg/html-media/rev/196e6b904b22

TODO:
* I didn't add the logic around key release and moving through PENDING to CLOSED - do we want to reuse PENDING or PENDINGCLOSE? Since the state is currently internal (no external property) a new state would make spec'ing easier.

* I haven't updated the state diagram yet - I wanted to focus on the text changes first.

The closed event is missing from https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-media.html#event-summary and thus the #dom-eventclosed links are broken.

(In reply to Adrian Bateman [MSFT] from comment #19)
> * I haven't updated the state diagram yet - I wanted to focus on the text
> changes first.

I updated the state diagram.
https://dvcs.w3.org/hg/html-media/rev/a717962fc322

I updated the spec to use a session close algorithm and a queue a "message" event algorithm and then referenced these algorithms appropriately in the text. I modified release() to be clear that it may either run the session close algorithm or schedule some other work leaded to this. The other work could be to queue a "message" event that participates in secure key release and which ultimately runs the session close algorithm. I think with these changes, we can remove section 5 (Key Release) to resolve bug 17199.

https://dvcs.w3.org/hg/html-media/rev/e35d8e6618b3

(In reply to David Dorwin from comment #20)
> The closed event is missing from
> https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-
> media.html#event-summary and thus the #dom-eventclosed links are broken.

I have fixed this issue. I believe this is the last outstanding action for this bug. RESOLVED FIXED
https://dvcs.w3.org/hg/html-media/rev/c148640a920d

(In reply to Adrian Bateman [MSFT] from comment #22)
> https://dvcs.w3.org/hg/html-media/rev/e35d8e6618b3

I updated the algorithm to (always) schedule a task and explicitly uses the CDM. This clarifies some scenarios and makes the algorithm now consistent with the other MediaKeySession methods.

https://dvcs.w3.org/hg/html-media/rev/a79bce351b73

Note that "release(); release();" would result in two tasks, the second of which would be redundant. However, "release();" followed sometime later by another "release();" may result in exiting silently after the first step. As far as the application knows, there is no difference.

Do we need to explicitly handle release() being called in the CREATED state? 

createSession(), loadSession(), and release() all schedule tasks to do their work. Are these tasks required to run serially? (If so, does the spec need to be more explicit about this or is this implied by "schedule a task"?) If not, the createSession() task may not complete before release() is called.


It may actually be that we need to handle this in createSession() and loadSession() by adding a condition to check for a release as the first condition in the last substep of the "Schedule a task" step.

(In reply to David Dorwin from comment #25)
> Do we need to explicitly handle release() being called in the CREATED state? 

The behaviour of release() is implementation specific. There is no requirement to do anything with release(). If the implementation does cause a close to occur, the spec already allows moving from CREATED to CLOSED at any point (either because of release() or because the CDM wants to close the session).

> createSession(), loadSession(), and release() all schedule tasks to do their
> work. Are these tasks required to run serially? (If so, does the spec need
> to be more explicit about this or is this implied by "schedule a task"?) If
> not, the createSession() task may not complete before release() is called.

I think we need to change the createSession/loadSession task steps to say Spin the event loop until the MediaKeys constructor task is complete. The MediaKeys constructor task should say to spin the event loop until the CDM is loaded and initialized (step 2). Then we will need to handle release() being called before the session is completely created/loaded.

> It may actually be that we need to handle this in createSession() and
> loadSession() by adding a condition to check for a release as the first
> condition in the last substep of the "Schedule a task" step.

Perhaps all we need to do here is to make the first step of the release task spin the event loop until the MediaKeys constructor task is complete?

Unrelated to the above discussion, I rewrote some of the Session Close text in https://dvcs.w3.org/hg/html-media/rev/6374e2195b04.

Should release() cause all other tasks in the MediaKeySession() object to be aborted? For example, once release() is called, there is no need to continue generating a license request. On the other hand, an application might call update() to commit some data* then immediately call release().

* However, this use case doesn't make any sense since release() is supposed to cause the CDM to release/delete any resources associated with the session.

Would that use case cause a problem? I could see an app calling update() which causes a license to be cached and then on receiving release(), immediately tearing down the session. That should be fine. 

If release() is called while building a request, that should definitely be aborted. Or if release() is called after the app has received the response, but before update() has been called, the session should still be torn down and the app can drop the response on the floor.

(In reply to Joe Steele from comment #29)
> Would that use case cause a problem? I could see an app calling update()
> which causes a license to be cached and then on receiving release(),
> immediately tearing down the session. That should be fine. 

I assume you mean "cause a license to be stored persistently...".

If so, release() would cause it to be erased from persistent storage. release() is *not* close() - it actually releases all resources related to the session. See the discussion starting at Comment 16.

(In reply to David Dorwin from comment #30)
> (In reply to Joe Steele from comment #29)
> > Would that use case cause a problem? I could see an app calling update()
> > which causes a license to be cached and then on receiving release(),
> > immediately tearing down the session. That should be fine. 
> 
> I assume you mean "cause a license to be stored persistently...".
> 
> If so, release() would cause it to be erased from persistent storage.
> release() is *not* close() - it actually releases all resources related to
> the session. See the discussion starting at Comment 16.

Sorry - I should have been more clear. What I was saying is that the license would be persisted to disk AND then erased as part of the release() when the session is torn down. I was not suggesting that the license would be retained. You seemed to be indicating this might be a problem, which is what I was triggering off of.

Having said this -- I don't know how I will support this yet. I am not happy with the idea of tying a set of opaque key server responses to a session. It is unclear how applications will be able to determine which sessions to load or release without having visibility into the key server responses. This either restricts the key server severely in what keys it can return OR it places an additional burden on the application to be parse key responses to determine exactly what it in them. I don't have a better proposal at this point though.

(In reply to Joe Steele from comment #31)
> (In reply to David Dorwin from comment #30)
> > (In reply to Joe Steele from comment #29)
> > > Would that use case cause a problem? I could see an app calling update()
> > > which causes a license to be cached and then on receiving release(),
> > > immediately tearing down the session. That should be fine. 
> > 
> > I assume you mean "cause a license to be stored persistently...".
> > 
> > If so, release() would cause it to be erased from persistent storage.
> > release() is *not* close() - it actually releases all resources related to
> > the session. See the discussion starting at Comment 16.
> 
> Sorry - I should have been more clear. What I was saying is that the license
> would be persisted to disk AND then erased as part of the release() when the
> session is torn down. I was not suggesting that the license would be
> retained. You seemed to be indicating this might be a problem, which is what
> I was triggering off of.

I think I was thinking of something like the key release model where the update might need to be processed before releasing the session. I didn't have an exact use case in mind.

The reason an application might call update(response); release(); is that there is no way to wait for an update to be processed. Promises would address this (as well as other yet to be resolved issues in this bug) - see bug 25199.

> Having said this -- I don't know how I will support this yet. I am not happy
> with the idea of tying a set of opaque key server responses to a session. It
> is unclear how applications will be able to determine which sessions to load
> or release without having visibility into the key server responses. This
> either restricts the key server severely in what keys it can return OR it
> places an additional burden on the application to be parse key responses to
> determine exactly what it in them. I don't have a better proposal at this
> point though.

What is "this" that you need to support? I don't follow this paragraph or how it relates to this bug and close().

(In reply to David Dorwin from comment #32)
> (In reply to Joe Steele from comment #31)
> > Having said this -- I don't know how I will support this yet. I am not happy
> > with the idea of tying a set of opaque key server responses to a session. It
> > is unclear how applications will be able to determine which sessions to load
> > or release without having visibility into the key server responses. This
> > either restricts the key server severely in what keys it can return OR it
> > places an additional burden on the application to be parse key responses to
> > determine exactly what it in them. I don't have a better proposal at this
> > point though.
> 
> What is "this" that you need to support? I don't follow this paragraph or
> how it relates to this bug and close().

The "this" I was referring to is "clearing keys associated with a session". Once keys are persistently cached for Access DRM, you lose all concept of the session they were retrieved in. 

Thinking through supporting this -- the CDM will have to persist the session id along with any keys so it can look it up later to destroy it. My gut tells me that content providers will either always use release() or they will never use it. But since I can't be sure of that, I have to allow for both cases. This means that I can't clear all persistent licenses I have when I see a release, I have to be selective and clear just the ones tied to this session. And I have to mark shared keys differently (per comment 18) to avoid clearing those as well.

So just to be clear -- 
1) CDMs will have to store licenses in two groups segmented by session id and no-session (i.e. shared). 
2) CDMs should abort any current request processes when close() is received
3) CDMs should clear licenses associated with the current session off disk and in memory when release() is received.

What if the release method took a key ID (or set of key IDs) as a parameter? This would be much cleaner from my perspective as it gets away from the somewhat nebulous idea of a "session" as a unit of storage. This way the app could explicitly say:

    keys.release("MonthyPython.198237")

And get the very well-defined result that a particular key is cleared and no longer accessible to the CDM.

A couple quick notes:
* There is no such thing as a session-less "shared key" in the spec.
* Even if the core DRM does not support session IDs, it seems possible to build a shim around it that can associate session IDs with licenses/keys.

We're getting off topic, though, and we should probably return this bug to its original topic. I believe the remaining questions are in Comment 26 and Comment 28. Both might be addressed by Promises (bug 25199).

I think most of the remaining questions have been resolved with the implementation of promises (bug 25199).

(In reply to Adrian Bateman [MSFT] from comment #26)
> (In reply to David Dorwin from comment #25)
> > Do we need to explicitly handle release() being called in the CREATED state? 
> 
> The behaviour of release() is implementation specific. There is no
> requirement to do anything with release(). If the implementation does cause
> a close to occur, the spec already allows moving from CREATED to CLOSED at
> any point (either because of release() or because the CDM wants to close the
> session).

With promises implemented, a session is not returned until it has been initialized, which means release() cannot be called until that point.

release() is also now entirely asynchronous except for the closed check, so back-to-back calls cannot have different results.

> > createSession(), loadSession(), and release() all schedule tasks to do their
> > work. Are these tasks required to run serially? (If so, does the spec need
> > to be more explicit about this or is this implied by "schedule a task"?) If
> > not, the createSession() task may not complete before release() is called.
> 
> I think we need to change the createSession/loadSession task steps to say
> Spin the event loop until the MediaKeys constructor task is complete. The
> MediaKeys constructor task should say to spin the event loop until the CDM
> is loaded and initialized (step 2). Then we will need to handle release()
> being called before the session is completely created/loaded.

With promises implemented, the algorithms no longer schedule a task. As mentioned above, no session object is returned until createSession()/loadSession() complete, so release() cannot be called before that.

> > It may actually be that we need to handle this in createSession() and
> > loadSession() by adding a condition to check for a release as the first
> > condition in the last substep of the "Schedule a task" step.
> 
> Perhaps all we need to do here is to make the first step of the release task
> spin the event loop until the MediaKeys constructor task is complete?


(In reply to David Dorwin from comment #28)
> Should release() cause all other tasks in the MediaKeySession() object to be
> aborted? For example, once release() is called, there is no need to continue
> generating a license request. On the other hand, an application might call
> update() to commit some data* then immediately call release().
> 
> * However, this use case doesn't make any sense since release() is supposed
> to cause the CDM to release/delete any resources associated with the session.

With promises implemented, the algorithms no longer have tasks - just asynchronous steps - and license generation is performed before returning the session object. There is still the possibility of "session.update(); session.release();", but I guess implementations will need to handle that in a reasonable way.