Count usage of MediaStream active and inactive events

Count usage of MediaStream active and inactive events

BUG=621913
Committed: https://crrev.com/85b0410d2fd19498da09684ca33c47d06aa44ff7
Cr-Commit-Position: refs/heads/master@{#401345}

[Guido Urdaneta, 2016-06-21 13:57:27]

Description was changed from

==========
Count usage of MediaStream active and inactive events

BUG=621913
==========

to

==========
Count usage of MediaStream active and inactive events

BUG=621913
==========

[Guido Urdaneta, 2016-06-21 13:57:27]

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

[Guido Urdaneta, 2016-06-21 13:58:53]

guidou@chromium.org changed reviewers:
+ foolip@chromium.org

[Guido Urdaneta, 2016-06-21 13:58:53]

holte@chromium.org: please review changes in histogram.xml
foolip@chromium.org: please review changes in WebKit

[Steven Holte, 2016-06-21 17:55:27]

lgtm

[foolip, 2016-06-22 12:17:05]

https://codereview.chromium.org/2081193002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/mediastream/MediaStream.idl (right):

https://codereview.chromium.org/2081193002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/mediastream/MediaStream.idl:48:
[MeasureAs=MediaStreamOnActive] attribute EventHandler onactive;
I think you shouldn't actually need to measure this. If one assigns an event
listener, doesn't it reach addEventListenerInternal anyway? If it doesn't, can
you count where the code paths meet?

The potential problem here is that the counter will be triggered by merely
reading the attributes, which could happen by enumerating attributes. This
completely poisons use counters for attributes on Window, Document and Event,
and it's hard to guess if it might play in here too.

[Guido Urdaneta, 2016-06-22 13:50:37]

On 2016/06/22 12:17:05, Philip Jägenstedt wrote:
>
https://codereview.chromium.org/2081193002/diff/1/third_party/WebKit/Source/m...
> File third_party/WebKit/Source/modules/mediastream/MediaStream.idl (right):
> 
>
https://codereview.chromium.org/2081193002/diff/1/third_party/WebKit/Source/m...
> third_party/WebKit/Source/modules/mediastream/MediaStream.idl:48:
> [MeasureAs=MediaStreamOnActive] attribute EventHandler onactive;
> I think you shouldn't actually need to measure this. If one assigns an event
> listener, doesn't it reach addEventListenerInternal anyway? If it doesn't, can
> you count where the code paths meet?
> 
> The potential problem here is that the counter will be triggered by merely
> reading the attributes, which could happen by enumerating attributes. This
> completely poisons use counters for attributes on Window, Document and Event,
> and it's hard to guess if it might play in here too.

Removed the MeasureAs annotation. 
I confirmed that adding an event listener via the on* attributes hits
addEventListenerInternal.

[foolip, 2016-06-22 14:56:32]

lgtm

[Guido Urdaneta, 2016-06-22 15:59:25]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2016-06-22 15:59:25]

The patchset sent to the CQ was uploaded after l-g-t-m from holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2081193002/#ps20001
(title: "foolip comment")

[commit-bot: I haz the power, 2016-06-22 15:59:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2081193002/20001

[commit-bot: I haz the power, 2016-06-22 16:02:33]

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

[commit-bot: I haz the power, 2016-06-22 16:02:34]

Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Guido Urdaneta, 2016-06-22 16:11:50]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2016-06-22 16:11:50]

The patchset sent to the CQ was uploaded after l-g-t-m from holte@chromium.org,
foolip@chromium.org
Link to the patchset: https://codereview.chromium.org/2081193002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-22 16:12:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2081193002/40001

[commit-bot: I haz the power, 2016-06-22 18:11:22]

Description was changed from

==========
Count usage of MediaStream active and inactive events

BUG=621913
==========

to

==========
Count usage of MediaStream active and inactive events

BUG=621913
==========

[commit-bot: I haz the power, 2016-06-22 18:11:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-22 18:17:01]

Description was changed from

==========
Count usage of MediaStream active and inactive events

BUG=621913
==========

to

==========
Count usage of MediaStream active and inactive events

BUG=621913

Committed: https://crrev.com/85b0410d2fd19498da09684ca33c47d06aa44ff7
Cr-Commit-Position: refs/heads/master@{#401345}
==========

[commit-bot: I haz the power, 2016-06-22 18:17:02]

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