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

Issue 2742633002: [Mac] UMA Metrics for the Default Touch Bar (Closed)

Created:
3 years, 9 months ago by spqchan
Modified:
3 years, 9 months ago
CC:
chromium-reviews, asvitkine+watch_chromium.org, mac-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Mac] UMA Metrics for the Default Touch Bar BUG=699321 Review-Url: https://codereview.chromium.org/2742633002 Cr-Commit-Position: refs/heads/master@{#456769} Committed: https://chromium.googlesource.com/chromium/src/+/89dcacd0a5b19051970fafaab3321338a50833b4

Patch Set 1 #

Patch Set 2 : Added a comment #

Patch Set 3 : nit #

Total comments: 5

Patch Set 4 : update for home button #

Total comments: 2

Patch Set 5 : Fix for rsesek #

Total comments: 2

Patch Set 6 : Fix for isherman #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -5 lines) Patch
M chrome/browser/ui/cocoa/browser_window_touch_bar.mm View 1 2 3 4 4 chunks +53 lines, -5 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 2 chunks +16 lines, -0 lines 0 comments Download

Messages

Total messages: 55 (41 generated)
spqchan
PTAL +rsesek for cocoa +isherman for histogram.xml
3 years, 9 months ago (2017-03-09 18:09:01 UTC) #16
Robert Sesek
https://codereview.chromium.org/2742633002/diff/40001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right): https://codereview.chromium.org/2742633002/diff/40001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm#newcode114 chrome/browser/ui/cocoa/browser_window_touch_bar.mm:114: UMA_HISTOGRAM_ENUMERATION("DefaultTouchBar.Metrics", action, Since we may have multiple touch bars, ...
3 years, 9 months ago (2017-03-09 18:11:55 UTC) #17
spqchan
On 2017/03/09 18:11:55, Robert Sesek wrote: > https://codereview.chromium.org/2742633002/diff/40001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm > File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right): > > https://codereview.chromium.org/2742633002/diff/40001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm#newcode114 ...
3 years, 9 months ago (2017-03-09 18:32:31 UTC) #18
Ilya Sherman
Metrics LGTM % the metric name discussion. https://codereview.chromium.org/2742633002/diff/40001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right): https://codereview.chromium.org/2742633002/diff/40001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm#newcode114 chrome/browser/ui/cocoa/browser_window_touch_bar.mm:114: UMA_HISTOGRAM_ENUMERATION("DefaultTouchBar.Metrics", action, ...
3 years, 9 months ago (2017-03-09 20:13:44 UTC) #19
Robert Sesek
On 2017/03/09 18:32:31, spqchan wrote: > Sorry, can we hold the CL? I just got ...
3 years, 9 months ago (2017-03-09 23:46:55 UTC) #27
spqchan
On 2017/03/09 23:46:55, Robert Sesek wrote: > On 2017/03/09 18:32:31, spqchan wrote: > > Sorry, ...
3 years, 9 months ago (2017-03-09 23:58:44 UTC) #28
spqchan
PTAL, I added the metric for the home button here. isherman@, can you look at ...
3 years, 9 months ago (2017-03-13 20:58:05 UTC) #38
Robert Sesek
https://codereview.chromium.org/2742633002/diff/100001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right): https://codereview.chromium.org/2742633002/diff/100001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm#newcode104 chrome/browser/ui/cocoa/browser_window_touch_bar.mm:104: if (command == IDC_BACK) Would this be better as ...
3 years, 9 months ago (2017-03-13 21:05:25 UTC) #39
Ilya Sherman
LGTM https://codereview.chromium.org/2742633002/diff/120001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2742633002/diff/120001/tools/metrics/histograms/histograms.xml#newcode72228 tools/metrics/histograms/histograms.xml:72228: +<histogram name="TouchBar.Default.Metrics" enum="DefaultTouchBarAction"> nit: Maybe s/Metrics/Actions?
3 years, 9 months ago (2017-03-13 22:25:03 UTC) #40
spqchan
Thanks! rsesek, PTAL https://codereview.chromium.org/2742633002/diff/100001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm File chrome/browser/ui/cocoa/browser_window_touch_bar.mm (right): https://codereview.chromium.org/2742633002/diff/100001/chrome/browser/ui/cocoa/browser_window_touch_bar.mm#newcode104 chrome/browser/ui/cocoa/browser_window_touch_bar.mm:104: if (command == IDC_BACK) On 2017/03/13 ...
3 years, 9 months ago (2017-03-14 00:04:33 UTC) #45
Robert Sesek
LGTM
3 years, 9 months ago (2017-03-14 02:20:05 UTC) #48
spqchan
On 2017/03/14 02:20:05, Robert Sesek wrote: > LGTM thanks!
3 years, 9 months ago (2017-03-14 18:25:51 UTC) #49
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/2742633002/140001
3 years, 9 months ago (2017-03-14 18:26:41 UTC) #52
commit-bot: I haz the power
3 years, 9 months ago (2017-03-14 18:34:34 UTC) #55
Message was sent while issue was closed.
Committed patchset #6 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/89dcacd0a5b19051970fafaab332...

Powered by Google App Engine
This is Rietveld 408576698