New issue
Advanced search Search tips

Issue 774353 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Team-Security-UX


Show other hotlists

Hotlists containing this issue:
EnamelAndFriendsFixIt


Sign in to add a comment

Investigate permission ignore count on Android with modals enabled

Project Member Reported by timloh@chromium.org, Oct 13 2017

Issue description

Modal permission prompts on Android are really hard to ignore, but for some reason our ignore rates right now are much higher than 0%. We should look into this and make sure our logging is accurate.
- Geolocation: 22%
- Notifications/Push: 15%
- EME: 24%
- Mic/Camera: 4%

Right now we log prompts from background tabs that never get shown as ignores. We shouldn't do this, but I don't expect it to be such a large percentage.

Ignoring is possible if a page navigates or is closed while the prompt is active. I think this would usually be due to a user clicking a link while a page is still loading. If the page fires an event on load, it will disappear once the navigation happens.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab06431dbeaa54b61fe44c535ac0b956f287c2cd

commit ab06431dbeaa54b61fe44c535ac0b956f287c2cd
Author: Timothy Loh <timloh@chromium.org>
Date: Mon Oct 16 05:07:05 2017

Don't log permission prompts that weren't shown as ignored

This patch fixes logging of ignored permission prompts so as to not log
in cases where prompts weren't shown (background tabs, queued requests).
Currently we log in ~PermissionRequestImpl; this patch moves it to
PermissionRequestManager::FinalizeBubble, which is the same place as we
use to log site engagement metrics for ignores. Repeated ignores embargo
is similarly moved.

BUG= 774353 

Change-Id: Ibcc1c09d63d58106d2db1640bfcbed003ded4169
Reviewed-on: https://chromium-review.googlesource.com/714798
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508986}
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_context_base.cc
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_request_impl.cc
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_request_impl.h
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_request_manager.cc
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_request_manager_browsertest.cc
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_request_manager_test_api.cc
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_request_manager_test_api.h
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_uma_util.cc
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/permissions/permission_uma_util.h
[modify] https://crrev.com/ab06431dbeaa54b61fe44c535ac0b956f287c2cd/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_interactive_uitest.mm

Comment 2 by timloh@chromium.org, Oct 19 2017

Labels: -Pri-3 Merge-Request-63 M-63 Pri-2
Requesting merge for the patch in comment #1, which make some metrics more accurate. This will help us understand the impact of modal permission prompts, which has been enabled in M63.
Project Member

Comment 3 by sheriffbot@chromium.org, Oct 20 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 23 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5

commit 114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5
Author: Timothy Loh <timloh@chromium.org>
Date: Mon Oct 23 02:31:17 2017

Don't log permission prompts that weren't shown as ignored

This patch fixes logging of ignored permission prompts so as to not log
in cases where prompts weren't shown (background tabs, queued requests).
Currently we log in ~PermissionRequestImpl; this patch moves it to
PermissionRequestManager::FinalizeBubble, which is the same place as we
use to log site engagement metrics for ignores. Repeated ignores embargo
is similarly moved.

BUG= 774353 

Change-Id: Ibcc1c09d63d58106d2db1640bfcbed003ded4169
Reviewed-on: https://chromium-review.googlesource.com/714798
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#508986}(cherry picked from commit ab06431dbeaa54b61fe44c535ac0b956f287c2cd)
Reviewed-on: https://chromium-review.googlesource.com/732757
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#142}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_context_base.cc
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_request_impl.cc
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_request_impl.h
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_request_manager.cc
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_request_manager_browsertest.cc
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_request_manager_test_api.cc
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_request_manager_test_api.h
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_uma_util.cc
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/permissions/permission_uma_util.h
[modify] https://crrev.com/114b82b05c6ce4bcbcc7c92f7ddd97dc14d7b1e5/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_interactive_uitest.mm

Looks a lot better now. Perhaps it was mostly due to background tabs...
- Geolocation: 4.5%
- Notifications: 3%
- EME: 2%
- Mic/Camera: 1-2%

Going to leave this bug open for now and will check back in some time in the future.

Comment 6 by est...@chromium.org, Nov 10 2017

Labels: Hotlist-EnamelAndFriendsFixIt
Status: Fixed (was: Started)
Numbers haven't really changed since comment #5, going to close because I don't think there's more for us to do right now.

Sign in to add a comment