Convert between |init_data_type| and content type.

Convert between |init_data_type| and content type.

This allows both to be used while Blink and the tests are transitioned.

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

[ddorwin, 2014-08-13 20:59:55]

Some of my comments relate to later (or potentially before) changes.

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/c...
File content/renderer/media/crypto/proxy_media_keys.cc (right):

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/c...
content/renderer/media/crypto/proxy_media_keys.cc:60: if (init_data_type ==
"audio/mp4" ||
After we transition unprefixed, these will only be passed by
WebMediaPlayerAndroid::GenerateKeyRequestInternal(). At that time, we should do
the conversion there and only support the new values here, probably addressing
the TODO above at the same time.

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
File content/renderer/media/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:88: //
TODO(ddorwin): Guard against this in supported types check and remove this.
Here's another TODO for more pushing stuff up into Blink.

This may already be handled as it is step #3 of
https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-...

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:102:
DLOG(WARNING) << "init_data_type '" << init_data_type_as_ascii
IIRC, the DLOG_IF was a heads up that you may have used an old type. This will
be relevant after this conversion period, so it probably makes sense to just
leave it as it was. That also simplifies the logic below (even though it runs
more than might be necessary).

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:105: //
TODO(sandersd): Move to ProxyDecryptor. http://crbug.com/385874
When? After converting the prefixed APIs and tests?
TODOs should have a timeline if possible.

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:106: if
(init_data_type_as_ascii == "audio/mp4" ||
Use braces for this complex multi-line conditional.

https://codereview.chromium.org/472493003/diff/20001/media/cdm/ppapi/cdm_wrap...
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/472493003/diff/20001/media/cdm/ppapi/cdm_wrap...
media/cdm/ppapi/cdm_wrapper.h:130: // Starting in cdm::Host_6 |init_data_type|
is a registered type instead of a
"Prior to CDM_6, |init_data_type| was a mime type."
           ^ It's not actually Host.

[sandersd (OOO until July 31), 2014-08-13 21:36:20]

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/c...
File content/renderer/media/crypto/proxy_media_keys.cc (right):

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/c...
content/renderer/media/crypto/proxy_media_keys.cc:60: if (init_data_type ==
"audio/mp4" ||
On 2014/08/13 20:59:54, ddorwin wrote:
> After we transition unprefixed, these will only be passed by
> WebMediaPlayerAndroid::GenerateKeyRequestInternal(). At that time, we should
do
> the conversion there and only support the new values here, probably addressing
> the TODO above at the same time.

Done.

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
File content/renderer/media/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:88: //
TODO(ddorwin): Guard against this in supported types check and remove this.
On 2014/08/13 20:59:54, ddorwin wrote:
> Here's another TODO for more pushing stuff up into Blink.
> 
> This may already be handled as it is step #3 of
>
https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-...

Done.

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:102:
DLOG(WARNING) << "init_data_type '" << init_data_type_as_ascii
On 2014/08/13 20:59:54, ddorwin wrote:
> IIRC, the DLOG_IF was a heads up that you may have used an old type. This will
> be relevant after this conversion period, so it probably makes sense to just
> leave it as it was. That also simplifies the logic below (even though it runs
> more than might be necessary).

Done.

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:105: //
TODO(sandersd): Move to ProxyDecryptor. http://crbug.com/385874
On 2014/08/13 20:59:55, ddorwin wrote:
> When? After converting the prefixed APIs and tests?
> TODOs should have a timeline if possible.

Done.

https://codereview.chromium.org/472493003/diff/20001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:106: if
(init_data_type_as_ascii == "audio/mp4" ||
On 2014/08/13 20:59:54, ddorwin wrote:
> Use braces for this complex multi-line conditional.

Done.

https://codereview.chromium.org/472493003/diff/20001/media/cdm/ppapi/cdm_wrap...
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/472493003/diff/20001/media/cdm/ppapi/cdm_wrap...
media/cdm/ppapi/cdm_wrapper.h:130: // Starting in cdm::Host_6 |init_data_type|
is a registered type instead of a
On 2014/08/13 20:59:55, ddorwin wrote:
> "Prior to CDM_6, |init_data_type| was a mime type."
>            ^ It's not actually Host.

Done.

[ddorwin, 2014-08-13 21:43:35]

LGTM % comments

https://codereview.chromium.org/472493003/diff/40001/content/renderer/media/c...
File content/renderer/media/crypto/proxy_decryptor.cc (right):

https://codereview.chromium.org/472493003/diff/40001/content/renderer/media/c...
content/renderer/media/crypto/proxy_decryptor.cc:132: // Translate content
types.
nit: "Convert MIME type's used in the prefixed implementation."

https://codereview.chromium.org/472493003/diff/40001/content/renderer/media/c...
content/renderer/media/crypto/proxy_decryptor.cc:139: init_data_type =
content_type;
I wonder if we can add NOTREACHED(). If it only comes from ChunkDemuxer or
WMPA/WMPI, we know the values.

[sandersd (OOO until July 31), 2014-08-13 21:50:59]

https://codereview.chromium.org/472493003/diff/40001/content/renderer/media/c...
File content/renderer/media/crypto/proxy_decryptor.cc (right):

https://codereview.chromium.org/472493003/diff/40001/content/renderer/media/c...
content/renderer/media/crypto/proxy_decryptor.cc:132: // Translate content
types.
On 2014/08/13 21:43:34, ddorwin wrote:
> nit: "Convert MIME type's used in the prefixed implementation."

Done.

https://codereview.chromium.org/472493003/diff/40001/content/renderer/media/c...
content/renderer/media/crypto/proxy_decryptor.cc:139: init_data_type =
content_type;
On 2014/08/13 21:43:34, ddorwin wrote:
> I wonder if we can add NOTREACHED(). If it only comes from ChunkDemuxer or
> WMPA/WMPI, we know the values.

Well, it's worth a try.

[sandersd (OOO until July 31), 2014-08-13 21:51:02]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-08-13 21:54:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sandersd@chromium.org/472493003/60001

[ddorwin, 2014-08-13 22:26:59]

https://codereview.chromium.org/472493003/diff/60001/content/renderer/media/w...
File content/renderer/media/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/472493003/diff/60001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:98: if
(init_data_type_as_ascii == "audio/mp4" ||
You need to call something like ToLowerCase().

[sandersd (OOO until July 31), 2014-08-13 22:34:43]

https://codereview.chromium.org/472493003/diff/60001/content/renderer/media/w...
File content/renderer/media/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/472493003/diff/60001/content/renderer/media/w...
content/renderer/media/webcontentdecryptionmodulesession_impl.cc:98: if
(init_data_type_as_ascii == "audio/mp4" ||
On 2014/08/13 22:26:59, ddorwin wrote:
> You need to call something like ToLowerCase().

Done.

[sandersd (OOO until July 31), 2014-08-13 22:34:47]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-08-13 22:38:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sandersd@chromium.org/472493003/80001

[commit-bot: I haz the power, 2014-08-14 10:43:25]

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

[commit-bot: I haz the power, 2014-08-14 16:01:44]

Committed patchset #5 (80001) as 289605