Encrypted Media: Add DefaultCdmFactory.

Encrypted Media: Add DefaultCdmFactory.

The DefaultCdmFactory is the default CdmFactory provided by media/.

BUG=422730, 441957
Committed: https://crrev.com/55f65bd702f7a21f0068159e580a9943b3b641fb
Cr-Commit-Position: refs/heads/master@{#309451}

[xhwang, 2014-12-19 17:30:18]

xhwang@chromium.org changed reviewers:
+ ddorwin@chromium.org

[xhwang, 2014-12-19 17:30:19]

ddorwin: PTAL
gunsch: FYI

[ddorwin, 2014-12-19 18:16:56]

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
File media/blink/cdm_session_adapter.cc (right):

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
media/blink/cdm_session_adapter.cc:36: if (cdm_factory) {
If we have a default, can this ever be null?

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
media/blink/cdm_session_adapter.cc:37: media_keys_ = cdm_factory->Create(
Clear Key isn't added to the existing factories. How can Clear Key be created on
desktop and Android?

https://codereview.chromium.org/814143004/diff/1/media/cdm/default_cdm_factor...
File media/cdm/default_cdm_factory.cc (right):

https://codereview.chromium.org/814143004/diff/1/media/cdm/default_cdm_factor...
media/cdm/default_cdm_factory.cc:22: const SessionReadyCB& session_ready_cb,
You or John will have to deal with a conflict on this line.

https://codereview.chromium.org/814143004/diff/1/media/cdm/proxy_decryptor.cc
File media/cdm/proxy_decryptor.cc (right):

https://codereview.chromium.org/814143004/diff/1/media/cdm/proxy_decryptor.cc...
media/cdm/proxy_decryptor.cc:203: if (!cdm_factory)
ditto

[gunsch, 2014-12-19 19:06:18]

gunsch@chromium.org changed reviewers:
+ gunsch@chromium.org

[gunsch, 2014-12-19 19:06:19]

thanks for FYI, no comments

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
File media/blink/cdm_session_adapter.cc (right):

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
media/blink/cdm_session_adapter.cc:37: media_keys_ = cdm_factory->Create(
On 2014/12/19 18:16:56, ddorwin wrote:
> Clear Key isn't added to the existing factories. How can Clear Key be created
on
> desktop and Android?

I think AesDecryptor is still created in RenderCdmFactory:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

[xhwang, 2014-12-19 19:18:17]

comments addressed

[xhwang, 2014-12-19 19:18:30]

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
File media/blink/cdm_session_adapter.cc (right):

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
media/blink/cdm_session_adapter.cc:36: if (cdm_factory) {
On 2014/12/19 18:16:56, ddorwin wrote:
> If we have a default, can this ever be null?

Good point. Done.

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
media/blink/cdm_session_adapter.cc:37: media_keys_ = cdm_factory->Create(
On 2014/12/19 19:06:19, gunsch wrote:
> On 2014/12/19 18:16:56, ddorwin wrote:
> > Clear Key isn't added to the existing factories. How can Clear Key be
created
> on
> > desktop and Android?
> 
> I think AesDecryptor is still created in RenderCdmFactory:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

Thanks for the link!

https://codereview.chromium.org/814143004/diff/1/media/blink/cdm_session_adap...
media/blink/cdm_session_adapter.cc:37: media_keys_ = cdm_factory->Create(
On 2014/12/19 18:16:56, ddorwin wrote:
> Clear Key isn't added to the existing factories. How can Clear Key be created
on
> desktop and Android?

See gunsch's reply. We still create AesDecryptor in RenderCdmFactory (which
should be removed in the last CL ;)

We also use RenderCdmFactory on Android, so Clear Key is taken care of:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

https://codereview.chromium.org/814143004/diff/1/media/cdm/default_cdm_factor...
File media/cdm/default_cdm_factory.cc (right):

https://codereview.chromium.org/814143004/diff/1/media/cdm/default_cdm_factor...
media/cdm/default_cdm_factory.cc:22: const SessionReadyCB& session_ready_cb,
On 2014/12/19 18:16:56, ddorwin wrote:
> You or John will have to deal with a conflict on this line.

Acknowledged.

https://codereview.chromium.org/814143004/diff/1/media/cdm/proxy_decryptor.cc
File media/cdm/proxy_decryptor.cc (right):

https://codereview.chromium.org/814143004/diff/1/media/cdm/proxy_decryptor.cc...
media/cdm/proxy_decryptor.cc:203: if (!cdm_factory)
On 2014/12/19 18:16:56, ddorwin wrote:
> ditto

Done.

[ddorwin, 2014-12-19 19:21:13]

lgtm

[xhwang, 2014-12-19 19:22:42]

xhwang@chromium.org changed reviewers:
+ jamesr@chromium.org

[xhwang, 2014-12-19 19:22:43]

jamesr: Please OWNERS review the change in
mojo/services/html_viewer/webmediaplayer_factory.cc. Thanks!

[xhwang, 2014-12-19 21:18:48]

xhwang@chromium.org changed reviewers:
+ aa@chromium.org

[xhwang, 2014-12-19 21:18:48]

It's the weekend before holiday so I am not sure whether jamesr@ is available
today.

aa: Could you please OWNERS review the change in
mojo/services/html_viewer/webmediaplayer_factory.cc? It's a trivial change.

[jamesr, 2014-12-21 17:05:09]

aa and I are both out on vacation, actually. I'll be back on the 29th. Feel free
to TBR minor patches if you need to make progress before then.

lgtm

[xhwang, 2014-12-22 05:38:45]

On 2014/12/21 17:05:09, jamesr wrote:
> aa and I are both out on vacation, actually. I'll be back on the 29th. Feel
free
> to TBR minor patches if you need to make progress before then.
> 
> lgtm

Thanks for the info!

[xhwang, 2014-12-22 05:38:49]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-12-22 05:39:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/814143004/20001

[commit-bot: I haz the power, 2014-12-22 05:45:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-22 05:45:09]

Try jobs failed on following builders:
  android_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)

[xhwang, 2014-12-22 17:57:51]

rebase only

[xhwang, 2014-12-22 18:15:55]

rebase only

[xhwang, 2014-12-22 19:22:24]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-12-22 19:22:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/814143004/60001

[commit-bot: I haz the power, 2014-12-22 20:21:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-22 20:21:48]

Patchset 4 (id:??) landed as
https://crrev.com/55f65bd702f7a21f0068159e580a9943b3b641fb
Cr-Commit-Position: refs/heads/master@{#309451}