Clean up Media_* actions by marking them obsolete and not_user_trigerred.

Clean up Media_* actions by marking them obsolete and not_user_trigerred.

As recommended in the actions.xml file, non user triggered actions
should be marked with this attribute so they can be filtered out in
tools that only show user actions.

Furthermore, the Media_* actions are mostly superseded by Media.Controls.*
so they have been marked as obsolete. Histograms can be added on a need
basis if we need to register occurrences of some events.

BUG=627840
R=isherman@chromium.org, liberato@chromium.org

Committed: https://crrev.com/4224693a21ceefcc4ada486d3616e0555f99b025
Cr-Commit-Position: refs/heads/master@{#407939}

[mlamouri (slow - plz ping), 2016-07-18 16:29:42]

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

[mlamouri (slow - plz ping), 2016-07-18 16:29:58]

PTAL

[commit-bot: I haz the power, 2016-07-18 16:30: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, 2016-07-18 17:08:30]

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

[commit-bot: I haz the power, 2016-07-18 17:08:31]

Dry run: This issue passed the CQ dry run.

[liberato (no reviews please), 2016-07-18 17:50:57]

On 2016/07/18 16:29:58, Mounir Lamouri wrote:
> PTAL

lgtm

[Ilya Sherman, 2016-07-18 23:00:12]

https://codereview.chromium.org/2159813002/diff/1/tools/metrics/actions/actio...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2159813002/diff/1/tools/metrics/actions/actio...
tools/metrics/actions/actions.xml:8567: <description>A call to play() with a
user gesture succeeded.</description>
Hmm, this seems like it is a user-triggered gesture -- it requires a user
gesture.  Are these metrics perhaps simply obsolete, and replaced by other
metrics?  If so, please remove the code that logs the obsolete metrics.  If not,
could you please clarify what I might be misunderstanding?

[mlamouri (slow - plz ping), 2016-07-19 10:23:50]

PTAL

https://codereview.chromium.org/2159813002/diff/1/tools/metrics/actions/actio...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2159813002/diff/1/tools/metrics/actions/actio...
tools/metrics/actions/actions.xml:8567: <description>A call to play() with a
user gesture succeeded.</description>
On 2016/07/18 at 23:00:12, Ilya Sherman wrote:
> Hmm, this seems like it is a user-triggered gesture -- it requires a user
gesture.  Are these metrics perhaps simply obsolete, and replaced by other
metrics?  If so, please remove the code that logs the obsolete metrics.  If not,
could you please clarify what I might be misunderstanding?

You are right, this is coming from a gesture. It was an oversight because in the
cases I'm focusing on, the request comes from the controls. Unfortunately, that
means we will get Play from the controls they Play_WithGesture from the Element
:(

I've also fixed two other oversights: RequestRemotePlayback and
RequestRemotePlayback_Control can only be called from a path requiring a user
gesture.

Note that in all cases, even though these actions must come from a user gesture,
they are arguably not user triggered because they are trigerred from script and
the user might try to do something entirely unrelated.

[mlamouri (slow - plz ping), 2016-07-19 10:24:16]

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

[commit-bot: I haz the power, 2016-07-19 10:24:32]

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, 2016-07-19 11:20:17]

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

[commit-bot: I haz the power, 2016-07-19 11:20:18]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2016-07-20 02:22:19]

