Create Pepper-based CDM for EME WD (Blink changes)

Moving creation of WebContentDecryptionModule from Platform to public/web.

This is done in order to get the origin and WebFrame needed to create the
CDM. This is now similar to how WebMediaPlayer is created. This change adds
a Page supplement for MediaKeys to enable this.

BUG=250049, 353324
TEST=EME content tests pass (plus disabled ECK browser tests now pass)

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

[jrummell, 2014-03-11 18:02:20]

PTAL.

[ddorwin, 2014-03-12 01:01:09]

Looking good. I would change the description to focus on moving creation from
Platform to WebFrameClient. You can then note that this allows us to get the
origin and WebFrame.

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

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeys.cpp:80:
OwnPtr<blink::WebContentDecryptionModule> cdm =
adoptPtr(controller->client()->createContentDecryptionModule(context,
keySystem));
Just FYI, this is supposed to be done in a task, which makes things a bit
interesting. Basically, we won't have the cdm yet and will have to [have the new
MediaKeys] set it when it's initialized.

We'll probably need to either a) save a RefPtr to document or controller or b)
do this but defer initialization (new method on WebCDM) to the task.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeysClient.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysClient.h:1: /*
Use the new short copyright header for new code:
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/oSaYHvSLg2M

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysClient.h:47: virtual
blink::WebContentDecryptionModule*
createContentDecryptionModule(ExecutionContext*, const String& keySystem) = 0;
nit q: Is the context usually the first parameter? Usually const params are
first in Chrome, but I've seen otherwise in Blink.

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

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.cpp:1: /*
ditto

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:1: /*
ditto

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:35: #include
"modules/encryptedmedia/MediaKeysClient.h"
fwd declare

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:40: class
MediaKeysController FINAL : public Supplement<Page> {
I wonder whether Controller is the right name. Do the other controllers actually
do controlling?

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:43: MediaKeysClient*
client() const { return m_client; }
Is use of "const" common? We usually don't use const when returning non-const
pointers.

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
File Source/web/MediaKeysClientImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.cpp:1: /*
ditto

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.cpp:36: #include "WebViewImpl.h"
not used

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
File Source/web/MediaKeysClientImpl.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.h:1: /*
ditto

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.h:45: // WebCore::MediaKeysClient
----------------------------------------------
It seems like there must be a more prefered style than this.

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.cpp#n...
Source/web/WebViewImpl.cpp:391: provideMediaKeysTo(*m_page,
m_mediaKeysClientImpl.get());
Note how you've grouped it with user media here. :)

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.h
File Source/web/WebViewImpl.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.h#new...
Source/web/WebViewImpl.h:763: OwnPtr<MediaKeysClientImpl> m_mediaKeysClientImpl;
Should we maybe group this with other media ones above at 752-753?

[jrummell, 2014-03-17 20:48:37]

Updated.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeysClient.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysClient.h:1: /*
On 2014/03/12 01:01:09, ddorwin wrote:
> Use the new short copyright header for new code:
> https://groups.google.com/a/chromium.org/forum/#%21msg/blink-dev/oSaYHvSLg2M

Done.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysClient.h:47: virtual
blink::WebContentDecryptionModule*
createContentDecryptionModule(ExecutionContext*, const String& keySystem) = 0;
On 2014/03/12 01:01:09, ddorwin wrote:
> nit q: Is the context usually the first parameter? Usually const params are
> first in Chrome, but I've seen otherwise in Blink.

There are lots of cases in blink where const parameters are not always first
(e.g. public/web/WebBindings.h). Happy to change it if people think it is better
style. I don't see anything in the Blink coding style guidelines that mention
this.

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

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.cpp:1: /*
On 2014/03/12 01:01:09, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:1: /*
On 2014/03/12 01:01:09, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:35: #include
"modules/encryptedmedia/MediaKeysClient.h"
On 2014/03/12 01:01:09, ddorwin wrote:
> fwd declare

Done.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:40: class
MediaKeysController FINAL : public Supplement<Page> {
On 2014/03/12 01:01:09, ddorwin wrote:
> I wonder whether Controller is the right name. Do the other controllers
actually
> do controlling?

Most of the controllers I looked at (UserMediaController, MIDIController,
SpeechRecognitionController) do little work other than provide access to the
client (either as client() or by providing inline methods that call
m_client->something()).

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:43: MediaKeysClient*
client() const { return m_client; }
On 2014/03/12 01:01:09, ddorwin wrote:
> Is use of "const" common? We usually don't use const when returning non-const
> pointers.

I just copied what was in UserMediaController.h. Could also get rid of client(),
and create a passthru method createContentDecryptionModule().

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
File Source/web/MediaKeysClientImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.cpp:1: /*
On 2014/03/12 01:01:09, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.cpp:36: #include "WebViewImpl.h"
On 2014/03/12 01:01:09, ddorwin wrote:
> not used

Done.

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
File Source/web/MediaKeysClientImpl.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.h:1: /*
On 2014/03/12 01:01:09, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/195053004/diff/1/Source/web/MediaKeysClientIm...
Source/web/MediaKeysClientImpl.h:45: // WebCore::MediaKeysClient
----------------------------------------------
On 2014/03/12 01:01:09, ddorwin wrote:
> It seems like there must be a more prefered style than this.

I cloned UserMediaClientImpl.h. Looks like there are various styles, not sure
what is most prevalent (WebCore::MediaKeysClient implementation,
WebCore::MediaKeysClient:, no comment).

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.cpp#n...
Source/web/WebViewImpl.cpp:391: provideMediaKeysTo(*m_page,
m_mediaKeysClientImpl.get());
On 2014/03/12 01:01:09, ddorwin wrote:
> Note how you've grouped it with user media here. :)

:)

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.h
File Source/web/WebViewImpl.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/web/WebViewImpl.h#new...
Source/web/WebViewImpl.h:763: OwnPtr<MediaKeysClientImpl> m_mediaKeysClientImpl;
On 2014/03/12 01:01:09, ddorwin wrote:
> Should we maybe group this with other media ones above at 752-753?

Done.

[ddorwin, 2014-03-17 21:47:43]

Two minor comments. LGTM. Thanks.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:43: MediaKeysClient*
client() const { return m_client; }
On 2014/03/17 20:48:37, jrummell wrote:
> On 2014/03/12 01:01:09, ddorwin wrote:
> > Is use of "const" common? We usually don't use const when returning
non-const
> > pointers.
> 
> I just copied what was in UserMediaController.h. Could also get rid of
client(),
> and create a passthru method createContentDecryptionModule().

Yeah, I don't see an advantage to having the client pointer be separate in this
case. We're not passing it anywhere. Of course, sticking with consistent
patterns is nice.

https://codereview.chromium.org/195053004/diff/40001/Source/web/MediaKeysClie...
File Source/web/MediaKeysClientImpl.h (right):

https://codereview.chromium.org/195053004/diff/40001/Source/web/MediaKeysClie...
Source/web/MediaKeysClientImpl.h:19: // WebCore::MediaKeysClient
----------------------------------------------
I think "... implementation" is the better of the various styles.

[jrummell, 2014-03-17 22:42:13]

Updated.

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/195053004/diff/1/Source/modules/encryptedmedi...
Source/modules/encryptedmedia/MediaKeysController.h:43: MediaKeysClient*
client() const { return m_client; }
On 2014/03/17 21:47:43, ddorwin wrote:
> On 2014/03/17 20:48:37, jrummell wrote:
> > On 2014/03/12 01:01:09, ddorwin wrote:
> > > Is use of "const" common? We usually don't use const when returning
> non-const
> > > pointers.
> > 
> > I just copied what was in UserMediaController.h. Could also get rid of
> client(),
> > and create a passthru method createContentDecryptionModule().
> 
> Yeah, I don't see an advantage to having the client pointer be separate in
this
> case. We're not passing it anywhere. Of course, sticking with consistent
> patterns is nice.

Done. Converted to a passthru method rather than exposing client() that could
get misused.

https://codereview.chromium.org/195053004/diff/40001/Source/web/MediaKeysClie...
File Source/web/MediaKeysClientImpl.h (right):

https://codereview.chromium.org/195053004/diff/40001/Source/web/MediaKeysClie...
Source/web/MediaKeysClientImpl.h:19: // WebCore::MediaKeysClient
----------------------------------------------
On 2014/03/17 21:47:44, ddorwin wrote:
> I think "... implementation" is the better of the various styles.

Done.

[acolwell GONE FROM CHROMIUM, 2014-03-17 23:34:44]

looks good to me. Just a few comments.

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeys.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeys.cpp:77: ASSERT(document);
nit: No need for an ASSERT here since you'll NPE on the next line if it's null.

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeys.cpp:79: ASSERT(controller);
ditto.

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysClient.h (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysClient.h:27: void
provideMediaKeysTo(Page&, MediaKeysClient*);
Shouldn't this be provideMediaKeysControllerTo() since you are actually
attaching a MediaKeysController to the page? If so it seems like this decl
should be in the MediaKeysController header.

https://codereview.chromium.org/195053004/diff/60001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/web/WebViewImpl.c...
Source/web/WebViewImpl.cpp:392: provideMediaKeysTo(*m_page,
m_mediaKeysClientImpl.get());
Could you get away w/ passing MediaKeysClient& here instead since I'm assuming
we should never pass a null and m_mediaKeysClientImpl will outlive the page?

[eseidel, 2014-03-17 23:48:03]

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeys.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeys.cpp:80:
OwnPtr<blink::WebContentDecryptionModule> cdm =
adoptPtr(controller->createContentDecryptionModule(context, keySystem));
Is this really the pattern for web/platform apis, that they return a raw pointer
and expect you to adoptPtr it?  In Blink's implementation we normally use
PassOwnPtr for this, I guess there isn't a web/ equivalent?

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysClient.h (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysClient.h:27: void
provideMediaKeysTo(Page&, MediaKeysClient*);
It's also odd that this is in one header and defined in a completely different
file.

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysController.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysController.cpp:37: void
provideMediaKeysTo(Page& page, MediaKeysClient* client)
Who calls this?  Where is it declared?

https://codereview.chromium.org/195053004/diff/60001/Source/web/MediaKeysClie...
File Source/web/MediaKeysClientImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/web/MediaKeysClie...
Source/web/MediaKeysClientImpl.cpp:28: ASSERT(executionContext->isDocument());
doesn't toDocument ASSERT this for you?

https://codereview.chromium.org/195053004/diff/60001/Source/web/MediaKeysClie...
Source/web/MediaKeysClientImpl.cpp:31: WebSecurityOrigin
securityOrigin(executionContext->securityOrigin());
I'm slightly surprised these are not implicitly constructed (and thus you could
just pass this directly on the next line).

[jrummell, 2014-03-18 17:05:30]

Updated.

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeys.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeys.cpp:77: ASSERT(document);
On 2014/03/17 23:34:45, acolwell wrote:
> nit: No need for an ASSERT here since you'll NPE on the next line if it's
null.

Done.

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeys.cpp:79: ASSERT(controller);
On 2014/03/17 23:34:45, acolwell wrote:
> ditto.

Done.

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeys.cpp:80:
OwnPtr<blink::WebContentDecryptionModule> cdm =
adoptPtr(controller->createContentDecryptionModule(context, keySystem));
On 2014/03/17 23:48:03, eseidel wrote:
> Is this really the pattern for web/platform apis, that they return a raw
pointer
> and expect you to adoptPtr it?  In Blink's implementation we normally use
> PassOwnPtr for this, I guess there isn't a web/ equivalent?

Moved the adoptPtr() to MediaKeysClientImpl (where it crosses into Chromium).

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysClient.h (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysClient.h:27: void
provideMediaKeysTo(Page&, MediaKeysClient*);
On 2014/03/17 23:34:45, acolwell wrote:
> Shouldn't this be provideMediaKeysControllerTo() since you are actually
> attaching a MediaKeysController to the page? If so it seems like this decl
> should be in the MediaKeysController header.

I just followed what was done for UserMediaClient/Controller
(https://code.google.com/p/chromium/codesearch#search/&q=provideUserMediaTo&sq...).
Is there a better example to follow?

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysController.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysController.cpp:37: void
provideMediaKeysTo(Page& page, MediaKeysClient* client)
On 2014/03/17 23:48:03, eseidel wrote:
> Who calls this?  Where is it declared?

See previous comment on UserMediaClient/Controller.

https://codereview.chromium.org/195053004/diff/60001/Source/web/MediaKeysClie...
File Source/web/MediaKeysClientImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/web/MediaKeysClie...
Source/web/MediaKeysClientImpl.cpp:28: ASSERT(executionContext->isDocument());
On 2014/03/17 23:48:03, eseidel wrote:
> doesn't toDocument ASSERT this for you?

Does. Removed.

https://codereview.chromium.org/195053004/diff/60001/Source/web/MediaKeysClie...
Source/web/MediaKeysClientImpl.cpp:31: WebSecurityOrigin
securityOrigin(executionContext->securityOrigin());
On 2014/03/17 23:48:03, eseidel wrote:
> I'm slightly surprised these are not implicitly constructed (and thus you
could
> just pass this directly on the next line).

Doesn't appear that implicit constructor exists.

https://codereview.chromium.org/195053004/diff/60001/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/60001/Source/web/WebViewImpl.c...
Source/web/WebViewImpl.cpp:392: provideMediaKeysTo(*m_page,
m_mediaKeysClientImpl.get());
On 2014/03/17 23:34:45, acolwell wrote:
> Could you get away w/ passing MediaKeysClient& here instead since I'm assuming
> we should never pass a null and m_mediaKeysClientImpl will outlive the page?

Done.

[acolwell GONE FROM CHROMIUM, 2014-03-19 00:28:53]

LGTM % comments

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysClient.h (right):

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysClient.h:28: void
provideMediaKeysTo(Page&, MediaKeysClient*);
I still think this is weird. Is there any reason not to just put this in a
static  MediaKeysController::provideTo(Page&) method and call that. It seems
like that would make things a lot clearer at the call site and this decl
wouldn't seem out of place.

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysController.h:26: static
PassOwnPtr<MediaKeysController> create(MediaKeysClient*);
nit: Can this and supplementName() be private?

[jrummell, 2014-03-19 00:42:41]

+abarth@ for OWNERS review since we have discussed these changes.
eseidel@, you're welcome to review it again.

I followed what was done for UserMediaClient/Controller
(https://code.google.com/p/chromium/codesearch#search/&q=provideUserMediaTo&sq...).
Since there have been a bunch of question about naming, is there a better
example to follow?

[abarth-chromium, 2014-03-20 17:09:23]

On 2014/03/17 23:48:03, eseidel wrote:
> Is this really the pattern for web/platform apis, that they return a raw
pointer
> and expect you to adoptPtr it?  In Blink's implementation we normally use
> PassOwnPtr for this, I guess there isn't a web/ equivalent?

Unfortunately, yes.  We've tried a couple approaches to making a smart pointer
to transfer from scoped_ptr to OwnPtr, but we haven't quite figured it out yet. 
Yet another thing that will improve when we merge repos.  :)

[jrummell, 2014-03-20 19:17:31]

Updated to move provideMediaKeysTo() to MediaKeysController.

Once we agree on the correct pattern, I'll work on another CL to update the
other Supplement<Page> classes to use the same pattern.

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysClient.h (right):

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysClient.h:28: void
provideMediaKeysTo(Page&, MediaKeysClient*);
On 2014/03/19 00:28:54, acolwell wrote:
> I still think this is weird. Is there any reason not to just put this in a
> static  MediaKeysController::provideTo(Page&) method and call that. It seems
> like that would make things a lot clearer at the call site and this decl
> wouldn't seem out of place.

Done. Still have to provide the client since we need MediaKeysClientImpl, and
MediaKeysController in modules/ can't see it.

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/195053004/diff/80001/Source/modules/encrypted...
Source/modules/encryptedmedia/MediaKeysController.h:26: static
PassOwnPtr<MediaKeysController> create(MediaKeysClient*);
On 2014/03/19 00:28:54, acolwell wrote:
> nit: Can this and supplementName() be private?

Now that provideMediaKeysTo is in this class, not need for create().
supplementName() made local.

[eseidel, 2014-03-21 17:48:04]

lgtm

https://codereview.chromium.org/195053004/diff/120001/Source/web/MediaKeysCli...
File Source/web/MediaKeysClientImpl.cpp (right):

https://codereview.chromium.org/195053004/diff/120001/Source/web/MediaKeysCli...
Source/web/MediaKeysClientImpl.cpp:25: WebFrameImpl* webFrame =
WebFrameImpl::fromFrame(document->frame());
Hopefully this is never called on a detached document. :)

var detachedDocument = iframe.contentDocument;
iframe.src = "newthing";
detachedDocument.createContentDescriptionModule(); // boom!  document->frame()
is null!

[jrummell, 2014-03-22 06:41:38]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-03-22 06:41:49]

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

[commit-bot: I haz the power, 2014-03-22 20:50:01]

Change committed as 169797