|
|
Description[ash-md] Adds UMA items count and selection depth for Alt+Tab
Adds Ash.WindowCycleController.Items UMA histogram that records how
many windows are included in Alt+Tab enumeration. The count is captured
when Alt+Tab is pressed.
Adds Ash.WindowCycleController.SelectionDepth UMA histogram that records
how often users finish Alt+Tab selection on a window that is N-th in MRU
order. Selecting a window that is already currently active (top in MRU
order) records 1.
BUG=621562
Committed: https://crrev.com/a81779b51a943a37acf879c458d9c84fade53251
Cr-Commit-Position: refs/heads/master@{#402365}
Patch Set 1 : [ash-md] Adds UMA items count and selection depth for Alt+Tab (rebased) #
Total comments: 10
Patch Set 2 : [ash-md] Adds UMA items count and selection depth for Alt+Tab (comments) #
Messages
Total messages: 32 (15 generated)
The CQ bit was checked by varkha@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...
varkha@chromium.org changed reviewers: + rkaplow@chromium.org, tdanderson@chromium.org
Can you please take a look? tdanderson@ for changes in ash/wm/. rkaplow@ for OWNERS in tools/metrics/histograms/histograms.xml. Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
The CQ bit was checked by varkha@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...
Patchset #1 (id:1) has been deleted
LGTM with the comments below https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_con... File ash/wm/window_cycle_controller.cc (right): https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_con... ash/wm/window_cycle_controller.cc:112: int depth = window_cycle_list_->current_index(); nit: no need for a local variable, just inline on line 123 https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_list.h File ash/wm/window_cycle_list.h (right): https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_lis... ash/wm/window_cycle_list.h:53: // i.e. position of an active window in a global MRU ordering. micro nit: "... query selection depth, i.e., the position ..." https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:1530: + <owner>tbuckley@google.com</owner> are these actually the owners you want or is this a copy/paste error? https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:1532: + The number of windows in the Alt+Tab selector when Alt+Tab is pressed. I would clarify that it's only recorded when you start cycling, not each time Alt+Tab is pressed (e.g., if I press Alt+Tab three times consecutively without releasing Alt then the histogram is only written to once, not three times) https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:1541: + When a window is selected after pressing Alt+Tab, records that window's Similar to above, perhaps clarify that it's recorded when Alt+Tab cycling has finished.
https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_con... File ash/wm/window_cycle_controller.cc (right): https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_con... ash/wm/window_cycle_controller.cc:112: int depth = window_cycle_list_->current_index(); On 2016/06/24 19:25:56, tdanderson wrote: > nit: no need for a local variable, just inline on line 123 |window_cycle_list_| goes away in the next line but I think I can move the UMA statement here. https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_list.h File ash/wm/window_cycle_list.h (right): https://codereview.chromium.org/2096063002/diff/20001/ash/wm/window_cycle_lis... ash/wm/window_cycle_list.h:53: // i.e. position of an active window in a global MRU ordering. On 2016/06/24 19:25:56, tdanderson wrote: > micro nit: "... query selection depth, i.e., the position ..." Done. https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:1530: + <owner>tbuckley@google.com</owner> On 2016/06/24 19:25:56, tdanderson wrote: > are these actually the owners you want or is this a copy/paste error? Done. https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:1532: + The number of windows in the Alt+Tab selector when Alt+Tab is pressed. On 2016/06/24 19:25:56, tdanderson wrote: > I would clarify that it's only recorded when you start cycling, not each time > Alt+Tab is pressed (e.g., if I press Alt+Tab three times consecutively without > releasing Alt then the histogram is only written to once, not three times) Done. https://codereview.chromium.org/2096063002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:1541: + When a window is selected after pressing Alt+Tab, records that window's On 2016/06/24 19:25:56, tdanderson wrote: > Similar to above, perhaps clarify that it's recorded when Alt+Tab cycling has > finished. Done.
The CQ bit was checked by varkha@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.
varkha@chromium.org changed reviewers: + holte@chromium.org - rkaplow@chromium.org
holte@, can you please take a look for tools/metrics/OWNERS? Thanks!
varkha@chromium.org changed reviewers: + isherman@chromium.org
+isherman@ for OWNERS just in case.
varkha@chromium.org changed reviewers: + rkaplow@chromium.org - holte@chromium.org, isherman@chromium.org
Back to rkaplow@ for owners. Thanks!
lgtm
The CQ bit was checked by varkha@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tdanderson@chromium.org Link to the patchset: https://codereview.chromium.org/2096063002/#ps40001 (title: "[ash-md] Adds UMA items count and selection depth for Alt+Tab (comments)")
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
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by varkha@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [ash-md] Adds UMA items count and selection depth for Alt+Tab Adds Ash.WindowCycleController.Items UMA histogram that records how many windows are included in Alt+Tab enumeration. The count is captured when Alt+Tab is pressed. Adds Ash.WindowCycleController.SelectionDepth UMA histogram that records how often users finish Alt+Tab selection on a window that is N-th in MRU order. Selecting a window that is already currently active (top in MRU order) records 1. BUG=621562 ========== to ========== [ash-md] Adds UMA items count and selection depth for Alt+Tab Adds Ash.WindowCycleController.Items UMA histogram that records how many windows are included in Alt+Tab enumeration. The count is captured when Alt+Tab is pressed. Adds Ash.WindowCycleController.SelectionDepth UMA histogram that records how often users finish Alt+Tab selection on a window that is N-th in MRU order. Selecting a window that is already currently active (top in MRU order) records 1. BUG=621562 Committed: https://crrev.com/a81779b51a943a37acf879c458d9c84fade53251 Cr-Commit-Position: refs/heads/master@{#402365} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a81779b51a943a37acf879c458d9c84fade53251 Cr-Commit-Position: refs/heads/master@{#402365} |