[Blink>Media] Adding rappor metrics to record autoplay muted by attribute usage

[Blink>Media] Adding rappor metrics to record autoplay muted by attribute usage

We need to analysis which sites are using autoplay muted by attribute.
This CL adds rappor metrics to record the eTLD+1 of the child frame and
top-level frame.

BUG=693889
Review-Url: https://codereview.chromium.org/2706593002
Cr-Commit-Position: refs/heads/master@{#452010}
Committed: https://chromium.googlesource.com/chromium/src/+/d0698b2368b6e48592d9c0231f63055fff323364

[Zhiqiang Zhang (Slow), 2017-02-18 14:33:22]

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

[mlamouri (slow - plz ping), 2017-02-20 11:21:30]

Would it make sense to record all type of autoplay instead of only autoplay from
the attribute? It sounds that it might be more interesting going forward and we
can specialize if we need to. WDYT?

[mlamouri (slow - plz ping), 2017-02-20 11:23:43]

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

https://codereview.chromium.org/2706593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp:86:
m_element->document().topDocument().url());
FWIW, I'm not sure if that's the right thing to do: it actually doesn't record
the URL of the top level frame but the top level local frame. Maybe not
recording when the top level is a remote frame would be better than recording a
wrong value?

[Zhiqiang Zhang (Slow), 2017-02-20 17:49:43]

PTAL

On 2017/02/20 at 11:21:30, mlamouri wrote:
> Would it make sense to record all type of autoplay instead of only autoplay
from the attribute? It sounds that it might be more interesting going forward
and we can specialize if we need to. WDYT?

I'm thinking it's better to keep them separate as we don't care play() that much
but it still takes a big proportion. Tracking them separately would help us
understand typical usecases of autoplay attributes and play() methods, so it
might help us make future UX decisions maybe?

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

https://codereview.chromium.org/2706593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/AutoplayUmaHelper.cpp:86:
m_element->document().topDocument().url());
On 2017/02/20 at 11:23:43, mlamouri wrote:
> FWIW, I'm not sure if that's the right thing to do: it actually doesn't record
the URL of the top level frame but the top level local frame. Maybe not
recording when the top level is a remote frame would be better than recording a
wrong value?

Maybe we just don't record the top-level frames at all? After OOPIFs is enabled,
the metric would barely make sense unless we can find some other way to get the
top-level frame URL.

[mlamouri (slow - plz ping), 2017-02-20 21:22:44]

Let's do:
- Media.Video.Autoplay.Muted.Attribute.Frame
- Media.Video.Autoplay.Muted.Method.Frame

Also, you forgot to check for the source when recorded the rappor metric :)

[Zhiqiang Zhang (Slow), 2017-02-20 21:32:16]

PTAL

On 2017/02/20 at 21:22:44, mlamouri wrote:
> Let's do:
> - Media.Video.Autoplay.Muted.Attribute.Frame
> - Media.Video.Autoplay.Muted.Method.Frame
> 
> Also, you forgot to check for the source when recorded the rappor metric :)

Done. Using "PlayMethod" instead of "Method" as we use it everywhere in the UMA.

[Zhiqiang Zhang (Slow), 2017-02-20 21:32:21]

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

[commit-bot: I haz the power, 2017-02-20 21:32:27]

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

[mlamouri (slow - plz ping), 2017-02-20 21:33:16]

lgtm

[commit-bot: I haz the power, 2017-02-20 22:55:08]

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

[commit-bot: I haz the power, 2017-02-20 22:55:09]

Dry run: This issue passed the CQ dry run.

[Zhiqiang Zhang (Slow), 2017-02-21 11:50:34]

zqzhang@chromium.org changed reviewers:
+ holte@chromium.org

[Zhiqiang Zhang (Slow), 2017-02-21 11:50:34]

+holte: rappor.xml

[Steven Holte, 2017-02-21 22:04:09]

lgtm

[Zhiqiang Zhang (Slow), 2017-02-22 10:51:37]

The CQ bit was checked by zqzhang@chromium.org

[commit-bot: I haz the power, 2017-02-22 10:52:12]

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-02-22 12:21:03]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487760697749770,
"parent_rev": "8ca96bbc8d62e8a4dac58894fbfece10c4e1fc0d", "commit_rev":
"d0698b2368b6e48592d9c0231f63055fff323364"}

[commit-bot: I haz the power, 2017-02-22 12:21:41]

Description was changed from

==========
[Blink>Media] Adding rappor metrics to record autoplay muted by attribute usage

We need to analysis which sites are using autoplay muted by attribute.
This CL adds rappor metrics to record the eTLD+1 of the child frame and
top-level frame.

BUG=693889
==========

to

==========
[Blink>Media] Adding rappor metrics to record autoplay muted by attribute usage

We need to analysis which sites are using autoplay muted by attribute.
This CL adds rappor metrics to record the eTLD+1 of the child frame and
top-level frame.

BUG=693889

Review-Url: https://codereview.chromium.org/2706593002
Cr-Commit-Position: refs/heads/master@{#452010}
Committed:
https://chromium.googlesource.com/chromium/src/+/d0698b2368b6e48592d9c0231f63...
==========

[commit-bot: I haz the power, 2017-02-22 12:21:42]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/d0698b2368b6e48592d9c0231f63...