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

Issue 2903583006: Collect metrics on running Accessibility services on Android (Closed)

Created:
3 years, 7 months ago by dmazzoni
Modified:
3 years, 6 months ago
CC:
aboxhall+watch_chromium.org, agrieve+watch_chromium.org, asvitkine+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dougt+watch_chromium.org, dtseng+watch_chromium.org, jam, je_julie, nektar+watch_chromium.org, yuzo+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Collect metrics on running Accessibility services on Android We'd like to optimize accessibility by only enabling the features actually needed by running accessibility services. In order to determine what would be the best signals of what running accessibility services actually need, add some code to collect metrics on running accessibility services that we can analyze later. Manually verified that TalkBack, Select-to-Speak, and LastPass all result in very different sets of metrics being collected. BUG=428494 Review-Url: https://codereview.chromium.org/2903583006 Cr-Commit-Position: refs/heads/master@{#477721} Committed: https://chromium.googlesource.com/chromium/src/+/f33b08933cff6791612633f2e4a191917bb9d567

Patch Set 1 #

Patch Set 2 : Rebase #

Total comments: 6

Patch Set 3 : aelias #

Total comments: 2

Patch Set 4 : Remove duplicate, check ContentViewCore #

Total comments: 8

Patch Set 5 : Address feedback on metrics #

Total comments: 6

Patch Set 6 : Last feedback from mpearson #

Patch Set 7 : Remove change to unrelated file #

Patch Set 8 : Try again, rebase and remove unrelated file. #

Patch Set 9 : Rebase #

Messages

