Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(231)

Issue 1414533002: Usage counter for add/removeEventListener third argument. (Closed)

Created:
5 years, 2 months ago by dtapuska
Modified:
5 years, 2 months ago
Reviewers:
esprehn, Ilya Sherman
CC:
asvitkine+watch_chromium.org, blink-reviews, blink-reviews-bindings_chromium.org, chromium-reviews, Rick Byers
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Usage counter for add/removeEventListener third argument. Check to see if the third argument on the add/removeEventListener is a boolean; if it isn't log a UMA metric. BUG=543685 Committed: https://crrev.com/bd5f09eed76868c8aa00a9ab4fa66a9c72ccc6dc Cr-Commit-Position: refs/heads/master@{#354908}

Patch Set 1 #

Patch Set 2 : Adjust uma metric to check just for object #

Patch Set 3 : Fix histograms.xml #

Patch Set 4 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -0 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/custom/V8EventTargetCustom.cpp View 1 2 chunks +8 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/UseCounter.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 23 (7 generated)
dtapuska
5 years, 2 months ago (2015-10-16 20:35:02 UTC) #2
Ilya Sherman
histograms.xml lgtm
5 years, 2 months ago (2015-10-16 21:47:29 UTC) #3
esprehn
I think you actually want to check how often the third parameter is an object, ...
5 years, 2 months ago (2015-10-16 21:56:55 UTC) #4
philipj_slow
On 2015/10/16 21:56:55, esprehn wrote: > I think you actually want to check how often ...
5 years, 2 months ago (2015-10-19 13:58:50 UTC) #5
dtapuska
On 2015/10/16 21:56:55, esprehn wrote: > I think you actually want to check how often ...
5 years, 2 months ago (2015-10-19 14:52:53 UTC) #6
dtapuska
5 years, 2 months ago (2015-10-19 14:52:58 UTC) #7
dtapuska
On 2015/10/19 14:52:58, dtapuska wrote: Updated the code to include null and undefined; as the ...
5 years, 2 months ago (2015-10-19 17:44:58 UTC) #8
dtapuska
On 2015/10/19 17:44:58, dtapuska wrote: > On 2015/10/19 14:52:58, dtapuska wrote: > > Updated the ...
5 years, 2 months ago (2015-10-19 17:49:47 UTC) #10
esprehn
Lgtm, we don't usually add lines to that XML file... we just check the UMA ...
5 years, 2 months ago (2015-10-19 18:01:08 UTC) #11
dtapuska
On 2015/10/19 18:01:08, esprehn wrote: > Lgtm, we don't usually add lines to that XML ...
5 years, 2 months ago (2015-10-19 18:03:37 UTC) #12
Alexei Svitkine (slow)
On 2015/10/19 18:01:08, esprehn wrote: > Lgtm, we don't usually add lines to that XML ...
5 years, 2 months ago (2015-10-19 18:03:54 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1414533002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1414533002/40001
5 years, 2 months ago (2015-10-19 18:09:16 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/122534)
5 years, 2 months ago (2015-10-19 20:28:59 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1414533002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1414533002/80001
5 years, 2 months ago (2015-10-19 20:43:19 UTC) #21
commit-bot: I haz the power
Committed patchset #4 (id:80001)
5 years, 2 months ago (2015-10-19 23:28:01 UTC) #22
commit-bot: I haz the power
5 years, 2 months ago (2015-10-19 23:28:50 UTC) #23
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/bd5f09eed76868c8aa00a9ab4fa66a9c72ccc6dc
Cr-Commit-Position: refs/heads/master@{#354908}

Powered by Google App Engine
This is Rietveld 408576698