Add Chip to UI dispositions enum for metrics.
This patch adds Chip (under the value LOCATION_BAR_LEFT_CHIP) to the
PermissionPromptDisposition enum in permission_uma_util. Since it's hard
for PermissionRequestManager to determine whether the chip is being
shown as it currently does, this patch also moves the logic for getting
the disposition to PermissionPrompt.
(cherry picked from commit 87d183ab10cdd5ac9464f121785fd74b8990eb74)
Bug: 1019129
Change-Id: I56563d245fdf84a6e5acadb361695a15ae775e33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438389
Auto-Submit: Bret Sepulveda <[email protected]>
Reviewed-by: Mark Pearson <[email protected]>
Reviewed-by: Balazs Engedy <[email protected]>
Commit-Queue: Bret Sepulveda <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#813107}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2460887
Reviewed-by: Bret Sepulveda <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#137}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/browser/ui/content_settings/content_setting_image_model.cc b/chrome/browser/ui/content_settings/content_setting_image_model.cc
index b40309b..0c23009 100644
--- a/chrome/browser/ui/content_settings/content_setting_image_model.cc
+++ b/chrome/browser/ui/content_settings/content_setting_image_model.cc
@@ -941,16 +941,11 @@
// Show promo the first time a quiet prompt is shown to the user.
set_should_show_promo(
QuietNotificationPermissionUiState::ShouldShowPromo(profile));
- using QuietUiReason = permissions::PermissionRequestManager::QuietUiReason;
- switch (manager->ReasonForUsingQuietUi()) {
- case QuietUiReason::kEnabledInPrefs:
- set_explanatory_string_id(IDS_NOTIFICATIONS_OFF_EXPLANATORY_TEXT);
- break;
- case QuietUiReason::kTriggeredByCrowdDeny:
- case QuietUiReason::kTriggeredDueToAbusiveRequests:
- case QuietUiReason::kTriggeredDueToAbusiveContent:
- set_explanatory_string_id(0);
- break;
+ if (permissions::NotificationPermissionUiSelector::ShouldSuppressAnimation(
+ manager->ReasonForUsingQuietUi())) {
+ set_explanatory_string_id(0);
+ } else {
+ set_explanatory_string_id(IDS_NOTIFICATIONS_OFF_EXPLANATORY_TEXT);
}
return true;
}
diff --git a/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc b/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
index 999223a..7d5afdb7 100644
--- a/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
+++ b/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
@@ -13,8 +13,10 @@
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.h"
+#include "components/permissions/notification_permission_ui_selector.h"
#include "components/permissions/permission_request.h"
#include "components/permissions/permission_request_manager.h"
+#include "components/permissions/permission_uma_util.h"
#include "content/public/browser/web_contents.h"
#include "ui/views/bubble/bubble_frame_view.h"
@@ -111,3 +113,23 @@
return permissions::PermissionPrompt::TabSwitchingBehavior::
kDestroyPromptButKeepRequestPending;
}
+
+permissions::PermissionPromptDisposition
+PermissionPromptImpl::GetPromptDisposition() const {
+ switch (prompt_style_) {
+ case PromptStyle::kBubble:
+ return permissions::PermissionPromptDisposition::ANCHORED_BUBBLE;
+ case PromptStyle::kChip:
+ return permissions::PermissionPromptDisposition::LOCATION_BAR_LEFT_CHIP;
+ case PromptStyle::kQuiet: {
+ permissions::PermissionRequestManager* manager =
+ permissions::PermissionRequestManager::FromWebContents(web_contents_);
+ return permissions::NotificationPermissionUiSelector::
+ ShouldSuppressAnimation(manager->ReasonForUsingQuietUi())
+ ? permissions::PermissionPromptDisposition::
+ LOCATION_BAR_RIGHT_STATIC_ICON
+ : permissions::PermissionPromptDisposition::
+ LOCATION_BAR_RIGHT_ANIMATED_ICON;
+ }
+ }
+}
diff --git a/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h b/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h
index 9b8926c..fdebbb51 100644
--- a/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h
+++ b/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h
@@ -30,6 +30,8 @@
// permissions::PermissionPrompt:
void UpdateAnchorPosition() override;
TabSwitchingBehavior GetTabSwitchingBehavior() override;
+ permissions::PermissionPromptDisposition GetPromptDisposition()
+ const override;
PermissionPromptBubbleView* prompt_bubble_for_testing() {
return prompt_bubble_;
diff --git a/components/permissions/android/permission_prompt_android.cc b/components/permissions/android/permission_prompt_android.cc
index e7e561c..2bacb24 100644
--- a/components/permissions/android/permission_prompt_android.cc
+++ b/components/permissions/android/permission_prompt_android.cc
@@ -10,6 +10,7 @@
#include "components/infobars/core/infobar_manager.h"
#include "components/permissions/android/permission_dialog_delegate.h"
#include "components/permissions/permission_request.h"
+#include "components/permissions/permission_uma_util.h"
#include "components/permissions/permissions_client.h"
#include "components/resources/android/theme_resources.h"
#include "components/strings/grit/components_strings.h"
@@ -64,6 +65,13 @@
return TabSwitchingBehavior::kKeepPromptAlive;
}
+permissions::PermissionPromptDisposition
+PermissionPromptAndroid::GetPromptDisposition() const {
+ return permission_infobar_
+ ? permissions::PermissionPromptDisposition::MINI_INFOBAR
+ : permissions::PermissionPromptDisposition::MODAL_DIALOG;
+}
+
void PermissionPromptAndroid::Closing() {
delegate_->Closing();
}
diff --git a/components/permissions/android/permission_prompt_android.h b/components/permissions/android/permission_prompt_android.h
index e1ee71e..a2069f2 100644
--- a/components/permissions/android/permission_prompt_android.h
+++ b/components/permissions/android/permission_prompt_android.h
@@ -33,6 +33,8 @@
// permissions::PermissionPrompt:
void UpdateAnchorPosition() override;
TabSwitchingBehavior GetTabSwitchingBehavior() override;
+ permissions::PermissionPromptDisposition GetPromptDisposition()
+ const override;
void Closing();
void Accept();
diff --git a/components/permissions/notification_permission_ui_selector.cc b/components/permissions/notification_permission_ui_selector.cc
index 46bdd77..5261001 100644
--- a/components/permissions/notification_permission_ui_selector.cc
+++ b/components/permissions/notification_permission_ui_selector.cc
@@ -6,6 +6,19 @@
namespace permissions {
+// static
+bool NotificationPermissionUiSelector::ShouldSuppressAnimation(
+ QuietUiReason reason) {
+ switch (reason) {
+ case QuietUiReason::kEnabledInPrefs:
+ return false;
+ case QuietUiReason::kTriggeredByCrowdDeny:
+ case QuietUiReason::kTriggeredDueToAbusiveRequests:
+ case QuietUiReason::kTriggeredDueToAbusiveContent:
+ return true;
+ }
+}
+
NotificationPermissionUiSelector::Decision::Decision(
base::Optional<QuietUiReason> quiet_ui_reason,
base::Optional<WarningReason> warning_reason)
diff --git a/components/permissions/notification_permission_ui_selector.h b/components/permissions/notification_permission_ui_selector.h
index 444dec5..91e62fb 100644
--- a/components/permissions/notification_permission_ui_selector.h
+++ b/components/permissions/notification_permission_ui_selector.h
@@ -62,6 +62,10 @@
virtual ~NotificationPermissionUiSelector() {}
+ // Determines whether animations should be suppressed because we're very
+ // confident the user does not want notifications (e.g. they're abusive).
+ static bool ShouldSuppressAnimation(QuietUiReason reason);
+
// Determines the UI to use for the given |request|, and invokes |callback|
// when done, either synchronously or asynchronously. The |callback| is
// guaranteed never to be invoked after |this| goes out of scope. Only one
diff --git a/components/permissions/permission_prompt.h b/components/permissions/permission_prompt.h
index de0be84..f327492d 100644
--- a/components/permissions/permission_prompt.h
+++ b/components/permissions/permission_prompt.h
@@ -17,6 +17,8 @@
}
namespace permissions {
+enum class PermissionPromptDisposition;
+
class PermissionRequest;
// This class is the platform-independent interface through which the permission
@@ -74,6 +76,9 @@
// Get the behavior of this prompt when the user switches away from the
// associated tab.
virtual TabSwitchingBehavior GetTabSwitchingBehavior() = 0;
+
+ // Get the type of prompt UI shown for metrics.
+ virtual PermissionPromptDisposition GetPromptDisposition() const = 0;
};
} // namespace permissions
diff --git a/components/permissions/permission_request_manager.cc b/components/permissions/permission_request_manager.cc
index c61e7fd..9b12bf4 100644
--- a/components/permissions/permission_request_manager.cc
+++ b/components/permissions/permission_request_manager.cc
@@ -732,18 +732,9 @@
PermissionPromptDisposition
PermissionRequestManager::DetermineCurrentRequestUIDispositionForUMA() {
-#if defined(OS_ANDROID)
- return ShouldCurrentRequestUseQuietUI()
- ? PermissionPromptDisposition::MINI_INFOBAR
- : PermissionPromptDisposition::MODAL_DIALOG;
-#else
- return !ShouldCurrentRequestUseQuietUI()
- ? PermissionPromptDisposition::ANCHORED_BUBBLE
- : ReasonForUsingQuietUi() == QuietUiReason::kTriggeredByCrowdDeny
- ? PermissionPromptDisposition::LOCATION_BAR_RIGHT_STATIC_ICON
- : PermissionPromptDisposition::
- LOCATION_BAR_RIGHT_ANIMATED_ICON;
-#endif
+ if (view_)
+ return view_->GetPromptDisposition();
+ return PermissionPromptDisposition::NONE_VISIBLE;
}
void PermissionRequestManager::LogWarningToConsole(const char* message) {
diff --git a/components/permissions/permission_uma_util.cc b/components/permissions/permission_uma_util.cc
index 45cc8445..422f18b 100644
--- a/components/permissions/permission_uma_util.cc
+++ b/components/permissions/permission_uma_util.cc
@@ -161,6 +161,8 @@
switch (ui_disposition) {
case PermissionPromptDisposition::ANCHORED_BUBBLE:
return "AnchoredBubble";
+ case PermissionPromptDisposition::LOCATION_BAR_LEFT_CHIP:
+ return "LocationBarLeftChip";
case PermissionPromptDisposition::LOCATION_BAR_RIGHT_ANIMATED_ICON:
return "LocationBarRightAnimatedIcon";
case PermissionPromptDisposition::LOCATION_BAR_RIGHT_STATIC_ICON:
@@ -169,6 +171,8 @@
return "MiniInfobar";
case PermissionPromptDisposition::MODAL_DIALOG:
return "ModalDialog";
+ case PermissionPromptDisposition::NONE_VISIBLE:
+ return "NoneVisible";
case PermissionPromptDisposition::NOT_APPLICABLE:
return "NotApplicable";
}
diff --git a/components/permissions/permission_uma_util.h b/components/permissions/permission_uma_util.h
index a8a4040e..964033b 100644
--- a/components/permissions/permission_uma_util.h
+++ b/components/permissions/permission_uma_util.h
@@ -69,7 +69,7 @@
// Enum used in UKMs and UMAs, do not re-order or change values. Deprecated
// items should only be commented out. New items should be added at the end,
// and the "PermissionPromptDisposition" histogram suffix needs to be updated to
-// match (tools/metrics/histograms/histograms.xml).
+// match (tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml).
enum class PermissionPromptDisposition {
// Not all permission actions will have an associated permission prompt (e.g.
// changing permission via the settings page).
@@ -92,6 +92,14 @@
// Only used on Android, an initially-collapsed infobar at the bottom of the
// page.
MINI_INFOBAR = 5,
+
+ // Only used on desktop, a chip on the left-hand side of the location bar that
+ // shows a bubble when clicked.
+ LOCATION_BAR_LEFT_CHIP = 6,
+
+ // There was no UI being shown. This is usually because the user closed an
+ // inactive tab that had a pending permission request.
+ NONE_VISIBLE = 7,
};
enum class AdaptiveTriggers {
diff --git a/components/permissions/test/mock_permission_prompt.cc b/components/permissions/test/mock_permission_prompt.cc
index b87746ea..2af4bfbc 100644
--- a/components/permissions/test/mock_permission_prompt.cc
+++ b/components/permissions/test/mock_permission_prompt.cc
@@ -7,6 +7,7 @@
#include "base/bind.h"
#include "base/run_loop.h"
#include "build/build_config.h"
+#include "components/permissions/permission_uma_util.h"
#include "components/permissions/test/mock_permission_prompt_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -32,6 +33,14 @@
#endif
}
+PermissionPromptDisposition MockPermissionPrompt::GetPromptDisposition() const {
+#if defined(OS_ANDROID)
+ return PermissionPromptDisposition::MODAL_DIALOG;
+#else
+ return PermissionPromptDisposition::ANCHORED_BUBBLE;
+#endif
+}
+
MockPermissionPrompt::MockPermissionPrompt(MockPermissionPromptFactory* factory,
Delegate* delegate)
: factory_(factory), delegate_(delegate) {
diff --git a/components/permissions/test/mock_permission_prompt.h b/components/permissions/test/mock_permission_prompt.h
index 2ae804b..220dd9a4 100644
--- a/components/permissions/test/mock_permission_prompt.h
+++ b/components/permissions/test/mock_permission_prompt.h
@@ -21,6 +21,7 @@
// PermissionPrompt:
void UpdateAnchorPosition() override;
TabSwitchingBehavior GetTabSwitchingBehavior() override;
+ PermissionPromptDisposition GetPromptDisposition() const override;
bool IsVisible();
diff --git a/tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml b/tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml
index 35e8e50..381adbe 100644
--- a/tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml
+++ b/tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml
@@ -13687,6 +13687,9 @@
<histogram_suffixes name="PermissionPromptDisposition" separator=".">
<suffix name="AnchoredBubble"
label="A bubble under the site settings padlock"/>
+ <suffix name="LocationBarLeftChip"
+ label="A chip on the left-hand side of the location bar that shows a
+ bubble when clicked"/>
<suffix name="LocationBarRightAnimatedIcon"
label="An animated indicator on the right-hand side of the location bar"/>
<suffix name="LocationBarRightStaticIcon"
@@ -13694,7 +13697,8 @@
<suffix name="MiniInfobar"
label="An initially-collapsed infobar at the bottom of the page"/>
<suffix name="ModalDialog" label="A modal dialog"/>
- <suffix name="NotApplicable" label="No permission prompt"/>
+ <suffix name="NoneVisible" label="There was no UI being shown"/>
+ <suffix name="NotApplicable" label="No permission prompt at all"/>
<affected-histogram name="Permissions.Action.WithDisposition"/>
</histogram_suffixes>