https://codereview.chromium.org/2159813002/diff/1/tools/metrics/actions/actio...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2159813002/diff/1/tools/metrics/actions/actio...
tools/metrics/actions/actions.xml:8567: <description>A call to play() with a
user gesture succeeded.</description>
On 2016/07/19 10:23:50, Mounir Lamouri wrote:
> On 2016/07/18 at 23:00:12, Ilya Sherman wrote:
> > Hmm, this seems like it is a user-triggered gesture -- it requires a user
> gesture.  Are these metrics perhaps simply obsolete, and replaced by other
> metrics?  If so, please remove the code that logs the obsolete metrics.  If
not,
> could you please clarify what I might be misunderstanding?
> 
> You are right, this is coming from a gesture. It was an oversight because in
the
> cases I'm focusing on, the request comes from the controls. Unfortunately,
that
> means we will get Play from the controls they Play_WithGesture from the
Element
> :(
> 
> I've also fixed two other oversights: RequestRemotePlayback and
> RequestRemotePlayback_Control can only be called from a path requiring a user
> gesture.
> 
> Note that in all cases, even though these actions must come from a user
gesture,
> they are arguably not user triggered because they are trigerred from script
and
> the user might try to do something entirely unrelated.

Sorry, I'm still not quite clear: Are all of these actions that you're marking
logged only when the action is triggered from a script?  Or, are they logged
both when the user interacts with the controls, and also when JavaScript invokes
these actions?  It would be great if you could update the descriptions for these
metrics to clarify, and indicate what metric to compare them against (if there
is one).

Also: Are these not_user_triggered actions still valuable in some context, or
would it be worthwhile to simply stop logging them?  If they are getting in the
way of your analysis because they are essentially duplicates of other metrics,
it definitely raises the question of what value we're getting from having two
parallel sets of user action metrics for media playback.

[mlamouri (slow - plz ping), 2016-07-20 13:34:48]

On 2016/07/20 at 02:22:19, isherman wrote:
> Sorry, I'm still not quite clear: Are all of these actions that you're marking
logged only when the action is triggered from a script?  Or, are they logged
both when the user interacts with the controls, and also when JavaScript invokes
these actions?  It would be great if you could update the descriptions for these
metrics to clarify, and indicate what metric to compare them against (if there
is one).

These actions are invoked from both script and controls.

> Also: Are these not_user_triggered actions still valuable in some context, or
would it be worthwhile to simply stop logging them?  If they are getting in the
way of your analysis because they are essentially duplicates of other metrics,
it definitely raises the question of what value we're getting from having two
parallel sets of user action metrics for media playback.

I did not add these actions. I am analysing controls usage and what users do
after doing action X. The issue here is that pressing Mute for example in the
controls UI will call the Mute_{On,Off} action. I don't feel comfortable
removing actions I didn't add myself.

@liberato, what was the original need for these actions? It was related to some
autoplay experiments, wasn't it? Do you think we should keep them around?

[Ilya Sherman, 2016-07-21 00:47:07]

On 2016/07/20 13:34:48, Mounir Lamouri wrote:
> On 2016/07/20 at 02:22:19, isherman wrote:
> > Sorry, I'm still not quite clear: Are all of these actions that you're
marking
> logged only when the action is triggered from a script?  Or, are they logged
> both when the user interacts with the controls, and also when JavaScript
invokes
> these actions?  It would be great if you could update the descriptions for
these
> metrics to clarify, and indicate what metric to compare them against (if there
> is one).
> 
> These actions are invoked from both script and controls.
> 
> > Also: Are these not_user_triggered actions still valuable in some context,
or
> would it be worthwhile to simply stop logging them?  If they are getting in
the
> way of your analysis because they are essentially duplicates of other metrics,
> it definitely raises the question of what value we're getting from having two
> parallel sets of user action metrics for media playback.
> 
> I did not add these actions. I am analysing controls usage and what users do
> after doing action X. The issue here is that pressing Mute for example in the
> controls UI will call the Mute_{On,Off} action. I don't feel comfortable
> removing actions I didn't add myself.
> 
> @liberato, what was the original need for these actions? It was related to
some
> autoplay experiments, wasn't it? Do you think we should keep them around?

Thanks.  I'll wait for @liberato's input prior to continuing the review.

[liberato (no reviews please), 2016-07-21 00:54:48]

On 2016/07/20 13:34:48, Mounir Lamouri wrote:
> On 2016/07/20 at 02:22:19, isherman wrote:
> > Sorry, I'm still not quite clear: Are all of these actions that you're
marking
> logged only when the action is triggered from a script?  Or, are they logged
> both when the user interacts with the controls, and also when JavaScript
invokes
> these actions?  It would be great if you could update the descriptions for
these
> metrics to clarify, and indicate what metric to compare them against (if there
> is one).
> 
> These actions are invoked from both script and controls.
> 
> > Also: Are these not_user_triggered actions still valuable in some context,
or
> would it be worthwhile to simply stop logging them?  If they are getting in
the
> way of your analysis because they are essentially duplicates of other metrics,
> it definitely raises the question of what value we're getting from having two
> parallel sets of user action metrics for media playback.
> 
> I did not add these actions. I am analysing controls usage and what users do
> after doing action X. The issue here is that pressing Mute for example in the
> controls UI will call the Mute_{On,Off} action. I don't feel comfortable
> removing actions I didn't add myself.
> 
> @liberato, what was the original need for these actions? It was related to
some
> autoplay experiments, wasn't it? Do you think we should keep them around?

sorry about the delay -- i did entirely quit watching this thread.

you're correct, i added them as part of autoplay.  at the time, we were
struggling to understand some of the histograms we collected, and I thought that
seeing some of the actions on a timeline might help to shed some light on what
people were doing.

plus, user actions weren't plumbed to blink, so that seemed worthwhile in
itself.

but now, i have little use for them.  please remove or change whatever you like.

thanks
-fl

[mlamouri (slow - plz ping), 2016-07-25 16:29:09]

Description was changed from

==========
Mark Media_* actions as not_user_trigerred in faviour of the Media.Controls
ones.

As recommended in the actions.xml file, non user triggered actions
should be marked with this attribute so they can be filtered out in
tools that only show user actions.

BUG=627840
R=isherman@chromium.org, liberato@chromium.org
==========

to

==========
Mark Media_* actions as not_user_trigerred in faviour of the Media.Controls
ones.

As recommended in the actions.xml file, non user triggered actions
should be marked with this attribute so they can be filtered out in
tools that only show user actions.

Also marking a few Media_* actions as obsolete.

BUG=627840
R=isherman@chromium.org, liberato@chromium.org
==========

[mlamouri (slow - plz ping), 2016-07-25 16:29:18]

isherman@, PTAL

[Ilya Sherman, 2016-07-25 18:41:58]

Thanks!  Could you please also update the CL description, since this CL is no
longer primarily about marking some actions as not_user_triggered?

https://codereview.chromium.org/2159813002/diff/60001/tools/metrics/actions/a...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2159813002/diff/60001/tools/metrics/actions/a...
tools/metrics/actions/actions.xml:8560: <action name="Media_Paused"
not_user_triggered="true">
nit: Seems like the not_user_triggered flag shouldn't be needed if the action is
obsolete.  (Ditto for the others.)

[mlamouri (slow - plz ping), 2016-07-26 17:29:44]

Description was changed from

==========
Mark Media_* actions as not_user_trigerred in faviour of the Media.Controls
ones.

As recommended in the actions.xml file, non user triggered actions
should be marked with this attribute so they can be filtered out in
tools that only show user actions.

Also marking a few Media_* actions as obsolete.

BUG=627840
R=isherman@chromium.org, liberato@chromium.org
==========

to

==========
Clean up Media_* actions by marking them obsolete and not_user_trigerred.

As recommended in the actions.xml file, non user triggered actions
should be marked with this attribute so they can be filtered out in
tools that only show user actions.

Furthermore, the Media_* actions are mostly superseded by Media.Controls.*
so they have been marked as obsolete. Histograms can be added on a need
basis if we need to register occurrences of some events.

BUG=627840
R=isherman@chromium.org, liberato@chromium.org
==========

[mlamouri (slow - plz ping), 2016-07-26 17:29:52]

PTAL

https://codereview.chromium.org/2159813002/diff/60001/tools/metrics/actions/a...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2159813002/diff/60001/tools/metrics/actions/a...
tools/metrics/actions/actions.xml:8560: <action name="Media_Paused"
not_user_triggered="true">
On 2016/07/25 at 18:41:58, Ilya Sherman wrote:
> nit: Seems like the not_user_triggered flag shouldn't be needed if the action
is obsolete.  (Ditto for the others.)

Maybe it's better to mark them properly even if it is retroactive? I want to
make sure the tool I'm using will not show these actions even if they were
registered.

[Ilya Sherman, 2016-07-26 19:17:24]

https://codereview.chromium.org/2159813002/diff/60001/tools/metrics/actions/a...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2159813002/diff/60001/tools/metrics/actions/a...
tools/metrics/actions/actions.xml:8560: <action name="Media_Paused"
not_user_triggered="true">
On 2016/07/26 17:29:52, Mounir Lamouri wrote:
> On 2016/07/25 at 18:41:58, Ilya Sherman wrote:
> > nit: Seems like the not_user_triggered flag shouldn't be needed if the
action
> is obsolete.  (Ditto for the others.)
> 
> Maybe it's better to mark them properly even if it is retroactive? I want to
> make sure the tool I'm using will not show these actions even if they were
> registered.

It's not entirely obvious to me that it's "correct" to mark these as
non-user-triggered (since I'd imagine they typically are)... but I also don't
feel very strongly about this, so LGTM.

[mlamouri (slow - plz ping), 2016-07-26 21:23:20]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2016-07-26 21:23:21]

Thanks :)

