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

Issue 2986483002: Adding metrics logging for Simplified Gesture Navigation. (Closed)

Created:
3 years, 5 months ago by mfomitchev
Modified:
3 years, 5 months ago
Reviewers:
mohsen, nasko
CC:
chromium-reviews, jam, darin-cc_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Adding metrics logging for Simplified Gesture Navigation. UMA Metrics and User Actions similar to those logged with the default gesture navigation. BUG=668296 Review-Url: https://codereview.chromium.org/2986483002 Cr-Commit-Position: refs/heads/master@{#489483} Committed: https://chromium.googlesource.com/chromium/src/+/06b84645aa7afc4bb115a0341a38edf5a3baf98d

Patch Set 1 #

Patch Set 2 : Rebase. #

Patch Set 3 : Fixing tests. #

Total comments: 4

Patch Set 4 : Addressing feedback #

Total comments: 2

Patch Set 5 : Addressing a nit. #

Patch Set 6 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+126 lines, -66 lines) Patch
M content/browser/BUILD.gn View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/web_contents/aura/gesture_nav_simple.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/web_contents/aura/gesture_nav_simple.cc View 1 2 3 4 5 4 chunks +57 lines, -3 lines 0 comments Download
M content/browser/web_contents/aura/overscroll_navigation_overlay.h View 2 chunks +1 line, -2 lines 0 comments Download
M content/browser/web_contents/aura/overscroll_navigation_overlay.cc View 1 2 3 4 11 chunks +16 lines, -31 lines 0 comments Download
M content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc View 1 2 9 chunks +11 lines, -11 lines 0 comments Download
A + content/browser/web_contents/aura/types.h View 2 chunks +12 lines, -0 lines 0 comments Download
A content/browser/web_contents/aura/types.cc View 1 chunk +26 lines, -0 lines 0 comments Download
D content/browser/web_contents/aura/uma_navigation_type.h View 1 chunk +0 lines, -18 lines 0 comments Download

Messages

Total messages: 40 (25 generated)
mfomitchev
Mohsen, can you PTAL?
3 years, 5 months ago (2017-07-20 02:06:10 UTC) #14
mohsen
https://codereview.chromium.org/2986483002/diff/40001/content/browser/web_contents/aura/gesture_nav_simple.h File content/browser/web_contents/aura/gesture_nav_simple.h (right): https://codereview.chromium.org/2986483002/diff/40001/content/browser/web_contents/aura/gesture_nav_simple.h#newcode47 content/browser/web_contents/aura/gesture_nav_simple.h:47: NavigationDirection direction_ = NavigationDirection::NONE; Instead of adding |direction_|, can ...
3 years, 5 months ago (2017-07-20 18:00:41 UTC) #15
mfomitchev
https://codereview.chromium.org/2986483002/diff/40001/content/browser/web_contents/aura/gesture_nav_simple.h File content/browser/web_contents/aura/gesture_nav_simple.h (right): https://codereview.chromium.org/2986483002/diff/40001/content/browser/web_contents/aura/gesture_nav_simple.h#newcode47 content/browser/web_contents/aura/gesture_nav_simple.h:47: NavigationDirection direction_ = NavigationDirection::NONE; On 2017/07/20 18:00:41, mohsen wrote: ...
3 years, 5 months ago (2017-07-21 00:10:07 UTC) #16
mohsen
LGTM with a nit https://codereview.chromium.org/2986483002/diff/60001/content/browser/web_contents/aura/overscroll_navigation_overlay.cc File content/browser/web_contents/aura/overscroll_navigation_overlay.cc (right): https://codereview.chromium.org/2986483002/diff/60001/content/browser/web_contents/aura/overscroll_navigation_overlay.cc#newcode52 content/browser/web_contents/aura/overscroll_navigation_overlay.cc:52: OverscrollSource source) { nit: Fits ...
3 years, 5 months ago (2017-07-21 00:51:12 UTC) #17
mfomitchev
https://codereview.chromium.org/2986483002/diff/60001/content/browser/web_contents/aura/overscroll_navigation_overlay.cc File content/browser/web_contents/aura/overscroll_navigation_overlay.cc (right): https://codereview.chromium.org/2986483002/diff/60001/content/browser/web_contents/aura/overscroll_navigation_overlay.cc#newcode52 content/browser/web_contents/aura/overscroll_navigation_overlay.cc:52: OverscrollSource source) { On 2017/07/21 00:51:12, mohsen wrote: > ...
3 years, 5 months ago (2017-07-22 02:34:31 UTC) #18
mfomitchev
nasko@chromium.org: Please review changes in content/browser/BUILD.gn
3 years, 5 months ago (2017-07-22 02:35:30 UTC) #20
nasko
Rubberstamp LGTM
3 years, 5 months ago (2017-07-24 19:59:10 UTC) #21
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/2986483002/80001
3 years, 5 months ago (2017-07-24 20:12:08 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/507948)
3 years, 5 months ago (2017-07-24 20:16:39 UTC) #26
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/2986483002/100001
3 years, 5 months ago (2017-07-25 18:26:46 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/346705)
3 years, 5 months ago (2017-07-25 19:31:45 UTC) #31
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/2986483002/100001
3 years, 5 months ago (2017-07-25 20:46:50 UTC) #33
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/346865)
3 years, 5 months ago (2017-07-25 22:27:49 UTC) #35
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/2986483002/100001
3 years, 5 months ago (2017-07-25 22:35:44 UTC) #37
commit-bot: I haz the power
3 years, 5 months ago (2017-07-25 23:39:32 UTC) #40
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/06b84645aa7afc4bb115a0341a38...

Powered by Google App Engine
This is Rietveld 408576698