Look into full URL spec to see whether it is MP4

Look into full URL spec to see whether it is MP4

On certain Android devices MediaCodec is blacklisted.
To support H264 we fall back to MediaCodec, and we
decide whether this is H264 by looking at URL.

This CL improves that decision making: before we only
checked whether the path suffisc was ".mp4", now we look
for "video/mp4" in the whole URL spec as well.

In addition this CL makes canPlayType() return
"probably" for H264 codec.

BUG=642988
Committed: https://crrev.com/8cda8cd5f2766bb31636ebba30fca01d64d97f46
Cr-Commit-Position: refs/heads/master@{#419272}

[Tima Vaisburd, 2016-09-15 02:37:01]

timav@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[Tima Vaisburd, 2016-09-15 02:37:01]

PTAL. Feel free to update the description.

[DaleCurtis, 2016-09-15 16:26:57]

I'd just search mp4 without the video/ and guard this by !has platform decoder.
Also mime util unit test will need updating

[Tima Vaisburd, 2016-09-15 18:45:38]

On 2016/09/15 16:26:57, DaleCurtis wrote:
> I'd just search mp4 without the video/ and guard this by !has platform
decoder.
> Also mime util unit test will need updating

I see stuff that looks like random character string, e.g "cpn=HbVri1WMPk1bwvgY",
I believe it can accidentally contain "mp4". Shall we err on false positive
side?

[DaleCurtis, 2016-09-15 19:10:48]

It's fine either way, soon we'll be able to handle this without any URL checks,
so we just need something to unblock M54 users seeing this issue. The key thing
is to gate the checks on !has_platform_decoders, which should be quite rare in
practice.

[Tima Vaisburd, 2016-09-15 22:27:24]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-15 22:27:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[DaleCurtis, 2016-09-16 00:03:43]

lgtm

[commit-bot: I haz the power, 2016-09-16 00:07:27]

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

[commit-bot: I haz the power, 2016-09-16 00:07:27]

Dry run: This issue passed the CQ dry run.

[Tima Vaisburd, 2016-09-16 00:55:39]

The CQ bit was checked by timav@chromium.org

[commit-bot: I haz the power, 2016-09-16 00:56:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 01:07:41]

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

[commit-bot: I haz the power, 2016-09-16 01:07:42]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Tima Vaisburd, 2016-09-16 01:37:11]

timav@chromium.org changed reviewers:
+ piman@chromium.org

[Tima Vaisburd, 2016-09-16 01:37:11]

piman@: Asking for OWNERS review.

[piman, 2016-09-16 01:45:00]

lgtm

https://codereview.chromium.org/2338213005/diff/20001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2338213005/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:800: // Assume "mp4" means H264.
nit: please add a link to the bug. This logic is very suspicious at first
glance, the bug explains a bit more - it sounds like this is a temporary fix?

[Tima Vaisburd, 2016-09-16 19:04:22]

https://codereview.chromium.org/2338213005/diff/20001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2338213005/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:800: // Assume "mp4" means H264.
On 2016/09/16 01:45:00, piman OOO back 2016-09-12 wrote:
> nit: please add a link to the bug. This logic is very suspicious at first
> glance, the bug explains a bit more - it sounds like this is a temporary fix?

Done.

[Tima Vaisburd, 2016-09-16 19:04:37]

The CQ bit was checked by timav@chromium.org

[Tima Vaisburd, 2016-09-16 19:04:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2338213005/#ps40001
(title: "Added reference to the bug in the comments.")

[commit-bot: I haz the power, 2016-09-16 19:04:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 20:33:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-16 20:36:09]

Description was changed from

==========
Look into full URL spec to see whether it is MP4

On certain Android devices MediaCodec is blacklisted.
To support H264 we fall back to MediaCodec, and we
decide whether this is H264 by looking at URL.

This CL improves that decision making: before we only
checked whether the path suffisc was ".mp4", now we look
for "video/mp4" in the whole URL spec as well.

In addition this CL makes canPlayType() return
"probably" for H264 codec.

BUG=642988
==========

to

==========
Look into full URL spec to see whether it is MP4

On certain Android devices MediaCodec is blacklisted.
To support H264 we fall back to MediaCodec, and we
decide whether this is H264 by looking at URL.

This CL improves that decision making: before we only
checked whether the path suffisc was ".mp4", now we look
for "video/mp4" in the whole URL spec as well.

In addition this CL makes canPlayType() return
"probably" for H264 codec.

BUG=642988

Committed: https://crrev.com/8cda8cd5f2766bb31636ebba30fca01d64d97f46
Cr-Commit-Position: refs/heads/master@{#419272}
==========

[commit-bot: I haz the power, 2016-09-16 20:36:10]

Patchset 3 (id:??) landed as
https://crrev.com/8cda8cd5f2766bb31636ebba30fca01d64d97f46
Cr-Commit-Position: refs/heads/master@{#419272}