mash: Remove ash/touch/touch_uma.h usage in chrome/browser/ui/views/touch_uma |
||||
Issue descriptionash/metrics/gesture_action_type.h could move to //ash/public/cpp, then the histogram calls could be inlined in chrome.
,
May 18 2018
Here's a relatively simple first bug. The goal is to eliminate access into //ash from code in //chrome. Both sides can record their own UMA metrics via UMA_HISTOGRAM_ENUMERATION(). Ideally we would separate out browser metrics from ash metrics into separate values. If we can't do that, and we just need to share some constants, the constants can go into a header in //ash/public/cpp. (This works because today the ash process and chrome process are the same binary, hence built at same revision, hence the constants are always the same in both processes. Also, UMA values are not allowed to change.) Alternately, if you can figure out that no one uses these metrics, you could delete them.
,
May 19 2018
Will take a look, thanks
,
Jun 8 2018
Even after moving the header file to //ash/public/cpp it seems weird to me that chrome would be recording metrics for ash. In this case the metric is "Ash.GestureTarget", and will be sent at https://cs.chromium.org/chromium/src/chrome/browser/ui/views/touch_uma/touch_uma_ash.cc?rcl=0a6839983e0efa1d846dc99d5291e59553b5abff&l=29 once the call is inlined. The touch handler for views seems very chromeos-specific - the generic touch_uma.cc implementation does nothing. Will dig deeper and see if we really do need tab gesture counts in Ash.GestureTarget.
,
Jun 11 2018
Yeah, maybe it could be split into two separate metrics, one for ash touch things, and one for chrome touch things. Or maybe one set of metrics could just be eliminated.
,
Jun 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/50441c1a60811e371a208b3c3841172655fbcf03 commit 50441c1a60811e371a208b3c3841172655fbcf03 Author: Ryan Cui <rcui@chromium.org> Date: Fri Jun 29 04:26:41 2018 Touch metric refactoring Adds new Event.Touch.GestureTarget histogram for browser UI elements, and stops including browser events in Ash.GestureTarget. Removes header and runtime dependency between browser and ash for gesture metric handling. BUG: 826476 Change-Id: I87e8de2c4cae6f5e30939726917f5f811cb61ba8 Reviewed-on: https://chromium-review.googlesource.com/1116250 Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Ryan Cui <rcui@chromium.org> Cr-Commit-Position: refs/heads/master@{#571387} [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/ash/touch/touch_uma.cc [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/chrome/browser/ui/views/frame/browser_root_view.cc [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/chrome/browser/ui/views/tabs/tab.cc [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/chrome/browser/ui/views/tabs/tab_strip.cc [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/chrome/browser/ui/views/touch_uma/touch_uma.cc [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/chrome/browser/ui/views/touch_uma/touch_uma.h [delete] https://crrev.com/3ac19b43a9c276b0169dd263f7cd33f42785c21b/chrome/browser/ui/views/touch_uma/touch_uma_ash.cc [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/tools/metrics/histograms/enums.xml [modify] https://crrev.com/50441c1a60811e371a208b3c3841172655fbcf03/tools/metrics/histograms/histograms.xml
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/73ad4656d2b1104e08e8f97cf680a07070ec98bb commit 73ad4656d2b1104e08e8f97cf680a07070ec98bb Author: Ryan Cui <rcui@chromium.org> Date: Fri Jul 13 04:00:01 2018 Deprecate unused touch metrics Bug: 826476 Change-Id: Id24d14cd5dc0ac31fac4e865f71047aa75c8a786 Reviewed-on: https://chromium-review.googlesource.com/1130819 Commit-Queue: Ryan Cui <rcui@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#574837} [modify] https://crrev.com/73ad4656d2b1104e08e8f97cf680a07070ec98bb/ash/touch/touch_uma.cc [modify] https://crrev.com/73ad4656d2b1104e08e8f97cf680a07070ec98bb/tools/metrics/histograms/histograms.xml
,
Jul 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eb5b84e304cd9a05b8595774e287c0c38769807b commit eb5b84e304cd9a05b8595774e287c0c38769807b Author: Ryan Cui <rcui@chromium.org> Date: Thu Jul 19 01:36:02 2018 Enable pretarget handlers for touch events in mash Bug: 826476 Change-Id: I6b30a7b5c4c252896d557db14e14a54ad4d36ee5 Reviewed-on: https://chromium-review.googlesource.com/1130827 Commit-Queue: Ryan Cui <rcui@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/master@{#576305} [modify] https://crrev.com/eb5b84e304cd9a05b8595774e287c0c38769807b/ash/shell.cc
,
Jul 19
|
||||
►
Sign in to add a comment |
||||
Comment 1 by jamescook@chromium.org
, Apr 19 2018