Record user interactions with MediaSession by action type

Record user interactions with MediaSession by action type

In this CL we add histograms for recording how many times the user interacts
with MediaSession sorted by action type.

BUG=708617
Review-Url: https://codereview.chromium.org/2798083002
Cr-Commit-Position: refs/heads/master@{#463292}
Committed: https://chromium.googlesource.com/chromium/src/+/93807620c531744cdd963c95afa7077c9a44e58b

[Zhiqiang Zhang (Slow), 2017-04-05 17:21:00]

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

[Zhiqiang Zhang (Slow), 2017-04-05 17:22:11]

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

[commit-bot: I haz the power, 2017-04-05 17:22:44]

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-04-05 18:03:37]

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

[commit-bot: I haz the power, 2017-04-05 18:03:38]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Zhiqiang Zhang (Slow), 2017-04-05 18:52:43]

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

[commit-bot: I haz the power, 2017-04-05 18:53:26]

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-04-05 19:43:19]

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

[commit-bot: I haz the power, 2017-04-05 19:43:20]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2017-04-07 11:15:56]

Thanks! Could you add unit tests for this? :)

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
File content/browser/media/session/media_session_impl.cc (right):

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
content/browser/media/session/media_session_impl.cc:68: default:
Add the case for COUNT here so all cases are taken into account and if another
is added, it will fail to compile.

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
content/browser/media/session/media_session_impl.cc:365:
MediaSessionUmaHelper::MediaSessionUserAction::PlayDefault);
::PauseDefault?

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
File content/browser/media/session/media_session_uma_helper.h (right):

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
content/browser/media/session/media_session_uma_helper.h:55: void
RecordMediaSessionUserAction(MediaSessionUserAction action) const;
Could that be static?

https://codereview.chromium.org/2798083002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2798083002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:27570: + 
<owner>zqzhang@chromium.org</owner>
Can you just put you and media-dev@chromium.org?

[Zhiqiang Zhang (Slow), 2017-04-07 15:12:44]

PTAL. Added tests and addressed mlamouri@'s comments

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
File content/browser/media/session/media_session_impl.cc (right):

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
content/browser/media/session/media_session_impl.cc:68: default:
On 2017/04/07 at 11:15:55, mlamouri wrote:
> Add the case for COUNT here so all cases are taken into account and if another
is added, it will fail to compile.

Actually, the mojo enum only have LAST (= SEEK_FORWARD).
I removed default now.

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
content/browser/media/session/media_session_impl.cc:365:
MediaSessionUmaHelper::MediaSessionUserAction::PlayDefault);
On 2017/04/07 at 11:15:55, mlamouri wrote:
> ::PauseDefault?

Done.

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
File content/browser/media/session/media_session_uma_helper.h (right):

https://codereview.chromium.org/2798083002/diff/1/content/browser/media/sessi...
content/browser/media/session/media_session_uma_helper.h:55: void
RecordMediaSessionUserAction(MediaSessionUserAction action) const;
On 2017/04/07 at 11:15:56, mlamouri wrote:
> Could that be static?

Done.

https://codereview.chromium.org/2798083002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2798083002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:27570: + 
<owner>zqzhang@chromium.org</owner>
On 2017/04/07 at 11:15:56, mlamouri wrote:
> Can you just put you and media-dev@chromium.org?

Done.

[Zhiqiang Zhang (Slow), 2017-04-07 15:12:50]

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

[commit-bot: I haz the power, 2017-04-07 15:13:03]

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

[Zhiqiang Zhang (Slow), 2017-04-07 15:13:50]

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

[Zhiqiang Zhang (Slow), 2017-04-07 15:13:51]

+holte: histograms.xml

[commit-bot: I haz the power, 2017-04-07 15:20:00]

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

[commit-bot: I haz the power, 2017-04-07 15:20:01]

Dry run: 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...)

[Zhiqiang Zhang (Slow), 2017-04-07 17:12:57]

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

[commit-bot: I haz the power, 2017-04-07 17:13:27]

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-04-07 18:10:57]

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

[commit-bot: I haz the power, 2017-04-07 18:10:57]

Dry run: This issue passed the CQ dry run.

[Steven Holte, 2017-04-07 21:14:47]

histograms lgtm

[mlamouri (slow - plz ping), 2017-04-10 12:37:18]

lgtm

https://codereview.chromium.org/2798083002/diff/40001/content/browser/media/s...
File content/browser/media/session/media_session_impl_uma_unittest.cc (right):

https://codereview.chromium.org/2798083002/diff/40001/content/browser/media/s...
content/browser/media/session/media_session_impl_uma_unittest.cc:40:
RenderFrameHost* GetRenderFrameHost() const override {
style: render_frame_host()

https://codereview.chromium.org/2798083002/diff/40001/content/browser/media/s...
content/browser/media/session/media_session_impl_uma_unittest.cc:78:
MediaSessionImpl* session() { return MediaSessionImpl::Get(contents()); }
style: GetSession()

[Zhiqiang Zhang (Slow), 2017-04-10 15:43:05]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2017-04-10 15:43:05]

https://codereview.chromium.org/2798083002/diff/40001/content/browser/media/s...
File content/browser/media/session/media_session_impl_uma_unittest.cc (right):

https://codereview.chromium.org/2798083002/diff/40001/content/browser/media/s...
content/browser/media/session/media_session_impl_uma_unittest.cc:40:
RenderFrameHost* GetRenderFrameHost() const override {
On 2017/04/10 at 12:37:18, mlamouri wrote:
> style: render_frame_host()

Done.

https://codereview.chromium.org/2798083002/diff/40001/content/browser/media/s...
content/browser/media/session/media_session_impl_uma_unittest.cc:78:
MediaSessionImpl* session() { return MediaSessionImpl::Get(contents()); }
On 2017/04/10 at 12:37:18, mlamouri wrote:
> style: GetSession()

Done.

[Zhiqiang Zhang (Slow), 2017-04-10 15:43:06]

The patchset sent to the CQ was uploaded after l-g-t-m from holte@chromium.org,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2798083002/#ps60001
(title: "addressed style nits")

[commit-bot: I haz the power, 2017-04-10 15:43:36]

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-10 16:44:07]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491838985025920,
"parent_rev": "e5ed6412e37b70dd1e370b1ba1bd74ce2b14448e", "commit_rev":
"93807620c531744cdd963c95afa7077c9a44e58b"}

[commit-bot: I haz the power, 2017-04-10 16:45:07]

Description was changed from

==========
Record user interactions with MediaSession by action type

In this CL we add histograms for recording how many times the user interacts
with MediaSession sorted by action type.

BUG=708617
==========

to

==========
Record user interactions with MediaSession by action type

In this CL we add histograms for recording how many times the user interacts
with MediaSession sorted by action type.

BUG=708617

Review-Url: https://codereview.chromium.org/2798083002
Cr-Commit-Position: refs/heads/master@{#463292}
Committed:
https://chromium.googlesource.com/chromium/src/+/93807620c531744cdd963c95afa7...
==========

[commit-bot: I haz the power, 2017-04-10 16:45:08]

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