Update WebContentDecryptionModuleSessionImpl to the latest EME spec.

Update WebContentDecryptionModuleSessionImpl to the latest EME spec.

- generateKeyRequest() -> initializeNewSession()
- close() -> release()

This change is made to match the latest EME spec. The deprecated methods
will be dropped after Blink is updated. The Blink side change is at:
https://codereview.chromium.org/111043004/

BUG=224786
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244029

[xhwang, 2014-01-04 01:03:20]

PTAL

[acolwell GONE FROM CHROMIUM, 2014-01-06 20:27:17]

lgtm

[ddorwin, 2014-01-07 00:57:44]

lgtm

[ddorwin, 2014-01-07 01:17:58]

https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
File content/renderer/media/webcontentdecryptionmodulesession_impl.h (right):

https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual void
initialize(const blink::WebString& mime_type,
I don't think "initialize" is a good long term name.

If a session object was created with loadSession(), initialize() wouldn't make
sense. In that scenario, GKR() is more accurate.

Also, the spec now says to "Process type and initData" because there is no
guarantee that a request will be generated.

I don't have any suggestions better than processInitData(). I had a hard time
with the spec language too.

[xhwang, 2014-01-07 01:38:25]

comments only

https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
File content/renderer/media/webcontentdecryptionmodulesession_impl.h (right):

https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual void
initialize(const blink::WebString& mime_type,
On 2014/01/07 01:17:59, ddorwin wrote:
> I don't think "initialize" is a good long term name.
> 
> If a session object was created with loadSession(), initialize() wouldn't make
> sense. In that scenario, GKR() is more accurate.
> 
> Also, the spec now says to "Process type and initData" because there is no
> guarantee that a request will be generated.
> 
> I don't have any suggestions better than processInitData(). I had a hard time
> with the spec language too.

I like "initialize" because it's generic and can be interpreted as needed
(similar to "update"). I agree GKR is more accurate about what we do, but in
chromium implementation we also generate the session ID in this call. So GKR
doesn't cover all what we do.

Also, I feel "init"ialize("init"_data) matches pretty well. But this is not
important :)

For loadSession, the proposal is loadSession(DOMString sessionId), which doesn't
pass in a init data, so "initialize" or "GKR" won't be called in this class.

In summary, I like "initialize" better, but I am okay with GKR or other names if
you prefer :)

[ddorwin, 2014-01-07 02:41:37]

https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
File content/renderer/media/webcontentdecryptionmodulesession_impl.h (right):

https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual void
initialize(const blink::WebString& mime_type,
On 2014/01/07 01:17:59, ddorwin wrote:
> I don't think "initialize" is a good long term name.
> 
> If a session object was created with loadSession(), initialize() wouldn't make
> sense. In that scenario, GKR() is more accurate.
> 
> Also, the spec now says to "Process type and initData" because there is no
> guarantee that a request will be generated.
> 
> I don't have any suggestions better than processInitData(). I had a hard time
> with the spec language too.

Maybe something like finishCreation but better. If you think about the scenario
where we have a load() method here, the session has already been "created" and
load is finishing the work of loadSession(). The two methods should be similar.

I think the problem becomes more apparent when you look at the Blink side where
we have initializeTimer. What does the timer do? Would we have two separate
timers? If so, how would they be identified?

https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual void
initialize(const blink::WebString& mime_type,
On 2014/01/07 01:38:25, xhwang wrote:
> On 2014/01/07 01:17:59, ddorwin wrote:
> > I don't think "initialize" is a good long term name.
> > 
> > If a session object was created with loadSession(), initialize() wouldn't
make
> > sense. In that scenario, GKR() is more accurate.
> > 
> > Also, the spec now says to "Process type and initData" because there is no
> > guarantee that a request will be generated.
> > 
> > I don't have any suggestions better than processInitData(). I had a hard
time
> > with the spec language too.
> 
> I like "initialize" because it's generic and can be interpreted as needed
> (similar to "update"). I agree GKR is more accurate about what we do, but in
> chromium implementation we also generate the session ID in this call. So GKR
> doesn't cover all what we do.
> 
> Also, I feel "init"ialize("init"_data) matches pretty well. But this is not
> important :)
> 
> For loadSession, the proposal is loadSession(DOMString sessionId), which
doesn't
> pass in a init data, so "initialize" or "GKR" won't be called in this class.
> 
> In summary, I like "initialize" better, but I am okay with GKR or other names
if
> you prefer :)

