Skip to content

Commit

Permalink
Record permission prompt gesture metrics on Android.
Browse files Browse the repository at this point in the history
The permissions infrastructure currently records gesture metrics for
desktop platforms through PermissionRequestManager. This does not record
metrics on Android, and will not do so until in-progress refactoring to
use PermissionRequestManager on that platform is complete.

This CL adds recording of gesture metrics on Android in the Android Java
UI-specific code paths of PermissionQueueController and
PermissionBubbleMediaAccessHandler. This is a temporary measure so that
these stats are recorded; once Android uses PermissionRequestManager to
display permission prompts, the PermissionQueueController call site will
be unnecessary and can be removed. The
PermissionBubbleMediaAccessHandler call site will be obsolete when the
media stream permission handling is consolidated with all other
permissions.

BUG=614599

Review-Url: https://codereview.chromium.org/2463393003
Cr-Commit-Position: refs/heads/master@{#431384}
  • Loading branch information
dominickn authored and Commit bot committed Nov 10, 2016
1 parent 8f60271 commit 83c36cc
Show file tree
Hide file tree
Showing 12 changed files with 144 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ void DoNothing(bool update_content_setting, PermissionAction decision) {}
// static
bool MediaStreamInfoBarDelegateAndroid::Create(
content::WebContents* web_contents,
bool user_gesture,
std::unique_ptr<MediaStreamDevicesController> controller) {
InfoBarService* infobar_service =
InfoBarService::FromWebContents(web_contents);
Expand All @@ -47,6 +48,7 @@ bool MediaStreamInfoBarDelegateAndroid::Create(
CreatePermissionInfoBar(std::unique_ptr<PermissionInfoBarDelegate>(
new MediaStreamInfoBarDelegateAndroid(
Profile::FromBrowserContext(web_contents->GetBrowserContext()),
user_gesture,
std::move(controller)))));
for (size_t i = 0; i < infobar_service->infobar_count(); ++i) {
infobars::InfoBar* old_infobar = infobar_service->infobar_at(i);
Expand Down Expand Up @@ -76,6 +78,7 @@ int MediaStreamInfoBarDelegateAndroid::GetIconId() const {

MediaStreamInfoBarDelegateAndroid::MediaStreamInfoBarDelegateAndroid(
Profile* profile,
bool user_gesture,
std::unique_ptr<MediaStreamDevicesController> controller)
: PermissionInfoBarDelegate(
controller->GetOrigin(),
Expand All @@ -87,7 +90,7 @@ MediaStreamInfoBarDelegateAndroid::MediaStreamInfoBarDelegateAndroid(
// GroupedPermissionInfoBarDelegate. See crbug.com/606138.
content::PermissionType::AUDIO_CAPTURE,
CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,
false,
user_gesture,
profile,
// This is only passed in to fit into PermissionInfoBarDelegate.
// Infobar accepted/denied/dismissed is handled by controller, not via
Expand Down Expand Up @@ -137,12 +140,16 @@ int MediaStreamInfoBarDelegateAndroid::GetMessageResourceId() const {
}

bool MediaStreamInfoBarDelegateAndroid::Accept() {
// TODO(dominickn): fold these metrics calls into
// PermissionUmaUtil::PermissionGranted. See crbug.com/638076.
PermissionUmaUtil::RecordPermissionPromptAccepted(
controller_->GetPermissionRequestType(),
PermissionUtil::GetGestureType(user_gesture()));

bool persist_permission = true;
if (ShouldShowPersistenceToggle()) {
persist_permission = persist();

// TODO(dominickn): fold these metrics calls into
// PermissionUmaUtil::PermissionGranted. See crbug.com/638076.
if (controller_->IsAskingForAudio()) {
PermissionUmaUtil::PermissionPromptAcceptedWithPersistenceToggle(
content::PermissionType::AUDIO_CAPTURE, persist_permission);
Expand All @@ -159,12 +166,16 @@ bool MediaStreamInfoBarDelegateAndroid::Accept() {
}

bool MediaStreamInfoBarDelegateAndroid::Cancel() {
// TODO(dominickn): fold these metrics calls into
// PermissionUmaUtil::PermissionDenied. See crbug.com/638076.
PermissionUmaUtil::RecordPermissionPromptDenied(
controller_->GetPermissionRequestType(),
PermissionUtil::GetGestureType(user_gesture()));

bool persist_permission = true;
if (ShouldShowPersistenceToggle()) {
persist_permission = persist();

// TODO(dominickn): fold these metrics calls into
// PermissionUmaUtil::PermissionGranted. See crbug.com/638076.
if (controller_->IsAskingForAudio()) {
PermissionUmaUtil::PermissionPromptDeniedWithPersistenceToggle(
content::PermissionType::AUDIO_CAPTURE, persist_permission);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ class MediaStreamInfoBarDelegateAndroid : public PermissionInfoBarDelegate {
// found, or just adds the new infobar otherwise. Returns whether an infobar
// was created.
static bool Create(content::WebContents* web_contents,
bool user_gesture,
std::unique_ptr<MediaStreamDevicesController> controller);

MediaStreamInfoBarDelegateAndroid(
Profile* profile,
bool user_gesture,
std::unique_ptr<MediaStreamDevicesController> controller);
private:
friend class WebRtcTestBase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
#include "chrome/browser/media/webrtc/media_stream_infobar_delegate_android.h"
#include "chrome/browser/media/webrtc/screen_capture_infobar_delegate_android.h"
#include "chrome/browser/permissions/permission_dialog_delegate.h"
#include "chrome/browser/permissions/permission_uma_util.h"
#include "chrome/browser/permissions/permission_update_infobar_delegate_android.h"
#include "chrome/browser/permissions/permission_util.h"
#else
#include "chrome/browser/permissions/permission_request_manager.h"
#endif // BUILDFLAG(ANDROID_JAVA_UI)
Expand Down Expand Up @@ -74,8 +76,7 @@ PermissionBubbleMediaAccessHandler::PermissionBubbleMediaAccessHandler() {
content::NotificationService::AllSources());
}

PermissionBubbleMediaAccessHandler::~PermissionBubbleMediaAccessHandler() {
}
PermissionBubbleMediaAccessHandler::~PermissionBubbleMediaAccessHandler() {}

bool PermissionBubbleMediaAccessHandler::SupportsStreamType(
const content::MediaStreamType type,
Expand Down Expand Up @@ -103,7 +104,8 @@ bool PermissionBubbleMediaAccessHandler::CheckMediaAccessPermission(
: CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA;

MediaPermission permission(content_settings_type, security_origin,
web_contents->GetLastCommittedURL().GetOrigin(), profile);
web_contents->GetLastCommittedURL().GetOrigin(),
profile);
content::MediaStreamRequestResult unused;
return permission.GetPermissionStatus(&unused) == CONTENT_SETTING_ALLOW;
}
Expand Down Expand Up @@ -184,20 +186,22 @@ void PermissionBubbleMediaAccessHandler::ProcessQueuedAccessRequest(
web_contents, content_settings_types)) {
PermissionUpdateInfoBarDelegate::Create(
web_contents, content_settings_types,
base::Bind(
&OnPermissionConflictResolved, base::Passed(&controller)));
base::Bind(&OnPermissionConflictResolved, base::Passed(&controller)));
}
#endif
return;
}

#if BUILDFLAG(ANDROID_JAVA_UI)
PermissionUmaUtil::RecordPermissionPromptShown(
controller->GetPermissionRequestType(),
PermissionUtil::GetGestureType(request.user_gesture));
if (PermissionDialogDelegate::ShouldShowDialog(request.user_gesture)) {
PermissionDialogDelegate::CreateMediaStreamDialog(web_contents,
std::move(controller));
PermissionDialogDelegate::CreateMediaStreamDialog(
web_contents, request.user_gesture, std::move(controller));
} else {
MediaStreamInfoBarDelegateAndroid::Create(web_contents,
std::move(controller));
MediaStreamInfoBarDelegateAndroid::Create(
web_contents, request.user_gesture, std::move(controller));
}
#else
PermissionRequestManager* permission_request_manager =
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/permissions/permission_dialog_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,13 @@ void PermissionDialogDelegate::Create(
// static
void PermissionDialogDelegate::CreateMediaStreamDialog(
content::WebContents* web_contents,
bool user_gesture,
std::unique_ptr<MediaStreamDevicesController> controller) {
// Called this way because the infobar delegate has a private destructor.
std::unique_ptr<PermissionInfoBarDelegate> infobar_delegate;
infobar_delegate.reset(new MediaStreamInfoBarDelegateAndroid(
Profile::FromBrowserContext(web_contents->GetBrowserContext()),
user_gesture,
std::move(controller)));

// Dispatch the dialog to Java, which manages the lifetime of this object.
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/permissions/permission_dialog_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class PermissionDialogDelegate : content::WebContentsObserver {
// folded in with other permission requests.
static void CreateMediaStreamDialog(
content::WebContents* web_contents,
bool user_gesture,
std::unique_ptr<MediaStreamDevicesController> controller);

// Returns true if we should show the user a modal permission prompt rather
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/permissions/permission_infobar_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ class PermissionInfoBarDelegate : public ConfirmInfoBarDelegate {
void set_persist(bool persist) { persist_ = persist; }
bool persist() const { return persist_; }

bool user_gesture() const { return user_gesture_; }

// ConfirmInfoBarDelegate:
bool Accept() override;
bool Cancel() override;
Expand Down
24 changes: 20 additions & 4 deletions chrome/browser/permissions/permission_queue_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/permissions/permission_queue_controller.h"

#include "build/build_config.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/infobars/infobar_service.h"
Expand Down Expand Up @@ -64,6 +65,14 @@ class PermissionQueueController::PendingInfobarRequest {
bool IsForPair(const GURL& requesting_frame,
const GURL& embedder) const;

PermissionRequestType request_type() const {
return PermissionUtil::GetRequestType(type_);
}

PermissionRequestGestureType gesture_type() const {
return PermissionUtil::GetGestureType(user_gesture_);
}

const PermissionRequestID& id() const { return id_; }
const GURL& requesting_frame() const { return requesting_frame_; }
bool has_gesture() const { return user_gesture_; }
Expand Down Expand Up @@ -209,19 +218,22 @@ void PermissionQueueController::OnPermissionSet(const PermissionRequestID& id,
PermissionAction decision) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

// TODO(miguelg): move the permission persistence to
// PermissionContextBase once all the types are moved there.
PermissionRequestType request_type =
PermissionUtil::GetRequestType(permission_type_);
PermissionRequestGestureType gesture_type =
user_gesture ? PermissionRequestGestureType::GESTURE
: PermissionRequestGestureType::NO_GESTURE;
PermissionUtil::GetGestureType(user_gesture);
switch (decision) {
case GRANTED:
PermissionUmaUtil::PermissionGranted(permission_type_, gesture_type,
requesting_frame, profile_);
PermissionUmaUtil::RecordPermissionPromptAccepted(request_type,
gesture_type);
break;
case DENIED:
PermissionUmaUtil::PermissionDenied(permission_type_, gesture_type,
requesting_frame, profile_);
PermissionUmaUtil::RecordPermissionPromptDenied(request_type,
gesture_type);
break;
case DISMISSED:
PermissionUmaUtil::PermissionDismissed(permission_type_, gesture_type,
Expand All @@ -231,6 +243,8 @@ void PermissionQueueController::OnPermissionSet(const PermissionRequestID& id,
NOTREACHED();
}

// TODO(miguelg): move the permission persistence to
// PermissionContextBase once all the types are moved there.
if (update_content_setting)
UpdateContentSetting(requesting_frame, embedder, decision);

Expand Down Expand Up @@ -361,6 +375,8 @@ void PermissionQueueController::ShowQueuedInfoBarForTab(
if (!show_dialog)
RegisterForInfoBarNotifications(infobar_service);

PermissionUmaUtil::RecordPermissionPromptShown(i->request_type(),
i->gesture_type());
i->CreatePrompt(this, show_dialog);
return;
}
Expand Down
25 changes: 2 additions & 23 deletions chrome/browser/permissions/permission_request_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,33 +145,12 @@ bool PermissionRequestImpl::ShouldShowPersistenceToggle() const {

PermissionRequestType PermissionRequestImpl::GetPermissionRequestType()
const {
switch (permission_type_) {
case content::PermissionType::GEOLOCATION:
return PermissionRequestType::PERMISSION_GEOLOCATION;
#if defined(ENABLE_NOTIFICATIONS)
case content::PermissionType::NOTIFICATIONS:
return PermissionRequestType::PERMISSION_NOTIFICATIONS;
#endif
case content::PermissionType::MIDI_SYSEX:
return PermissionRequestType::PERMISSION_MIDI_SYSEX;
case content::PermissionType::PUSH_MESSAGING:
return PermissionRequestType::PERMISSION_PUSH_MESSAGING;
#if defined(OS_CHROMEOS)
case content::PermissionType::PROTECTED_MEDIA_IDENTIFIER:
return PermissionRequestType::PERMISSION_PROTECTED_MEDIA_IDENTIFIER;
#endif
case content::PermissionType::FLASH:
return PermissionRequestType::PERMISSION_FLASH;
default:
NOTREACHED();
return PermissionRequestType::UNKNOWN;
}
return PermissionUtil::GetRequestType(permission_type_);
}

PermissionRequestGestureType PermissionRequestImpl::GetGestureType()
const {
return has_gesture_ ? PermissionRequestGestureType::GESTURE
: PermissionRequestGestureType::NO_GESTURE;
return PermissionUtil::GetGestureType(has_gesture_);
}

ContentSettingsType PermissionRequestImpl::GetContentSettingsType() const {
Expand Down
51 changes: 33 additions & 18 deletions chrome/browser/permissions/permission_uma_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,7 @@ void PermissionUmaUtil::PermissionPromptShown(
permission_gesture_type = requests[0]->GetGestureType();
}

PERMISSION_BUBBLE_TYPE_UMA(kPermissionsPromptShown, permission_prompt_type);
PERMISSION_BUBBLE_GESTURE_TYPE_UMA(
kPermissionsPromptShownGesture, kPermissionsPromptShownNoGesture,
permission_gesture_type, permission_prompt_type);
RecordPermissionPromptShown(permission_prompt_type, permission_gesture_type);

UMA_HISTOGRAM_ENUMERATION(
kPermissionsPromptRequestsPerPrompt,
Expand Down Expand Up @@ -417,17 +414,11 @@ void PermissionUmaUtil::PermissionPromptAccepted(
}

if (all_accepted) {
PERMISSION_BUBBLE_TYPE_UMA(kPermissionsPromptAccepted,
permission_prompt_type);
PERMISSION_BUBBLE_GESTURE_TYPE_UMA(
kPermissionsPromptAcceptedGesture, kPermissionsPromptAcceptedNoGesture,
permission_gesture_type, permission_prompt_type);
RecordPermissionPromptAccepted(permission_prompt_type,
permission_gesture_type);
} else {
PERMISSION_BUBBLE_TYPE_UMA(kPermissionsPromptDenied,
permission_prompt_type);
PERMISSION_BUBBLE_GESTURE_TYPE_UMA(
kPermissionsPromptDeniedGesture, kPermissionsPromptDeniedNoGesture,
permission_gesture_type, permission_prompt_type);
RecordPermissionPromptDenied(permission_prompt_type,
permission_gesture_type);
}
}

Expand All @@ -436,11 +427,35 @@ void PermissionUmaUtil::PermissionPromptDenied(
DCHECK(!requests.empty());
DCHECK(requests.size() == 1);

PERMISSION_BUBBLE_TYPE_UMA(kPermissionsPromptDenied,
requests[0]->GetPermissionRequestType());
RecordPermissionPromptDenied(requests[0]->GetPermissionRequestType(),
requests[0]->GetGestureType());
}

void PermissionUmaUtil::RecordPermissionPromptShown(
PermissionRequestType request_type,
PermissionRequestGestureType gesture_type) {
PERMISSION_BUBBLE_TYPE_UMA(kPermissionsPromptShown, request_type);
PERMISSION_BUBBLE_GESTURE_TYPE_UMA(
kPermissionsPromptDeniedGesture, kPermissionsPromptDeniedNoGesture,
requests[0]->GetGestureType(), requests[0]->GetPermissionRequestType());
kPermissionsPromptShownGesture, kPermissionsPromptShownNoGesture,
gesture_type, request_type);
}

void PermissionUmaUtil::RecordPermissionPromptAccepted(
PermissionRequestType request_type,
PermissionRequestGestureType gesture_type) {
PERMISSION_BUBBLE_TYPE_UMA(kPermissionsPromptAccepted, request_type);
PERMISSION_BUBBLE_GESTURE_TYPE_UMA(kPermissionsPromptAcceptedGesture,
kPermissionsPromptAcceptedNoGesture,
gesture_type, request_type);
}

void PermissionUmaUtil::RecordPermissionPromptDenied(
PermissionRequestType request_type,
PermissionRequestGestureType gesture_type) {
PERMISSION_BUBBLE_TYPE_UMA(kPermissionsPromptDenied, request_type);
PERMISSION_BUBBLE_GESTURE_TYPE_UMA(kPermissionsPromptDeniedGesture,
kPermissionsPromptDeniedNoGesture,
gesture_type, request_type);
}

void PermissionUmaUtil::RecordPermissionPromptPriorCount(
Expand Down
16 changes: 16 additions & 0 deletions chrome/browser/permissions/permission_uma_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "base/logging.h"
#include "base/macros.h"
#include "chrome/browser/permissions/permission_request.h"
#include "chrome/browser/permissions/permission_util.h"

enum class PermissionRequestGestureType;
Expand Down Expand Up @@ -138,6 +139,21 @@ class PermissionUmaUtil {
static void PermissionPromptDenied(
const std::vector<PermissionRequest*>& requests);

// Records the request type and gesture type for a shown, accepted, and denied
// prompt. Defined separately as Android must call this method explicitly
// until the removal of PermissionQueueController is completed.
static void RecordPermissionPromptShown(
PermissionRequestType request_type,
PermissionRequestGestureType gesture_type);

static void RecordPermissionPromptAccepted(
PermissionRequestType request_type,
PermissionRequestGestureType gesture_type);

static void RecordPermissionPromptDenied(
PermissionRequestType request_type,
PermissionRequestGestureType gesture_type);

// A permission prompt was accepted or denied, and the prompt displayed a
// persistence toggle. Records whether the toggle was enabled (persist) or
// disabled (don't persist).
Expand Down
Loading

0 comments on commit 83c36cc

Please sign in to comment.