Add UMA logging to track bad MIME types passed to HTMLMediaElement

Add UMA logging to track bad MIME types passed to HTMLMediaElement

It would be nice to change HTMLMediaElement to use ParsedContentType when
checking MIME types passed to it. However, it currently uses ContentType
which is not properly spec compliant. Adding a UMA to determine how much
real world usage actually provides MIME types that don't parse according to
RFC 2045.

BUG=674329
TEST=media layout tests pass

Review-Url: https://codereview.chromium.org/2730853002
Cr-Commit-Position: refs/heads/master@{#462782}
Committed: https://chromium.googlesource.com/chromium/src/+/9049bebd837e0af0c1e9cba1669779c14978ccf4

[jrummell, 2017-03-03 04:13:42]

jrummell@chromium.org changed reviewers:
+ dalecurtis@chromium.org, ddorwin@chromium.org

[jrummell, 2017-03-03 04:13:43]

WDYT? This simply adds a UMA to report if ParsedContentType would fail parsing
the MIME type but passes using ContentType today. Hopefully parsing will never
fail (on IsSupported or MayBeSupported), so we can switch without concern.

[DaleCurtis, 2017-03-03 17:57:50]

idea sgtm, not familiar with how the histograms work in Blink so defer to WebKit
reviewer.

[jrummell, 2017-03-03 21:52:48]

Just noticed an issue, so noting it here.

https://codereview.chromium.org/2730853002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2730853002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:312:
MIMETypeRegistry::IsSupported;
This is wrong (should be != IsNotSupported).

[jrummell, 2017-03-14 21:40:16]

jrummell@chromium.org changed reviewers:
+ mlamouri@chromium.org

[jrummell, 2017-03-14 21:40:17]

+mlamouri@ for OWNERS review.

[jrummell, 2017-03-15 00:13:20]

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

[commit-bot: I haz the power, 2017-03-15 00:14:08]

Dry run: 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, 2017-03-15 00:33:59]

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

[commit-bot: I haz the power, 2017-03-15 00:33:59]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mlamouri (slow - plz ping), 2017-03-16 12:00:35]

lgtm

https://codereview.chromium.org/2730853002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2730853002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:284: bool
canLoadURL(const KURL& url, const String& contentType) {
nit: maybe you should rename the argument to `contentTypeStr` so you can keep
`contentType` and the reader can quickly understand the difference between both.

[jrummell, 2017-03-17 17:40:35]

jrummell@chromium.org changed reviewers:
+ jwd@chromium.org

[jrummell, 2017-03-17 17:40:35]

+jwd@ for OWNERS review of histograms.xml

https://codereview.chromium.org/2730853002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/2730853002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:284: bool
canLoadURL(const KURL& url, const String& contentType) {
On 2017/03/16 12:00:35, mlamouri wrote:
> nit: maybe you should rename the argument to `contentTypeStr` so you can keep
> `contentType` and the reader can quickly understand the difference between
both.

Done.

[jrummell, 2017-03-17 17:40:47]

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

[commit-bot: I haz the power, 2017-03-17 17:41:18]

Dry run: 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, 2017-03-17 19:22:49]

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

[commit-bot: I haz the power, 2017-03-17 19:22:51]

Dry run: This issue passed the CQ dry run.

[jwd, 2017-03-21 14:08:48]

lgtm

[jrummell, 2017-04-07 03:10:40]

The CQ bit was checked by jrummell@chromium.org

[jrummell, 2017-04-07 03:10:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org, jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2730853002/#ps60001
(title: "rebase only")

[commit-bot: I haz the power, 2017-04-07 03:10:59]

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, 2017-04-07 05:53:26]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491534640167410,
"parent_rev": "081904c5bdd3417b6bfda8cf35b3682b2fdde946", "commit_rev":
"9049bebd837e0af0c1e9cba1669779c14978ccf4"}

[commit-bot: I haz the power, 2017-04-07 05:54:21]

Description was changed from

==========
Add UMA logging to track bad MIME types passed to HTMLMediaElement

It would be nice to change HTMLMediaElement to use ParsedContentType when
checking MIME types passed to it. However, it currently uses ContentType
which is not properly spec compliant. Adding a UMA to determine how much
real world usage actually provides MIME types that don't parse according to
RFC 2045.

BUG=674329
TEST=media layout tests pass
==========

to

==========
Add UMA logging to track bad MIME types passed to HTMLMediaElement

It would be nice to change HTMLMediaElement to use ParsedContentType when
checking MIME types passed to it. However, it currently uses ContentType
which is not properly spec compliant. Adding a UMA to determine how much
real world usage actually provides MIME types that don't parse according to
RFC 2045.

BUG=674329
TEST=media layout tests pass

Review-Url: https://codereview.chromium.org/2730853002
Cr-Commit-Position: refs/heads/master@{#462782}
Committed:
https://chromium.googlesource.com/chromium/src/+/9049bebd837e0af0c1e9cba16697...
==========

[commit-bot: I haz the power, 2017-04-07 05:54:22]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/9049bebd837e0af0c1e9cba16697...