The problem with "initialize" is that something should always be initialized but
it doesn't apply to loadSession().

If you think about the scenario where we have a load() method here, the session
has already been "created" and load is finishing the work of loadSession(). The
two methods should be similar.

Maybe something like finishNewSessionCreation() or initializeCreatedSession()
but better. 

I also think the problem becomes more apparent when you look at the Blink side
where we have initializeTimer. What does the timer do? Would we have two
separate timers? If so, how would they be identified?

[xhwang, 2014-01-07 21:44:20]

On 2014/01/07 02:41:37, ddorwin wrote:
>
https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
> File content/renderer/media/webcontentdecryptionmodulesession_impl.h (right):
> 
>
https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
> content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual
void
> initialize(const blink::WebString& mime_type,
> On 2014/01/07 01:17:59, ddorwin wrote:
> > I don't think "initialize" is a good long term name.
> > 
> > If a session object was created with loadSession(), initialize() wouldn't
make
> > sense. In that scenario, GKR() is more accurate.
> > 
> > Also, the spec now says to "Process type and initData" because there is no
> > guarantee that a request will be generated.
> > 
> > I don't have any suggestions better than processInitData(). I had a hard
time
> > with the spec language too.
> 
> Maybe something like finishCreation but better. If you think about the
scenario
> where we have a load() method here, the session has already been "created" and
> load is finishing the work of loadSession(). The two methods should be
similar.
> 
> I think the problem becomes more apparent when you look at the Blink side
where
> we have initializeTimer. What does the timer do? Would we have two separate
> timers? If so, how would they be identified?
> 
>
https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
> content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual
void
> initialize(const blink::WebString& mime_type,
> On 2014/01/07 01:38:25, xhwang wrote:
> > On 2014/01/07 01:17:59, ddorwin wrote:
> > > I don't think "initialize" is a good long term name.
> > > 
> > > If a session object was created with loadSession(), initialize() wouldn't
> make
> > > sense. In that scenario, GKR() is more accurate.
> > > 
> > > Also, the spec now says to "Process type and initData" because there is no
> > > guarantee that a request will be generated.
> > > 
> > > I don't have any suggestions better than processInitData(). I had a hard
> time
> > > with the spec language too.
> > 
> > I like "initialize" because it's generic and can be interpreted as needed
> > (similar to "update"). I agree GKR is more accurate about what we do, but in
> > chromium implementation we also generate the session ID in this call. So GKR
> > doesn't cover all what we do.
> > 
> > Also, I feel "init"ialize("init"_data) matches pretty well. But this is not
> > important :)
> > 
> > For loadSession, the proposal is loadSession(DOMString sessionId), which
> doesn't
> > pass in a init data, so "initialize" or "GKR" won't be called in this class.
> > 
> > In summary, I like "initialize" better, but I am okay with GKR or other
names
> if
> > you prefer :)
> 
> The problem with "initialize" is that something should always be initialized
but
> it doesn't apply to loadSession().
> 
> If you think about the scenario where we have a load() method here, the
session
> has already been "created" and load is finishing the work of loadSession().
The
> two methods should be similar.
> 
> Maybe something like finishNewSessionCreation() or initializeCreatedSession()
> but better. 
> 
> I also think the problem becomes more apparent when you look at the Blink side
> where we have initializeTimer. What does the timer do? Would we have two
> separate timers? If so, how would they be identified?

How about initializeNewSession() and loadExistingSession() ?

[ddorwin, 2014-01-07 21:54:38]

