Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2600173002:
Add metrics to track buttons clicked in IME menu tray. (Closed)
|
Created:
3 years, 12 months ago by Azure Wei Modified:
3 years, 11 months ago CC:
chromium-reviews, sadrul, yusukes+watch_chromium.org, shuchen+watch_chromium.org, asvitkine+watch_chromium.org, nona+watch_chromium.org, oshima+watch_chromium.org, kalyank, tdanderson Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd metric to track buttons clicked in IME menu tray.
Add an enum histogram "InputMethod.ImeMenu.EmojiHandwritingVoiceButton"
to track the number of times users click emoji, handwriting or voice
button in opt-in IME menu.
The enum value represents:
1: Emoji
2: Handwriting
3: Voice
BUG=676798
TEST=None
Review-Url: https://codereview.chromium.org/2600173002
Cr-Commit-Position: refs/heads/master@{#442784}
Committed: https://chromium.googlesource.com/chromium/src/+/19203f9504b1d76ec4457c1775d0f789967ef8b7
Patch Set 1 #
Total comments: 2
Patch Set 2 : Use InputMethod.ImeMenu.EmojiHandwritingVoiceButton. #
Total comments: 2
Patch Set 3 : Add <enum> entry. #Patch Set 4 : sync #
Messages
Total messages: 28 (16 generated)
Description was changed from ========== Add metrics to track buttons clicked in IME menu tray. BUG= ========== to ========== Add metrics to track buttons clicked in IME menu tray. Add three counter metrics: InputMethod.ImeMenu.EmojiButton - track the number of times users click emoji button in opt-in IME menu. InputMethod.ImeMenu.HandwritingButton - track the number of times users click handwriting button in opt-in IME menu. InputMethod.ImeMenu.VoiceButton - track the number of times users click voice button in opt-in IME menu. BUG=676798 TEST=None ==========
The CQ bit was checked by azurewei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
azurewei@chromium.org changed reviewers: + asvitkine@chromium.org, shuchen@chromium.org, tdanderson@chromium.org
https://codereview.chromium.org/2600173002/diff/1/ash/common/system/chromeos/... File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right): https://codereview.chromium.org/2600173002/diff/1/ash/common/system/chromeos/... ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:214: UMA_HISTOGRAM_COUNTS("InputMethod.ImeMenu.HandwritingButton", 1); UMA_HISTOGRAM_COUNTS allocates 50 buckets but you're only logging to bucket 1. This is wasteful. It would be more efficient to log an UMA_HISTOGRAM_BOOLEAN. However, given you have 3 things that are related, you would be better served making this an enum histogram, with the different cases as different enum entries. If you do that, suggest making a helper function to log the enum histogram so that you don't repeat the macro - as each macro expands to a bunch of code and thus multiple macro statements bloat the binary.
Description was changed from ========== Add metrics to track buttons clicked in IME menu tray. Add three counter metrics: InputMethod.ImeMenu.EmojiButton - track the number of times users click emoji button in opt-in IME menu. InputMethod.ImeMenu.HandwritingButton - track the number of times users click handwriting button in opt-in IME menu. InputMethod.ImeMenu.VoiceButton - track the number of times users click voice button in opt-in IME menu. BUG=676798 TEST=None ========== to ========== Add metric to track buttons clicked in IME menu tray. Add an enum histogram "InputMethod.ImeMenu.EmojiHandwritingVoiceButton" to track the number of times users click emoji, handwriting or voice button in opt-in IME menu. The enum value represents: 1: Emoji 2: Handwriting 3: Voice BUG=676798 TEST=None ==========
The CQ bit was checked by azurewei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2600173002/diff/1/ash/common/system/chromeos/... File ash/common/system/chromeos/ime_menu/ime_menu_tray.cc (right): https://codereview.chromium.org/2600173002/diff/1/ash/common/system/chromeos/... ash/common/system/chromeos/ime_menu/ime_menu_tray.cc:214: UMA_HISTOGRAM_COUNTS("InputMethod.ImeMenu.HandwritingButton", 1); On 2017/01/03 15:47:22, Alexei Svitkine (slow) wrote: > UMA_HISTOGRAM_COUNTS allocates 50 buckets but you're only logging to bucket 1. > This is wasteful. > > It would be more efficient to log an UMA_HISTOGRAM_BOOLEAN. > > However, given you have 3 things that are related, you would be better served > making this an enum histogram, with the different cases as different enum > entries. > > If you do that, suggest making a helper function to log the enum histogram so > that you don't repeat the macro - as each macro expands to a bunch of code and > thus multiple macro statements bloat the binary. Thanks. I've updated these with enum histogram and added a helper method RecordButtonsClicked(). Please take another look. Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2600173002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2600173002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:21468: +<histogram name="InputMethod.ImeMenu.EmojiHandwritingVoiceButton"> The enum also needs to be added to histograms.xml - please add an <enum> entry and reference it from the histogram via enum= attribute.
https://codereview.chromium.org/2600173002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2600173002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:21468: +<histogram name="InputMethod.ImeMenu.EmojiHandwritingVoiceButton"> On 2017/01/04 15:12:16, Alexei Svitkine (slow) wrote: > The enum also needs to be added to histograms.xml - please add an <enum> entry > and reference it from the histogram via enum= attribute. Added enum entry ImeMenuButtonType. Thanks.
lgtm
azurewei@chromium.org changed reviewers: + jamescook@chromium.org - tdanderson@chromium.org
ping~
LGTM (sorry, I didn't get a review request email until your ping)
On 2017/01/10 18:26:42, James Cook wrote: > LGTM (sorry, I didn't get a review request email until your ping) Thanks for reviewing!
The CQ bit was checked by azurewei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from asvitkine@chromium.org Link to the patchset: https://codereview.chromium.org/2600173002/#ps60001 (title: "sync")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484099409830650,
"parent_rev": "995f87df43d91278d4b1e0c5ad86de7d0007125b", "commit_rev":
"19203f9504b1d76ec4457c1775d0f789967ef8b7"}
Message was sent while issue was closed.
Description was changed from ========== Add metric to track buttons clicked in IME menu tray. Add an enum histogram "InputMethod.ImeMenu.EmojiHandwritingVoiceButton" to track the number of times users click emoji, handwriting or voice button in opt-in IME menu. The enum value represents: 1: Emoji 2: Handwriting 3: Voice BUG=676798 TEST=None ========== to ========== Add metric to track buttons clicked in IME menu tray. Add an enum histogram "InputMethod.ImeMenu.EmojiHandwritingVoiceButton" to track the number of times users click emoji, handwriting or voice button in opt-in IME menu. The enum value represents: 1: Emoji 2: Handwriting 3: Voice BUG=676798 TEST=None Review-Url: https://codereview.chromium.org/2600173002 Cr-Commit-Position: refs/heads/master@{#442784} Committed: https://chromium.googlesource.com/chromium/src/+/19203f9504b1d76ec4457c1775d0... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/19203f9504b1d76ec4457c1775d0... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