[mlamouri (slow - plz ping), 2016-07-26 21:23:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
liberato@chromium.org
Link to the patchset: https://codereview.chromium.org/2159813002/#ps60001
(title: "mark obsolete")

[commit-bot: I haz the power, 2016-07-26 21:23:47]

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-07-26 22:33:48]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-26 22:35:24]

Description was changed from

==========
Clean up Media_* actions by marking them obsolete and not_user_trigerred.

As recommended in the actions.xml file, non user triggered actions
should be marked with this attribute so they can be filtered out in
tools that only show user actions.

Furthermore, the Media_* actions are mostly superseded by Media.Controls.*
so they have been marked as obsolete. Histograms can be added on a need
basis if we need to register occurrences of some events.

BUG=627840
R=isherman@chromium.org, liberato@chromium.org
==========

to

==========
Clean up Media_* actions by marking them obsolete and not_user_trigerred.

As recommended in the actions.xml file, non user triggered actions
should be marked with this attribute so they can be filtered out in
tools that only show user actions.

Furthermore, the Media_* actions are mostly superseded by Media.Controls.*
so they have been marked as obsolete. Histograms can be added on a need
basis if we need to register occurrences of some events.

BUG=627840
R=isherman@chromium.org, liberato@chromium.org

Committed: https://crrev.com/4224693a21ceefcc4ada486d3616e0555f99b025
Cr-Commit-Position: refs/heads/master@{#407939}
==========

[commit-bot: I haz the power, 2016-07-26 22:35:25]

Patchset 4 (id:??) landed as
https://crrev.com/4224693a21ceefcc4ada486d3616e0555f99b025
Cr-Commit-Position: refs/heads/master@{#407939}