On 2014/01/07 21:44:20, xhwang wrote:
> On 2014/01/07 02:41:37, ddorwin wrote:
> >
>
https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
> > File content/renderer/media/webcontentdecryptionmodulesession_impl.h
(right):
> > 
> >
>
https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
> > content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual
> void
> > initialize(const blink::WebString& mime_type,
> > On 2014/01/07 01:17:59, ddorwin wrote:
> > > I don't think "initialize" is a good long term name.
> > > 
> > > If a session object was created with loadSession(), initialize() wouldn't
> make
> > > sense. In that scenario, GKR() is more accurate.
> > > 
> > > Also, the spec now says to "Process type and initData" because there is no
> > > guarantee that a request will be generated.
> > > 
> > > I don't have any suggestions better than processInitData(). I had a hard
> time
> > > with the spec language too.
> > 
> > Maybe something like finishCreation but better. If you think about the
> scenario
> > where we have a load() method here, the session has already been "created"
and
> > load is finishing the work of loadSession(). The two methods should be
> similar.
> > 
> > I think the problem becomes more apparent when you look at the Blink side
> where
> > we have initializeTimer. What does the timer do? Would we have two separate
> > timers? If so, how would they be identified?
> > 
> >
>
https://codereview.chromium.org/101693009/diff/1/content/renderer/media/webco...
> > content/renderer/media/webcontentdecryptionmodulesession_impl.h:37: virtual
> void
> > initialize(const blink::WebString& mime_type,
> > On 2014/01/07 01:38:25, xhwang wrote:
> > > On 2014/01/07 01:17:59, ddorwin wrote:
> > > > I don't think "initialize" is a good long term name.
> > > > 
> > > > If a session object was created with loadSession(), initialize()
wouldn't
> > make
> > > > sense. In that scenario, GKR() is more accurate.
> > > > 
> > > > Also, the spec now says to "Process type and initData" because there is
no
> > > > guarantee that a request will be generated.
> > > > 
> > > > I don't have any suggestions better than processInitData(). I had a hard
> > time
> > > > with the spec language too.
> > > 
> > > I like "initialize" because it's generic and can be interpreted as needed
> > > (similar to "update"). I agree GKR is more accurate about what we do, but
in
> > > chromium implementation we also generate the session ID in this call. So
GKR
> > > doesn't cover all what we do.
> > > 
> > > Also, I feel "init"ialize("init"_data) matches pretty well. But this is
not
> > > important :)
> > > 
> > > For loadSession, the proposal is loadSession(DOMString sessionId), which
> > doesn't
> > > pass in a init data, so "initialize" or "GKR" won't be called in this
class.
> > > 
> > > In summary, I like "initialize" better, but I am okay with GKR or other
> names
> > if
> > > you prefer :)
> > 
> > The problem with "initialize" is that something should always be initialized
> but
> > it doesn't apply to loadSession().
> > 
> > If you think about the scenario where we have a load() method here, the
> session
> > has already been "created" and load is finishing the work of loadSession().
> The
> > two methods should be similar.
> > 
> > Maybe something like finishNewSessionCreation() or
initializeCreatedSession()
> > but better. 
> > 
> > I also think the problem becomes more apparent when you look at the Blink
side
> > where we have initializeTimer. What does the timer do? Would we have two
> > separate timers? If so, how would they be identified?
> 
> How about initializeNewSession() and loadExistingSession() ?

SG. See how that works in the Blink side CL, which is more complex.

[xhwang, 2014-01-08 19:59:25]

This CL is updated with the new name. I'll update the blink side shortly.

[xhwang, 2014-01-09 01:57:39]

On 2014/01/08 19:59:25, xhwang wrote:
> This CL is updated with the new name. I'll update the blink side shortly.

Does the new names look good in Blink? If so, can I commit this CL?

[ddorwin, 2014-01-09 02:00:52]

On 2014/01/09 01:57:39, xhwang wrote:
> On 2014/01/08 19:59:25, xhwang wrote:
> > This CL is updated with the new name. I'll update the blink side shortly.
> 
> Does the new names look good in Blink? If so, can I commit this CL?

Yes, but you should wait for public/platform OWNERS' blessing just in case.

[xhwang, 2014-01-09 20:07:36]

rebase only

[commit-bot: I haz the power, 2014-01-09 22:40:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/101693009/220001

[commit-bot: I haz the power, 2014-01-10 00:35:06]

Change committed as 244029