Set callback interface on WebCDMSession after creation.

Set callback interface on WebCDMSession after creation.

In order to allow MediaKeySession to be created only when resolving
the promise, add the ability to set the client callback interface on
WebContentDecryptionModuleSession after the object is created.

BUG=358271
TEST=EME layouts tests pass

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178407

[jrummell, 2014-07-09 23:54:14]

PTAL. Chromium changes in https://codereview.chromium.org/380953002/ and need to
be submitted first.

[ddorwin, 2014-07-10 23:22:30]

https://codereview.chromium.org/379343003/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeySession.cpp (right):

https://codereview.chromium.org/379343003/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:81:
m_session->setClientInterface(this);
The point of this change is to allow refactoring. Is there a reason not to do
that refactoring in this CL, which will land after the Chromium side anyway, so
that we can make sure the interface and Chromium changes enable that
refactoring?

[jrummell, 2014-07-11 00:55:54]

Discussed off-line. Will wait until EME promises land.

https://codereview.chromium.org/379343003/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeySession.cpp (right):

https://codereview.chromium.org/379343003/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeySession.cpp:81:
m_session->setClientInterface(this);
On 2014/07/10 23:22:30, ddorwin wrote:
> The point of this change is to allow refactoring. Is there a reason not to do
> that refactoring in this CL, which will land after the Chromium side anyway,
so
> that we can make sure the interface and Chromium changes enable that
> refactoring?

Without promises implemented, all responses come back as events. Need to
register the interface so we get something back.

[jrummell, 2014-07-16 23:41:24]

Updated to use the promises code. MediaKeySession now created only when
resolving the promise. PTAL.

[ddorwin, 2014-07-17 00:41:05]

LGTM

I assume the Chromium changes are already implemented or will be before landing.

[jrummell, 2014-07-17 00:45:43]

+acolwell, for review as media blink owner.
+abarth, for proper OWNERS review of public/platform.

[abarth-chromium, 2014-07-17 05:23:46]

public/platform LGTM

[acolwell GONE FROM CHROMIUM, 2014-07-17 17:45:25]

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySession.cpp (right):

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.cpp:229: MediaKeySession* session
= adoptRefCountedGarbageCollectedWillBeNoop(new
MediaKeySession(executionContext(), m_mediaKeys, m_cdmSession.release()));
Isn't this a memory leak for a non-Oilpan build?

[jrummell, 2014-07-17 19:03:08]

Answering acolwell's question.

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySession.cpp (right):

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.cpp:229: MediaKeySession* session
= adoptRefCountedGarbageCollectedWillBeNoop(new
MediaKeySession(executionContext(), m_mediaKeys, m_cdmSession.release()));
On 2014/07/17 17:45:25, acolwell wrote:
> Isn't this a memory leak for a non-Oilpan build?

Shouldn't be. The object is passed back to JavaScript with the resolve() call
below, so it owns the reference. There are a bunch of tests (e.g.
LayoutTests/media/encrypted-media/encrypted-media-lifetime-multiple-mediakeys.html)
that count the ActiveDOMObjects, and the MediaKeySession objects do get cleaned
up when they aren't needed anymore.

[acolwell GONE FROM CHROMIUM, 2014-07-17 19:16:04]

lgtm

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySession.cpp (right):

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.cpp:229: MediaKeySession* session
= adoptRefCountedGarbageCollectedWillBeNoop(new
MediaKeySession(executionContext(), m_mediaKeys, m_cdmSession.release()));
On 2014/07/17 19:03:07, jrummell wrote:
> On 2014/07/17 17:45:25, acolwell wrote:
> > Isn't this a memory leak for a non-Oilpan build?
> 
> Shouldn't be. The object is passed back to JavaScript with the resolve() call
> below, so it owns the reference. There are a bunch of tests (e.g.
>
LayoutTests/media/encrypted-media/encrypted-media-lifetime-multiple-mediakeys.html)
> that count the ActiveDOMObjects, and the MediaKeySession objects do get
cleaned
> up when they aren't needed anymore.

Ok. I'm just not used to the return value of adoptXXX being handed to a raw
pointer variable. I would have expected this to go into a
RefPtr<MediaKeySession>.

[jrummell, 2014-07-17 21:33:48]

Thanks for the reviews. I changed MKS* to RefPtrWillBeRawPtr<MKS> for clarity.
Compiles with and without Oilpan enabled. Ran the tests using ASAN to verify MKS
not leaked.

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeySession.cpp (right):

https://codereview.chromium.org/379343003/diff/20001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeySession.cpp:229: MediaKeySession* session
= adoptRefCountedGarbageCollectedWillBeNoop(new
MediaKeySession(executionContext(), m_mediaKeys, m_cdmSession.release()));
On 2014/07/17 19:16:04, acolwell wrote:
> On 2014/07/17 19:03:07, jrummell wrote:
> > On 2014/07/17 17:45:25, acolwell wrote:
> > > Isn't this a memory leak for a non-Oilpan build?
> > 
> > Shouldn't be. The object is passed back to JavaScript with the resolve()
call
> > below, so it owns the reference. There are a bunch of tests (e.g.
> >
>
LayoutTests/media/encrypted-media/encrypted-media-lifetime-multiple-mediakeys.html)
> > that count the ActiveDOMObjects, and the MediaKeySession objects do get
> cleaned
> > up when they aren't needed anymore.
> 
> Ok. I'm just not used to the return value of adoptXXX being handed to a raw
> pointer variable. I would have expected this to go into a
> RefPtr<MediaKeySession>. 

For clarity, changed to use RefPtrWillBeRawPtr<>.

[jrummell, 2014-07-17 21:37:13]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-07-17 21:38:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jrummell@chromium.org/379343003/40001

[commit-bot: I haz the power, 2014-07-17 22:42:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[commit-bot: I haz the power, 2014-07-17 23:31:16]

Change committed as 178407