Fix: canPlayType() gives false positive for */ogg and audio/x-wav

Fix: canPlayType() gives false positive for */ogg and audio/x-wav

Adding the following mime types and their codecs in format_codec_mappings[].
This will prevent the browser from giving wrong result for "canPlayType()" query
for these mime types:

audio/ogg
video/ogg
application/ogg
audio/x-wav

Adding unit test for checking StrictMediaMimeType() method

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

[amogh.bihani, 2014-03-29 09:22:24]

PTAL.
I have added some of the mime types in the strict list for which we are
confident about the results.

[ddorwin, 2014-04-01 00:10:44]

I don't think this really addresses BUG=357665, at least not directly - this is
just some of the work towards being able to address that.

[ddorwin, 2014-04-01 00:10:44]

I don't think this really addresses BUG=357665, at least not directly - this is
just some of the work towards being able to address that.

[ddorwin, 2014-04-01 00:12:05]

On 2014/04/01 00:10:44, ddorwin wrote:
> I don't think this really addresses BUG=357665, at least not directly - this
is
> just some of the work towards being able to address that.

Oops, submitted to soon.

This seems fine to me. Please have acolwell and scherkus (added) review any
(non-EME) canPlayType() changes.

[amogh.bihani, 2014-04-01 03:26:44]

On 2014/04/01 00:10:44, ddorwin wrote:
> I don't think this really addresses BUG=357665, at least not directly - this
is
> just some of the work towards being able to address that.

hmmm... yes this was to fix the wav and ogg  tests that were failing  for
android.

[amogh.bihani, 2014-04-02 02:39:03]

acolwell@
scherkus@
PTAL

[scherkus (not reviewing), 2014-04-02 16:26:45]

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util.cc
File net/base/mime_util.cc (right):

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util.cc#newcod...
net/base/mime_util.cc:439: { "video/ogg", "opus, theora, vorbis" },
nit: the other lists doesn't appear to not have spaces after the commas --
remove?

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util_unittest.cc
File net/base/mime_util_unittest.cc (right):

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util_unittest....
net/base/mime_util_unittest.cc:12: #define EXPECT_TRUE_NON_ANDROID EXPECT_FALSE
considering this is only used in one place I don't think it's worth defining a
macro here -- put the #if defined(OS_ANDROID) check in the test itself

[amogh.bihani, 2014-04-03 03:57:13]

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util.cc
File net/base/mime_util.cc (right):

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util.cc#newcod...
net/base/mime_util.cc:439: { "video/ogg", "opus, theora, vorbis" },
On 2014/04/02 16:26:46, scherkus wrote:
> nit: the other lists doesn't appear to not have spaces after the commas --
> remove?

Done.

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util_unittest.cc
File net/base/mime_util_unittest.cc (right):

https://codereview.chromium.org/218303002/diff/1/net/base/mime_util_unittest....
net/base/mime_util_unittest.cc:12: #define EXPECT_TRUE_NON_ANDROID EXPECT_FALSE
On 2014/04/02 16:26:46, scherkus wrote:
> considering this is only used in one place I don't think it's worth defining a
> macro here -- put the #if defined(OS_ANDROID) check in the test itself

Done.

[scherkus (not reviewing), 2014-04-03 19:55:06]

lgtm

[amogh.bihani, 2014-04-04 04:00:53]

The CQ bit was checked by amogh.bihani@samsung.com

[commit-bot: I haz the power, 2014-04-04 04:01:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/amogh.bihani@samsung.com/218303002/20001

[amogh.bihani, 2014-04-04 04:32:20]

@Ryan
OWNER's review please.

[amogh.bihani, 2014-04-04 04:32:24]

The CQ bit was unchecked by amogh.bihani@samsung.com

[Ryan Sleevi, 2014-04-07 16:47:46]

LGTM rubberstamp, with nit. Deferring to scherkus here on media/ correctness

https://codereview.chromium.org/218303002/diff/20001/net/base/mime_util_unitt...
File net/base/mime_util_unittest.cc (right):

https://codereview.chromium.org/218303002/diff/20001/net/base/mime_util_unitt...
net/base/mime_util_unittest.cc:112: // TODO(amogh.bihani): Fix these test when
bug 53193 is fixed.
nit: Rather than "fix these", which is somewhat ambiguous with respect to the
context of the bug, perhaps you can indicate in a small comment what's wrong

eg:
// TODO(amogh.bihani): These formats should do X, but do Y because of
// http://crbug.com/53193

[amogh.bihani, 2014-04-08 03:29:14]

Thanks for the review :)

https://codereview.chromium.org/218303002/diff/20001/net/base/mime_util_unitt...
File net/base/mime_util_unittest.cc (right):

https://codereview.chromium.org/218303002/diff/20001/net/base/mime_util_unitt...
net/base/mime_util_unittest.cc:112: // TODO(amogh.bihani): Fix these test when
bug 53193 is fixed.
On 2014/04/07 16:47:47, Ryan Sleevi wrote:
> nit: Rather than "fix these", which is somewhat ambiguous with respect to the
> context of the bug, perhaps you can indicate in a small comment what's wrong
> 
> eg:
> // TODO(amogh.bihani): These formats should do X, but do Y because of
> // http://crbug.com/53193

Done.

[amogh.bihani, 2014-04-08 03:29:21]

The CQ bit was checked by amogh.bihani@samsung.com

[commit-bot: I haz the power, 2014-04-08 03:29:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/amogh.bihani@samsung.com/218303002/40001

[amogh.bihani, 2014-04-08 06:06:35]

The CQ bit was checked by amogh.bihani@samsung.com

[commit-bot: I haz the power, 2014-04-08 06:06:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/amogh.bihani@samsung.com/218303002/60001

[amogh.bihani, 2014-04-08 07:27:18]

The CQ bit was unchecked by amogh.bihani@samsung.com

[amogh.bihani, 2014-04-08 09:07:41]

The test was failing on Android for video/ogg.
This is because the IsStrictMediaMimeType directly checks in the
stict_format_map which has video/ogg entry in it.
However for a browser this will never happen as first the
renderer_webkitplatformsupport_impl checks whether the
mime type is supported or not[1] and then only check if it is a strict supported
mime type or not[2].
Hence I am making the video/ogg unit test common for all platforms.

---------
[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...

[amogh.bihani, 2014-04-08 09:24:57]

The CQ bit was checked by amogh.bihani@samsung.com

[commit-bot: I haz the power, 2014-04-08 09:25:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/amogh.bihani@samsung.com/218303002/70005

[commit-bot: I haz the power, 2014-04-08 11:44:08]

Change committed as 262374