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

Issue 1903103005: Log UMA metrics for permission type when permission bubbles shown. (Closed)

Created:
4 years, 8 months ago by benwells
Modified:
4 years, 7 months ago
CC:
chromium-reviews, mlamouri+watch-permissions_chromium.org, markusheintz_, msramek+watch_chromium.org, raymes+watch_chromium.org, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Log UMA metrics for permission type when permission bubbles shown. BUG=603384 Committed: https://crrev.com/597ad9d072afd837dd59bf80ac6fb6fcf4c7094c Cr-Commit-Position: refs/heads/master@{#390292}

Patch Set 1 #

Patch Set 2 : Doh #

Patch Set 3 : Rebase #

Patch Set 4 : Just record one enum #

Patch Set 5 : Fix chromeos compile #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -3 lines) Patch
M chrome/browser/permissions/permission_bubble_request_impl.cc View 1 2 3 4 1 chunk +19 lines, -1 line 2 comments Download
M chrome/browser/ui/website_settings/permission_bubble_request.h View 1 2 3 3 chunks +8 lines, -1 line 2 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 2 chunks +15 lines, -1 line 0 comments Download

Messages

Total messages: 22 (5 generated)
benwells
Ignore the bot redness, I think it's unrelated
4 years, 8 months ago (2016-04-21 03:58:23 UTC) #2
tsergeant
On 2016/04/21 03:58:23, benwells wrote: > Ignore the bot redness, I think it's unrelated Is ...
4 years, 8 months ago (2016-04-21 06:45:55 UTC) #3
benwells
On 2016/04/21 06:45:55, tsergeant wrote: > On 2016/04/21 03:58:23, benwells wrote: > > Ignore the ...
4 years, 8 months ago (2016-04-21 07:31:26 UTC) #4
tsergeant
lgtm
4 years, 8 months ago (2016-04-21 07:37:07 UTC) #5
benwells
+felt, +asvitkine for histograms. felt - this is no. 2 in a series of patches ...
4 years, 8 months ago (2016-04-21 07:40:17 UTC) #7
felt
why do we need both the permission bubble type, and the permission type?
4 years, 8 months ago (2016-04-21 13:52:31 UTC) #8
benwells
On 2016/04/21 13:52:31, felt wrote: > why do we need both the permission bubble type, ...
4 years, 8 months ago (2016-04-21 23:15:42 UTC) #9
Alexei Svitkine (slow)
lgtm
4 years, 8 months ago (2016-04-22 14:59:20 UTC) #10
benwells
felt - I've rearranged this to just use the one enum. I think this will ...
4 years, 7 months ago (2016-04-27 12:07:42 UTC) #11
felt
thanks i think this looks much better https://codereview.chromium.org/1903103005/diff/80001/chrome/browser/permissions/permission_bubble_request_impl.cc File chrome/browser/permissions/permission_bubble_request_impl.cc (right): https://codereview.chromium.org/1903103005/diff/80001/chrome/browser/permissions/permission_bubble_request_impl.cc#newcode186 chrome/browser/permissions/permission_bubble_request_impl.cc:186: return PermissionBubbleType::UNKNOWN; ...
4 years, 7 months ago (2016-04-27 15:37:24 UTC) #12
benwells
https://codereview.chromium.org/1903103005/diff/80001/chrome/browser/permissions/permission_bubble_request_impl.cc File chrome/browser/permissions/permission_bubble_request_impl.cc (right): https://codereview.chromium.org/1903103005/diff/80001/chrome/browser/permissions/permission_bubble_request_impl.cc#newcode186 chrome/browser/permissions/permission_bubble_request_impl.cc:186: return PermissionBubbleType::UNKNOWN; On 2016/04/27 15:37:24, felt wrote: > weird ...
4 years, 7 months ago (2016-04-28 00:16:59 UTC) #13
felt
lgtm
4 years, 7 months ago (2016-04-28 00:23:28 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1903103005/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1903103005/80001
4 years, 7 months ago (2016-04-28 03:42:10 UTC) #17
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 7 months ago (2016-04-28 03:46:28 UTC) #18
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/597ad9d072afd837dd59bf80ac6fb6fcf4c7094c Cr-Commit-Position: refs/heads/master@{#390292}
4 years, 7 months ago (2016-04-30 17:16:00 UTC) #19
rkaplow
On 2016/04/30 17:16:00, commit-bot: I haz the power wrote: > Patchset 5 (id:??) landed as ...
4 years, 7 months ago (2016-05-02 20:39:13 UTC) #21
benwells
4 years, 7 months ago (2016-05-04 18:41:32 UTC) #22
Message was sent while issue was closed.
On 2016/05/02 20:39:13, rkaplow wrote:
> On 2016/04/30 17:16:00, commit-bot: I haz the power wrote:
> > Patchset 5 (id:??) landed as
> > https://crrev.com/597ad9d072afd837dd59bf80ac6fb6fcf4c7094c
> > Cr-Commit-Position: refs/heads/master@{#390292}
> 
> Looks like some files were missing on the commit, 
permission_bubble_request.cc,
> permission_uma_util.cc.
> These should be readded it looks like since the new metric
> Permissions.Prompt.ShownForPermission isn't being recorded.

Actually that part of this histograms.xml change shouldn't have been included.
It is removed in https://codereview.chromium.org/1926003002/.

Powered by Google App Engine
This is Rietveld 408576698