Total messages: 71 (42 generated)
dmazzoni
3 years, 7 months ago (2017-05-24 17:48:27 UTC) #3
paulmiller
Don't we need metrics people to review new histograms?
3 years, 7 months ago (2017-05-24 18:17:05 UTC) #8
dmazzoni
Yes, I'll loop them in for review next, if this looks reasonable to you. Looks ...
3 years, 7 months ago (2017-05-24 18:22:44 UTC) #9
aelias_OOO_until_Jul13
https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc File content/browser/accessibility/browser_accessibility_manager_android.cc (right): https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc#newcode1230 content/browser/accessibility/browser_accessibility_manager_android.cc:1230: static bool stats_collected = false; "static" has a bad ...
3 years, 7 months ago (2017-05-24 20:19:54 UTC) #14
paulmiller
Maybe I just don't understand because I've never designed any histograms before, but how are ...
3 years, 7 months ago (2017-05-24 21:03:21 UTC) #15
Changwan Ryu
Do you care to collect service.getId() as well in one session? Usually the ID looks ...
3 years, 7 months ago (2017-05-24 21:29:48 UTC) #17
dmazzoni
https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc File content/browser/accessibility/browser_accessibility_manager_android.cc (right): https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc#newcode1230 content/browser/accessibility/browser_accessibility_manager_android.cc:1230: static bool stats_collected = false; On 2017/05/24 20:19:54, aelias ...
3 years, 7 months ago (2017-05-24 21:34:27 UTC) #18
aelias_OOO_until_Jul13
https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc File content/browser/accessibility/browser_accessibility_manager_android.cc (right): https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc#newcode1230 content/browser/accessibility/browser_accessibility_manager_android.cc:1230: static bool stats_collected = false; On 2017/05/24 at 21:34:27, ...
3 years, 7 months ago (2017-05-24 21:44:50 UTC) #19
dmazzoni
On 2017/05/24 21:03:21, paulmiller wrote: > Maybe I just don't understand because I've never designed ...
3 years, 7 months ago (2017-05-24 22:01:12 UTC) #20
dmazzoni
On 2017/05/24 21:29:48, Changwan Ryu wrote: > Do you care to collect service.getId() as well ...
3 years, 7 months ago (2017-05-24 22:03:51 UTC) #21
aelias_OOO_until_Jul13
As far as I know, UMA doesn't support logging strings at all. I'm definitely interested ...
3 years, 7 months ago (2017-05-24 22:09:55 UTC) #22
Changwan Ryu
On 2017/05/24 22:09:55, aelias wrote: > As far as I know, UMA doesn't support logging ...
3 years, 7 months ago (2017-05-24 22:20:34 UTC) #23
dmazzoni
On 2017/05/24 21:44:50, aelias wrote: > https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc > File content/browser/accessibility/browser_accessibility_manager_android.cc > (right): > > https://codereview.chromium.org/2903583006/diff/20001/content/browser/accessibility/browser_accessibility_manager_android.cc#newcode1230 ...
3 years, 7 months ago (2017-05-25 21:04:44 UTC) #25
dmazzoni
https://codereview.chromium.org/2903583006/diff/40001/content/browser/accessibility/browser_accessibility_manager_android.cc File content/browser/accessibility/browser_accessibility_manager_android.cc (right): https://codereview.chromium.org/2903583006/diff/40001/content/browser/accessibility/browser_accessibility_manager_android.cc#newcode1256 content/browser/accessibility/browser_accessibility_manager_android.cc:1256: EVENT_TYPE_HISTOGRAM(event_type_mask, ANNOUNCEMENT); On 2017/05/24 21:44:50, aelias wrote: > Looks ...
3 years, 7 months ago (2017-05-25 21:04:52 UTC) #26
dmazzoni
+mpearson for metrics review
3 years, 7 months ago (2017-05-25 21:06:47 UTC) #29
aelias_OOO_until_Jul13
lgtm
3 years, 7 months ago (2017-05-25 21:07:01 UTC) #31
paulmiller
lgtm
3 years, 7 months ago (2017-05-25 22:13:03 UTC) #32
Mark P
>>> Manually verified that TalkBack, Select-to-Speak, and LastPass all result in very different sets of ...
3 years, 7 months ago (2017-05-26 18:22:31 UTC) #36
dmazzoni
On 2017/05/26 18:22:31, Mark P wrote: > >>> > Manually verified that TalkBack, Select-to-Speak, and ...
3 years, 6 months ago (2017-05-31 17:28:50 UTC) #37
dmazzoni
https://codereview.chromium.org/2903583006/diff/60001/content/browser/accessibility/browser_accessibility_manager_android.cc File content/browser/accessibility/browser_accessibility_manager_android.cc (right): https://codereview.chromium.org/2903583006/diff/60001/content/browser/accessibility/browser_accessibility_manager_android.cc#newcode40 content/browser/accessibility/browser_accessibility_manager_android.cc:40: UMA_CAPABILITY_CAN_CONTROL_MAGNIFICATION = 0, On 2017/05/26 18:22:30, Mark P wrote: ...
3 years, 6 months ago (2017-05-31 17:54:28 UTC) #39
dmazzoni
https://codereview.chromium.org/2903583006/diff/60001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2903583006/diff/60001/tools/metrics/histograms/histograms.xml#newcode86 tools/metrics/histograms/histograms.xml:86: + Tracks flags and capabilities of enabled accessibility services. ...
3 years, 6 months ago (2017-05-31 18:51:37 UTC) #43
Mark P
lgtm modulo some minor comments. thanks for the thorough revisions. --mark https://codereview.chromium.org/2903583006/diff/80001/content/browser/accessibility/browser_accessibility_manager_android.cc File content/browser/accessibility/browser_accessibility_manager_android.cc (right): ...
3 years, 6 months ago (2017-05-31 23:35:03 UTC) #44
dmazzoni
https://codereview.chromium.org/2903583006/diff/80001/content/browser/accessibility/browser_accessibility_manager_android.cc File content/browser/accessibility/browser_accessibility_manager_android.cc (right): https://codereview.chromium.org/2903583006/diff/80001/content/browser/accessibility/browser_accessibility_manager_android.cc#newcode40 content/browser/accessibility/browser_accessibility_manager_android.cc:40: // Note: The string names for these enums have ...
3 years, 6 months ago (2017-06-01 18:47:05 UTC) #45
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2903583006/100001
3 years, 6 months ago (2017-06-01 18:47:39 UTC) #48
commit-bot: I haz the power
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_presubmit/builds/453533)
3 years, 6 months ago (2017-06-01 18:59:18 UTC) #50
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2903583006/120001
3 years, 6 months ago (2017-06-06 18:13:12 UTC) #56
commit-bot: I haz the power
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_presubmit/builds/456844)
3 years, 6 months ago (2017-06-06 18:23:29 UTC) #58
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2903583006/160001
3 years, 6 months ago (2017-06-07 17:45:34 UTC) #68
commit-bot: I haz the power
3 years, 6 months ago (2017-06-07 19:17:06 UTC) #71
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/f33b08933cff6791612633f2e4a1...

Powered by Google App Engine
This is Rietveld 408576698