Add support for multiple permission UI selectors.

This will help support the future new selector for CPSS:
go/permissions-suggestions-client-doc

Also split the existing selector into a crowd deny and a pref selector.

Bug: 1138595
Change-Id: I460d979cc6746586a8c25a85c549d487a52908d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485213
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825837}

chrome/browser/BUILD.gn

@@ -1131,6 +1131,8 @@ static_library("browser") {
     "permissions/permission_decision_auto_blocker_factory.h",
     "permissions/permission_manager_factory.cc",
     "permissions/permission_manager_factory.h",
+    "permissions/pref_notification_permission_ui_selector.cc",
+    "permissions/pref_notification_permission_ui_selector.h",
     "permissions/quiet_notification_permission_ui_config.cc",
     "permissions/quiet_notification_permission_ui_config.h",
     "permissions/quiet_notification_permission_ui_state.cc",

chrome/browser/permissions/chrome_permissions_client.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/permissions/chrome_permissions_client.h"
 
+#include <vector>
+
 #include "base/feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/bluetooth/bluetooth_chooser_context.h"
@@ -18,6 +20,7 @@
 #include "chrome/browser/permissions/contextual_notification_permission_ui_selector.h"
 #include "chrome/browser/permissions/permission_decision_auto_blocker_factory.h"
 #include "chrome/browser/permissions/permission_manager_factory.h"
+#include "chrome/browser/permissions/pref_notification_permission_ui_selector.h"
 #include "chrome/browser/permissions/quiet_notification_permission_ui_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
@@ -195,11 +198,16 @@ ChromePermissionsClient::GetOverrideIconId(ContentSettingsType type) {
   return PermissionsClient::GetOverrideIconId(type);
 }
 
-std::unique_ptr<permissions::NotificationPermissionUiSelector>
-ChromePermissionsClient::CreateNotificationPermissionUiSelector(
+std::vector<std::unique_ptr<permissions::NotificationPermissionUiSelector>>
+ChromePermissionsClient::CreateNotificationPermissionUiSelectors(
     content::BrowserContext* browser_context) {
-  return std::make_unique<ContextualNotificationPermissionUiSelector>(
-      Profile::FromBrowserContext(browser_context));
+  std::vector<std::unique_ptr<permissions::NotificationPermissionUiSelector>>
+      selectors;
+  selectors.emplace_back(
+      std::make_unique<ContextualNotificationPermissionUiSelector>());
+  selectors.emplace_back(std::make_unique<PrefNotificationPermissionUiSelector>(
+      Profile::FromBrowserContext(browser_context)));
+  return selectors;
 }
 
 void ChromePermissionsClient::OnPromptResolved(

chrome/browser/permissions/chrome_permissions_client.h

@@ -42,8 +42,8 @@ class ChromePermissionsClient : public permissions::PermissionsClient {
                       GetUkmSourceIdCallback callback) override;
   permissions::PermissionRequest::IconId GetOverrideIconId(
       ContentSettingsType type) override;
-  std::unique_ptr<permissions::NotificationPermissionUiSelector>
-  CreateNotificationPermissionUiSelector(
+  std::vector<std::unique_ptr<permissions::NotificationPermissionUiSelector>>
+  CreateNotificationPermissionUiSelectors(
       content::BrowserContext* browser_context) override;
   void OnPromptResolved(content::BrowserContext* browser_context,
                         permissions::PermissionRequestType request_type,

chrome/browser/permissions/contextual_notification_permission_ui_selector.cc

@@ -18,12 +18,9 @@
 #include "chrome/browser/permissions/crowd_deny_preload_data.h"
 #include "chrome/browser/permissions/quiet_notification_permission_ui_config.h"
 #include "chrome/browser/permissions/quiet_notification_permission_ui_state.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/chrome_features.h"
-#include "components/content_settings/core/common/pref_names.h"
 #include "components/permissions/permission_request.h"
-#include "components/prefs/pref_service.h"
 #include "components/safe_browsing/core/db/database_manager.h"
 
 namespace {
@@ -134,8 +131,7 @@ bool ShouldHoldBackQuietUI(QuietUiReason quiet_ui_reason) {
 }  // namespace
 
 ContextualNotificationPermissionUiSelector::
-    ContextualNotificationPermissionUiSelector(Profile* profile)
-    : profile_(profile) {}
+    ContextualNotificationPermissionUiSelector() = default;
 
 void ContextualNotificationPermissionUiSelector::SelectUiToUse(
     permissions::PermissionRequest* request,
@@ -171,7 +167,7 @@ void ContextualNotificationPermissionUiSelector::EvaluatePerSiteTriggers(
   // If the PreloadData suggests this is an unacceptable site, ping Safe
   // Browsing to verify; but do not ping if it is not warranted.
   if (!decision || (!decision->quiet_ui_reason && !decision->warning_reason)) {
-    OnPerSiteTriggersEvaluated(Decision::UseNormalUiAndShowNoWarning());
+    Notify(Decision::UseNormalUiAndShowNoWarning());
     return;
   }
 
@@ -198,31 +194,18 @@ void ContextualNotificationPermissionUiSelector::OnSafeBrowsingVerdictReceived(
 
   switch (verdict) {
     case CrowdDenySafeBrowsingRequest::Verdict::kAcceptable:
-      OnPerSiteTriggersEvaluated(Decision::UseNormalUiAndShowNoWarning());
+      Notify(Decision::UseNormalUiAndShowNoWarning());
       break;
     case CrowdDenySafeBrowsingRequest::Verdict::kUnacceptable:
       if (candidate_decision.quiet_ui_reason &&
           ShouldHoldBackQuietUI(*(candidate_decision.quiet_ui_reason))) {
         candidate_decision.quiet_ui_reason.reset();
       }
-      OnPerSiteTriggersEvaluated(candidate_decision);
+      Notify(candidate_decision);
       break;
   }
 }
 
-void ContextualNotificationPermissionUiSelector::OnPerSiteTriggersEvaluated(
-    Decision decision) {
-  // Still show the quiet UI if it is enabled for all sites, even if per-site
-  // conditions did not trigger showing the quiet UI on this origin.
-  if (!decision.quiet_ui_reason &&
-      profile_->GetPrefs()->GetBoolean(
-          prefs::kEnableQuietNotificationPermissionUi)) {
-    decision.quiet_ui_reason = QuietUiReason::kEnabledInPrefs;
-  }
-
-  Notify(decision);
-}
-
 void ContextualNotificationPermissionUiSelector::Notify(
     const Decision& decision) {
   std::move(callback_).Run(decision);

chrome/browser/permissions/contextual_notification_permission_ui_selector.h

@@ -10,8 +10,6 @@
 #include "chrome/browser/permissions/crowd_deny_safe_browsing_request.h"
 #include "components/permissions/notification_permission_ui_selector.h"
 
-class Profile;
-
 namespace permissions {
 class PermissionRequest;
 }
@@ -20,22 +18,19 @@ namespace url {
 class Origin;
 }
 
-// Determines if the quiet prompt UI should be used to display a notification
-// permission request on a given site. This is the case when:
-//  1) the quiet UI is enabled in prefs for all sites, either directly by the
-//     user in settings, or by the AdaptiveQuietNotificationPermissionUiEnabler.
-//  2) the quiet UI is triggered by crowd deny, either through:
-//     a) CrowdDenyPreloadData, that is, the component updater, or
+// Determines if crowd deny or abusive blocklists prescribe that the quiet UI
+// should be used to display a notification permission request on a given site.
+// This is the case when the  both of the below sources classify the origin as
+// spammy or abusive:
+//   a) CrowdDenyPreloadData, that is, the component updater, and
 //   b) CrowdDenySafeBrowsingRequest, that is, on-demand Safe Browsing pings.
-// If both (1) and (2) are fulfilled, the crowd-deny UI is shown.
 //
 // Each instance of this class is long-lived and can support multiple requests,
 // but only one at a time.
 class ContextualNotificationPermissionUiSelector
     : public permissions::NotificationPermissionUiSelector {
  public:
-  // Constructs an instance in the context of the given |profile|.
-  explicit ContextualNotificationPermissionUiSelector(Profile* profile);
+  ContextualNotificationPermissionUiSelector();
   ~ContextualNotificationPermissionUiSelector() override;
 
   // NotificationPermissionUiSelector:
@@ -54,11 +49,8 @@ class ContextualNotificationPermissionUiSelector
   void OnSafeBrowsingVerdictReceived(
       Decision candidate_decision,
       CrowdDenySafeBrowsingRequest::Verdict verdict);
-  void OnPerSiteTriggersEvaluated(Decision decision);
   void Notify(const Decision& decision);
 
-  Profile* profile_;
-
   base::Optional<CrowdDenySafeBrowsingRequest> safe_browsing_request_;
   DecisionMadeCallback callback_;
 };

chrome/browser/permissions/contextual_notification_permission_ui_selector_unittest.cc

@@ -77,8 +77,7 @@ constexpr const char* kAllTestingOrigins[] = {
 class ContextualNotificationPermissionUiSelectorTest : public testing::Test {
  public:
   ContextualNotificationPermissionUiSelectorTest()
-      : testing_profile_(std::make_unique<TestingProfile>()),
-        contextual_selector_(testing_profile_.get()) {}
+      : testing_profile_(std::make_unique<TestingProfile>()) {}
   ~ContextualNotificationPermissionUiSelectorTest() override = default;
 
  protected:
@@ -230,6 +229,8 @@ class ContextualNotificationPermissionUiSelectorTest : public testing::Test {
 // With all the field trials enabled, test all combinations of:
 //   (a) quiet UI being enabled/disabled in prefs, and
 //   (b) positive/negative Safe Browsing verdicts.
+// The ContextualNotificationPermissionUiSelector should only take into account
+// the Safe Browsing verdict and ignore the user pref.
 TEST_F(ContextualNotificationPermissionUiSelectorTest,
        PrefAndSafeBrowsingCombinations) {
   using Config = QuietNotificationPermissionUiConfig;
@@ -245,33 +246,23 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
 
   LoadTestPreloadData();
 
-  {
-    SetQuietUiEnabledInPrefs(false);
+  for (const bool quiet_ui_enabled_in_prefs : {false, true}) {
+    SetQuietUiEnabledInPrefs(quiet_ui_enabled_in_prefs);
     ClearSafeBrowsingBlocklist();
 
-    SCOPED_TRACE("Quiet UI disabled in prefs, Safe Browsing verdicts negative");
-    for (const auto* origin_string : kAllTestingOrigins) {
-      SCOPED_TRACE(origin_string);
-      QueryAndExpectDecisionForUrl(GURL(origin_string), Decision::UseNormalUi(),
-                                   Decision::ShowNoWarning());
-    }
-  }
+    SCOPED_TRACE(quiet_ui_enabled_in_prefs ? "Quiet UI enabled in prefs"
+                                           : "Quiet UI disabled in prefs");
+    SCOPED_TRACE("Safe Browsing verdicts negative");
 
-  {
-    SetQuietUiEnabledInPrefs(true);
-    ClearSafeBrowsingBlocklist();
-
-    SCOPED_TRACE("Quiet UI enabled in prefs, Safe Browsing verdicts negative");
     for (const auto* origin_string : kAllTestingOrigins) {
       SCOPED_TRACE(origin_string);
-      QueryAndExpectDecisionForUrl(GURL(origin_string),
-                                   QuietUiReason::kEnabledInPrefs,
+      QueryAndExpectDecisionForUrl(GURL(origin_string), Decision::UseNormalUi(),
                                    Decision::ShowNoWarning());
     }
   }
 
-  {
-    SetQuietUiEnabledInPrefs(false);
+  for (const bool quiet_ui_enabled_in_prefs : {false, true}) {
+    SetQuietUiEnabledInPrefs(quiet_ui_enabled_in_prefs);
     LoadTestSafeBrowsingBlocklist();
 
     const struct {
@@ -298,44 +289,10 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
          WarningReason::kAbusiveContent},
     };
 
-    SCOPED_TRACE("Quiet UI disabled in prefs, Safe Browsing verdicts positive");
-    for (const auto& test : kTestCases) {
-      SCOPED_TRACE(test.origin_string);
-      QueryAndExpectDecisionForUrl(GURL(test.origin_string),
-                                   test.expected_ui_reason,
-                                   test.expected_warning_reason);
-    }
-  }
+    SCOPED_TRACE(quiet_ui_enabled_in_prefs ? "Quiet UI enabled in prefs"
+                                           : "Quiet UI disabled in prefs");
+    SCOPED_TRACE("Safe Browsing verdicts positive");
 
-  {
-    SetQuietUiEnabledInPrefs(true);
-    LoadTestSafeBrowsingBlocklist();
-
-    const struct {
-      const char* origin_string;
-      base::Optional<QuietUiReason> expected_ui_reason =
-          Decision::UseNormalUi();
-      base::Optional<WarningReason> expected_warning_reason =
-          Decision::ShowNoWarning();
-    } kTestCases[] = {
-        {kTestOriginNoData, QuietUiReason::kEnabledInPrefs},
-        {kTestOriginUnknown, QuietUiReason::kEnabledInPrefs},
-        {kTestOriginAcceptable, QuietUiReason::kEnabledInPrefs},
-        {kTestOriginSpammy, QuietUiReason::kTriggeredByCrowdDeny},
-        {kTestOriginSpammyWarn, QuietUiReason::kEnabledInPrefs},
-        {kTestOriginAbusivePrompts,
-         QuietUiReason::kTriggeredDueToAbusiveRequests},
-        {kTestOriginSubDomainOfAbusivePrompts,
-         QuietUiReason::kTriggeredDueToAbusiveRequests},
-        {kTestOriginAbusivePromptsWarn, QuietUiReason::kEnabledInPrefs,
-         WarningReason::kAbusiveRequests},
-        {kTestOriginAbusiveContent,
-         QuietUiReason::kTriggeredDueToAbusiveContent},
-        {kTestOriginAbusiveContentWarn, QuietUiReason::kEnabledInPrefs,
-         WarningReason::kAbusiveContent},
-    };
-
-    SCOPED_TRACE("Quiet UI enabled in prefs, Safe Browsing verdicts positive");
     for (const auto& test : kTestCases) {
       SCOPED_TRACE(test.origin_string);
       QueryAndExpectDecisionForUrl(GURL(test.origin_string),
@@ -349,7 +306,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest, FeatureDisabled) {
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndDisableFeature(features::kQuietNotificationPrompts);
 
-  SetQuietUiEnabledInPrefs(true);
   LoadTestPreloadData();
   LoadTestSafeBrowsingBlocklist();
 
@@ -360,7 +316,7 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest, FeatureDisabled) {
   }
 }
 
-// The feature is enabled but no adaptive triggers are enabled.
+// The feature is enabled but no triggers are enabled.
 TEST_F(ContextualNotificationPermissionUiSelectorTest, AllTriggersDisabled) {
   using Config = QuietNotificationPermissionUiConfig;
   base::test::ScopedFeatureList feature_list;
@@ -374,8 +330,7 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest, AllTriggersDisabled) {
 
   for (const auto* origin_string : kAllTestingOrigins) {
     SCOPED_TRACE(origin_string);
-    QueryAndExpectDecisionForUrl(GURL(origin_string),
-                                 QuietUiReason::kEnabledInPrefs,
+    QueryAndExpectDecisionForUrl(GURL(origin_string), Decision::UseNormalUi(),
                                  Decision::ShowNoWarning());
   }
 }
@@ -389,7 +344,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest, OnlyCrowdDenyEnabled) {
       {{Config::kEnableAdaptiveActivation, "true"},
        {Config::kEnableCrowdDenyTriggering, "true"}});
 
-  SetQuietUiEnabledInPrefs(false);
   LoadTestPreloadData();
   LoadTestSafeBrowsingBlocklist();
 
@@ -426,7 +380,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
       {{Config::kEnableAdaptiveActivation, "true"},
        {Config::kEnableAbusiveContentTriggeredRequestBlocking, "true"}});
 
-  SetQuietUiEnabledInPrefs(false);
   LoadTestPreloadData();
   LoadTestSafeBrowsingBlocklist();
 
@@ -463,7 +416,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
       {{Config::kEnableAdaptiveActivation, "true"},
        {Config::kEnableAbusiveContentTriggeredRequestWarning, "true"}});
 
-  SetQuietUiEnabledInPrefs(false);
   LoadTestPreloadData();
   LoadTestSafeBrowsingBlocklist();
 
@@ -501,7 +453,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
       {{Config::kEnableAdaptiveActivation, "true"},
        {Config::kEnableAbusiveRequestBlocking, "true"}});
 
-  SetQuietUiEnabledInPrefs(false);
   LoadTestPreloadData();
   LoadTestSafeBrowsingBlocklist();
 
@@ -540,7 +491,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
       {{Config::kEnableAdaptiveActivation, "true"},
        {Config::kEnableAbusiveRequestWarning, "true"}});
 
-  SetQuietUiEnabledInPrefs(false);
   LoadTestPreloadData();
   LoadTestSafeBrowsingBlocklist();
 
@@ -572,17 +522,13 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
        CrowdDenyHoldbackChance) {
   const struct {
     std::string holdback_chance;
-    bool enabled_in_prefs;
     base::Optional<QuietUiReason> expected_ui_reason;
     bool expected_histogram_bucket;
   } kTestCases[] = {
       // 100% chance to holdback, the UI used should be the normal UI.
-      {"1.0", false, Decision::UseNormalUi(), true},
+      {"1.0", Decision::UseNormalUi(), true},
       // 0% chance to holdback, the UI used should be the quiet UI.
-      {"0.0", false, QuietUiReason::kTriggeredByCrowdDeny},
-      // 100% chance to holdback but the quiet UI is enabled by the user in
-      // prefs, the UI used should be the quiet UI.
-      {"1.0", true, QuietUiReason::kEnabledInPrefs, true},
+      {"0.0", QuietUiReason::kTriggeredByCrowdDeny},
   };
 
   LoadTestPreloadData();
@@ -590,7 +536,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
 
   for (const auto& test : kTestCases) {
     SCOPED_TRACE(test.holdback_chance);
-    SCOPED_TRACE(test.enabled_in_prefs);
 
     using Config = QuietNotificationPermissionUiConfig;
     base::test::ScopedFeatureList feature_list;
@@ -604,8 +549,6 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
          {Config::kEnableCrowdDenyTriggering, "true"},
          {Config::kCrowdDenyHoldBackChance, test.holdback_chance}});
 
-    SetQuietUiEnabledInPrefs(test.enabled_in_prefs);
-
     base::HistogramTester histograms;
     QueryAndExpectDecisionForUrl(GURL(kTestOriginSpammy),
                                  test.expected_ui_reason,
@@ -616,18 +559,14 @@ TEST_F(ContextualNotificationPermissionUiSelectorTest,
                                  QuietUiReason::kTriggeredDueToAbusiveRequests,
                                  Decision::ShowNoWarning());
     QueryAndExpectDecisionForUrl(GURL(kTestOriginAbusivePromptsWarn),
-                                 test.enabled_in_prefs
-                                     ? QuietUiReason::kEnabledInPrefs
-                                     : Decision::UseNormalUi(),
+                                 Decision::UseNormalUi(),
                                  WarningReason::kAbusiveRequests);
 
     QueryAndExpectDecisionForUrl(GURL(kTestOriginAbusiveContent),
                                  QuietUiReason::kTriggeredDueToAbusiveContent,
                                  Decision::ShowNoWarning());
     QueryAndExpectDecisionForUrl(GURL(kTestOriginAbusiveContentWarn),
-                                 test.enabled_in_prefs
-                                     ? QuietUiReason::kEnabledInPrefs
-                                     : Decision::UseNormalUi(),
+                                 Decision::UseNormalUi(),
                                  WarningReason::kAbusiveContent);
 
     auto expected_bucket = static_cast<base::HistogramBase::Sample>(

chrome/browser/permissions/pref_notification_permission_ui_selector.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/permissions/pref_notification_permission_ui_selector.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_features.h"
+#include "components/content_settings/core/common/pref_names.h"
+#include "components/prefs/pref_service.h"
+
+PrefNotificationPermissionUiSelector::PrefNotificationPermissionUiSelector(
+    Profile* profile)
+    : profile_(profile) {}
+
+PrefNotificationPermissionUiSelector::~PrefNotificationPermissionUiSelector() =
+    default;
+
+void PrefNotificationPermissionUiSelector::SelectUiToUse(
+    permissions::PermissionRequest* request,
+    DecisionMadeCallback callback) {
+  if (base::FeatureList::IsEnabled(features::kQuietNotificationPrompts) &&
+      profile_->GetPrefs()->GetBoolean(
+          prefs::kEnableQuietNotificationPermissionUi)) {
+    std::move(callback).Run(
+        Decision(QuietUiReason::kEnabledInPrefs, Decision::ShowNoWarning()));
+    return;
+  }
+
+  std::move(callback).Run(Decision::UseNormalUiAndShowNoWarning());
+}
+
+void PrefNotificationPermissionUiSelector::Cancel() {}

chrome/browser/permissions/pref_notification_permission_ui_selector.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PERMISSIONS_PREF_NOTIFICATION_PERMISSION_UI_SELECTOR_H_
+#define CHROME_BROWSER_PERMISSIONS_PREF_NOTIFICATION_PERMISSION_UI_SELECTOR_H_
+
+#include "components/permissions/notification_permission_ui_selector.h"
+
+class Profile;
+
+namespace permissions {
+class PermissionRequest;
+}
+
+// Determines if the quiet prompt UI should be used to display a notification
+// permission request on a given site according to user prefs. The quiet UI can
+// be enabled in prefs for all sites, either directly by the user in settings,
+// or by the AdaptiveQuietNotificationPermissionUiEnabler.
+//
+// Each instance of this class is long-lived and can support multiple requests.
+class PrefNotificationPermissionUiSelector
+    : public permissions::NotificationPermissionUiSelector {
+ public:
+  // Constructs an instance in the context of the given |profile|.
+  explicit PrefNotificationPermissionUiSelector(Profile* profile);
+  ~PrefNotificationPermissionUiSelector() override;
+
+  // Disallow copying and assigning.
+  PrefNotificationPermissionUiSelector(
+      const PrefNotificationPermissionUiSelector&) = delete;
+  PrefNotificationPermissionUiSelector& operator=(
+      const PrefNotificationPermissionUiSelector&) = delete;
+
+  // NotificationPermissionUiSelector:
+  void SelectUiToUse(permissions::PermissionRequest* request,
+                     DecisionMadeCallback callback) override;
+
+  void Cancel() override;
+
+ private:
+  Profile* profile_;
+};
+
+#endif  // CHROME_BROWSER_PERMISSIONS_PREF_NOTIFICATION_PERMISSION_UI_SELECTOR_H_

chrome/browser/permissions/pref_notification_permission_ui_selector_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/permissions/pref_notification_permission_ui_selector.h"
+
+#include "base/optional.h"
+#include "base/run_loop.h"
+#include "base/test/mock_callback.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/content_settings/core/common/pref_names.h"
+#include "components/permissions/test/mock_permission_request.h"
+#include "components/prefs/testing_pref_service.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+using QuietUiReason = PrefNotificationPermissionUiSelector::QuietUiReason;
+using WarningReason = PrefNotificationPermissionUiSelector::WarningReason;
+using Decision = PrefNotificationPermissionUiSelector::Decision;
+
+ACTION_P(QuitMessageLoop, loop) {
+  loop->Quit();
+}
+
+}  // namespace
+
+class PrefNotificationPermissionUiSelectorTest : public testing::Test {
+ public:
+  PrefNotificationPermissionUiSelectorTest()
+      : testing_profile_(std::make_unique<TestingProfile>()),
+        pref_selector_(testing_profile_.get()) {}
+
+  PrefNotificationPermissionUiSelector* pref_selector() {
+    return &pref_selector_;
+  }
+
+  TestingProfile* profile() { return testing_profile_.get(); }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+
+  std::unique_ptr<TestingProfile> testing_profile_;
+
+  PrefNotificationPermissionUiSelector pref_selector_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrefNotificationPermissionUiSelectorTest);
+};
+
+TEST_F(PrefNotificationPermissionUiSelectorTest, FeatureAndPrefCombinations) {
+  const struct {
+    bool feature_enabled;
+    bool quiet_ui_enabled_in_prefs;
+    base::Optional<QuietUiReason> expected_reason;
+  } kTests[] = {
+      {true, false, Decision::UseNormalUi()},
+      {true, true, QuietUiReason::kEnabledInPrefs},
+      {false, true, Decision::UseNormalUi()},
+      {false, false, Decision::UseNormalUi()},
+  };
+
+  for (const auto& test_case : kTests) {
+    SCOPED_TRACE(base::StringPrintf("feature: %d, pref: %d",
+                                    test_case.feature_enabled,
+                                    test_case.quiet_ui_enabled_in_prefs));
+
+    // Init feature and pref settings.
+    base::test::ScopedFeatureList feature_list;
+    if (test_case.feature_enabled)
+      feature_list.InitAndEnableFeature(features::kQuietNotificationPrompts);
+    else
+      feature_list.InitAndDisableFeature(features::kQuietNotificationPrompts);
+    profile()->GetPrefs()->SetBoolean(
+        prefs::kEnableQuietNotificationPermissionUi,
+        test_case.quiet_ui_enabled_in_prefs);
+
+    // Setup and prepare for the request.
+    base::RunLoop callback_loop;
+    base::MockCallback<
+        PrefNotificationPermissionUiSelector::DecisionMadeCallback>
+        mock_callback;
+    Decision actual_decison(base::nullopt, base::nullopt);
+
+    // Make a request and wait for the callback.
+    EXPECT_CALL(mock_callback, Run)
+        .WillRepeatedly(DoAll(testing::SaveArg<0>(&actual_decison),
+                              QuitMessageLoop(&callback_loop)));
+
+    permissions::MockPermissionRequest mock_request(
+        std::string(),
+        permissions::PermissionRequestType::PERMISSION_NOTIFICATIONS,
+        GURL("http://example.com"));
+    pref_selector()->SelectUiToUse(&mock_request, mock_callback.Get());
+    callback_loop.Run();
+    testing::Mock::VerifyAndClearExpectations(&mock_callback);
+
+    // Check expectations.
+    EXPECT_EQ(test_case.expected_reason, actual_decison.quiet_ui_reason);
+    EXPECT_EQ(Decision::ShowNoWarning(), actual_decison.warning_reason);
+  }
+}

chrome/test/BUILD.gn

@@ -3565,6 +3565,7 @@ test("unit_tests") {
     "../browser/permissions/crowd_deny_preload_data_unittest.cc",
     "../browser/permissions/crowd_deny_safe_browsing_request_unittest.cc",
     "../browser/permissions/permission_context_base_feature_policy_unittest.cc",
+    "../browser/permissions/pref_notification_permission_ui_selector_unittest.cc",
     "../browser/persisted_state_db/persisted_state_db_factory_unittest.cc",
     "../browser/persisted_state_db/persisted_state_db_unittest.cc",
     "../browser/plugins/pdf_iframe_navigation_throttle_unittest.cc",

components/permissions/notification_permission_ui_selector.h

@@ -75,7 +75,8 @@ class NotificationPermissionUiSelector {
 
   // Cancel the pending request, if any. After this, the |callback| is
   // guaranteed not to be invoked anymore, and another call to SelectUiToUse()
-  // can be issued.
+  // can be issued. Can be called when there is no pending request which will
+  // simply be a no-op.
   virtual void Cancel() {}
 };
 

components/permissions/permission_request_manager.cc

@@ -426,8 +426,8 @@ PermissionRequestManager::PermissionRequestManager(
       tab_is_hidden_(web_contents->GetVisibility() ==
                      content::Visibility::HIDDEN),
       auto_response_for_test_(NONE),
-      notification_permission_ui_selector_(
-          PermissionsClient::Get()->CreateNotificationPermissionUiSelector(
+      notification_permission_ui_selectors_(
+          PermissionsClient::Get()->CreateNotificationPermissionUiSelectors(
               web_contents->GetBrowserContext())) {}
 
 void PermissionRequestManager::ScheduleShowBubble() {
@@ -474,14 +474,23 @@ void PermissionRequestManager::DequeueRequestIfNeeded() {
     }
   }
 
-  if (notification_permission_ui_selector_ &&
+  if (!notification_permission_ui_selectors_.empty() &&
       requests_.front()->GetPermissionRequestType() ==
           PermissionRequestType::PERMISSION_NOTIFICATIONS) {
-    notification_permission_ui_selector_->SelectUiToUse(
+    DCHECK(!current_request_ui_to_use_.has_value());
+    // Initialize the selector decisions vector.
+    DCHECK(selector_decisions_.empty());
+    selector_decisions_.resize(notification_permission_ui_selectors_.size());
+
+    for (size_t selector_index = 0;
+         selector_index < notification_permission_ui_selectors_.size();
+         ++selector_index) {
+      notification_permission_ui_selectors_[selector_index]->SelectUiToUse(
           requests_.front(),
           base::BindOnce(
-            &PermissionRequestManager::OnSelectedUiToUseForNotifications,
-            weak_factory_.GetWeakPtr()));
+              &PermissionRequestManager::OnNotificationPermissionUiSelectorDone,
+              weak_factory_.GetWeakPtr(), selector_index));
+    }
   } else {
     current_request_ui_to_use_ =
         UiDecision(UiDecision::UseNormalUi(), UiDecision::ShowNoWarning());
@@ -532,17 +541,6 @@ void PermissionRequestManager::ShowBubble() {
       base::RecordAction(base::UserMetricsAction(
           "Notifications.Quiet.PermissionRequestShown"));
     }
-
-    if (current_request_ui_to_use_->warning_reason) {
-      switch (*(current_request_ui_to_use_->warning_reason)) {
-        case WarningReason::kAbusiveRequests:
-          LogWarningToConsole(kAbusiveNotificationRequestsWarningMessage);
-          break;
-        case WarningReason::kAbusiveContent:
-          LogWarningToConsole(kAbusiveNotificationContentWarningMessage);
-          break;
-      }
-    }
   }
   current_request_already_displayed_ = true;
   NotifyBubbleAdded();
@@ -614,11 +612,12 @@ void PermissionRequestManager::FinalizeBubble(
   }
   requests_.clear();
 
-  if (notification_permission_ui_selector_)
-    notification_permission_ui_selector_->Cancel();
+  for (const auto& selector : notification_permission_ui_selectors_)
+    selector->Cancel();
 
   current_request_already_displayed_ = false;
   current_request_ui_to_use_.reset();
+  selector_decisions_.clear();
 
   if (view_)
     DeleteBubble();
@@ -716,7 +715,7 @@ bool PermissionRequestManager::ShouldCurrentRequestUseQuietUI() const {
     return false;
 
   // ContentSettingImageModel might call into this method if the user switches
-  // between tabs while the |notification_permission_ui_selector_| is pending.
+  // between tabs while the |notification_permission_ui_selectors_| are pending.
   return current_request_ui_to_use_ &&
          current_request_ui_to_use_->quiet_ui_reason;
 }
@@ -740,10 +739,49 @@ void PermissionRequestManager::NotifyBubbleRemoved() {
     observer.OnBubbleRemoved();
 }
 
-void PermissionRequestManager::OnSelectedUiToUseForNotifications(
+void PermissionRequestManager::OnNotificationPermissionUiSelectorDone(
+    size_t selector_index,
     const UiDecision& decision) {
-  current_request_ui_to_use_ = decision;
+  if (decision.warning_reason) {
+    switch (*(decision.warning_reason)) {
+      case WarningReason::kAbusiveRequests:
+        LogWarningToConsole(kAbusiveNotificationRequestsWarningMessage);
+        break;
+      case WarningReason::kAbusiveContent:
+        LogWarningToConsole(kAbusiveNotificationContentWarningMessage);
+        break;
+    }
+  }
+
+  // We have already made a decision because of a higher priority selector
+  // therefore this selector's decision can be discarded.
+  if (current_request_ui_to_use_.has_value())
+    return;
+
+  CHECK_LT(selector_index, selector_decisions_.size());
+  selector_decisions_[selector_index] = decision;
+
+  size_t decision_index = 0;
+  while (decision_index < selector_decisions_.size() &&
+         selector_decisions_[decision_index].has_value()) {
+    const UiDecision& current_decision =
+        selector_decisions_[decision_index++].value();
+
+    if (current_decision.quiet_ui_reason.has_value()) {
+      current_request_ui_to_use_ = current_decision;
+      break;
+    }
+  }
+
+  // All decisions have been considered and none was conclusive.
+  if (decision_index == selector_decisions_.size() &&
+      !current_request_ui_to_use_.has_value()) {
+    current_request_ui_to_use_ = UiDecision::UseNormalUiAndShowNoWarning();
+  }
+
+  if (current_request_ui_to_use_.has_value()) {
     ScheduleShowBubble();
+  }
 }
 
 PermissionPromptDisposition

components/permissions/permission_request_manager.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_PERMISSIONS_PERMISSION_REQUEST_MANAGER_H_
 #define COMPONENTS_PERMISSIONS_PERMISSION_REQUEST_MANAGER_H_
 
+#include <algorithm>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -145,10 +146,24 @@ class PermissionRequestManager
         web_contents_supports_permission_requests;
   }
 
-  // For testing only, used to override the default UI selector.
+  // For testing only, used to override the default UI selectors and instead use
+  // a new one.
   void set_notification_permission_ui_selector_for_testing(
       std::unique_ptr<NotificationPermissionUiSelector> selector) {
-    notification_permission_ui_selector_ = std::move(selector);
+    clear_notification_permission_ui_selector_for_testing();
+    add_notification_permission_ui_selector_for_testing(std::move(selector));
+  }
+
+  // For testing only, used to add a new selector without overriding the
+  // existing ones.
+  void add_notification_permission_ui_selector_for_testing(
+      std::unique_ptr<NotificationPermissionUiSelector> selector) {
+    notification_permission_ui_selectors_.emplace_back(std::move(selector));
+  }
+
+  // For testing only, clear the existing ui selectors.
+  void clear_notification_permission_ui_selector_for_testing() {
+    notification_permission_ui_selectors_.clear();
   }
 
   void set_view_factory_for_testing(PermissionPrompt::Factory view_factory) {
@@ -209,7 +224,8 @@ class PermissionRequestManager
   void NotifyBubbleAdded();
   void NotifyBubbleRemoved();
 
-  void OnSelectedUiToUseForNotifications(const UiDecision& decision);
+  void OnNotificationPermissionUiSelectorDone(size_t selector_index,
+                                              const UiDecision& decision);
 
   PermissionPromptDisposition DetermineCurrentRequestUIDispositionForUMA();
 
@@ -256,10 +272,17 @@ class PermissionRequestManager
   // origin requesting the permission.
   bool is_notification_prompt_cooldown_active_ = false;
 
-  // Decides if the quiet prompt UI should be used to display notification
-  // permission requests.
-  std::unique_ptr<NotificationPermissionUiSelector>
-      notification_permission_ui_selector_;
+  // A vector of selectors which decide if the quiet prompt UI should be used
+  // to display notification permission requests. Sorted from the highest
+  // priority to the lowest priority selector.
+  std::vector<std::unique_ptr<NotificationPermissionUiSelector>>
+      notification_permission_ui_selectors_;
+
+  // Holds the decisions returned by selectors. Needed in case a lower priority
+  // selector returns a decision first and we need to wait for the decisions of
+  // higher priority selectors before making use of it.
+  std::vector<base::Optional<NotificationPermissionUiSelector::Decision>>
+      selector_decisions_;
 
   // Whether the view for the current |requests_| has been shown to the user at
   // least once.
@@ -267,7 +290,7 @@ class PermissionRequestManager
 
   // Whether to use the normal or quiet UI to display the current permission
   // |requests_|, and whether to show warnings. This will be nullopt if we are
-  // still waiting on the result from |notification_permission_ui_selector_|.
+  // still waiting on the result from |notification_permission_ui_selectors_|.
   base::Optional<UiDecision> current_request_ui_to_use_;
 
   // Whether the bubble is being destroyed by this class, rather than in

components/permissions/permission_request_manager_unittest.cc

@@ -3,10 +3,12 @@
 // found in the LICENSE file.
 
 #include <stddef.h>
+#include <memory>
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/threading/sequenced_task_runner_handle.h"
@@ -576,7 +578,7 @@ class MockNotificationPermissionUiSelector
   static void CreateForManager(PermissionRequestManager* manager,
                                base::Optional<QuietUiReason> quiet_ui_reason,
                                bool async) {
-    manager->set_notification_permission_ui_selector_for_testing(
+    manager->add_notification_permission_ui_selector_for_testing(
         std::make_unique<MockNotificationPermissionUiSelector>(quiet_ui_reason,
                                                                async));
   }
@@ -589,6 +591,7 @@ class MockNotificationPermissionUiSelector
 TEST_F(PermissionRequestManagerTest,
        UiSelectorNotUsedForPermissionsOtherThanNotification) {
   for (auto* request : {&request_mic_, &request_camera_, &request_ptz_}) {
+    manager_->clear_notification_permission_ui_selector_for_testing();
     MockNotificationPermissionUiSelector::CreateForManager(
         manager_,
         NotificationPermissionUiSelector::QuietUiReason::kEnabledInPrefs,
@@ -620,6 +623,7 @@ TEST_F(PermissionRequestManagerTest, UiSelectorUsedForNotifications) {
   };
 
   for (const auto& test : kTests) {
+    manager_->clear_notification_permission_ui_selector_for_testing();
     MockNotificationPermissionUiSelector::CreateForManager(
         manager_, test.quiet_ui_reason, test.async);
 
@@ -644,6 +648,7 @@ TEST_F(PermissionRequestManagerTest, UiSelectorUsedForNotifications) {
 TEST_F(PermissionRequestManagerTest,
        UiSelectionHappensSeparatelyForEachRequest) {
   using QuietUiReason = NotificationPermissionUiSelector::QuietUiReason;
+  manager_->clear_notification_permission_ui_selector_for_testing();
   MockNotificationPermissionUiSelector::CreateForManager(
       manager_, QuietUiReason::kEnabledInPrefs, true);
   MockPermissionRequest request1(
@@ -657,6 +662,7 @@ TEST_F(PermissionRequestManagerTest,
   MockPermissionRequest request2(
       "request2", PermissionRequestType::PERMISSION_NOTIFICATIONS,
       PermissionRequestGestureType::GESTURE);
+  manager_->clear_notification_permission_ui_selector_for_testing();
   MockNotificationPermissionUiSelector::CreateForManager(
       manager_, NotificationPermissionUiSelector::Decision::UseNormalUi(),
       true);
@@ -677,4 +683,80 @@ TEST_F(PermissionRequestManagerTest, RequestsNotSupported) {
   manager_->AddRequest(web_contents()->GetMainFrame(), &request2_);
   EXPECT_TRUE(request2_.cancelled());
 }
+
+TEST_F(PermissionRequestManagerTest, MultipleUiSelectors) {
+  using QuietUiReason = NotificationPermissionUiSelector::QuietUiReason;
+
+  const struct {
+    std::vector<base::Optional<QuietUiReason>> quiet_ui_reasons;
+    std::vector<bool> simulate_delayed_decision;
+    base::Optional<QuietUiReason> expected_reason;
+  } kTests[] = {
+      // Simple sync selectors, first one should take priority.
+      {{QuietUiReason::kTriggeredByCrowdDeny, QuietUiReason::kEnabledInPrefs},
+       {false, false},
+       QuietUiReason::kTriggeredByCrowdDeny},
+      // First selector is async but should still take priority even if it
+      // returns later.
+      {{QuietUiReason::kTriggeredByCrowdDeny, QuietUiReason::kEnabledInPrefs},
+       {true, false},
+       QuietUiReason::kTriggeredByCrowdDeny},
+      // The first selector that has a quiet ui decision should be used.
+      {{base::nullopt, base::nullopt,
+        QuietUiReason::kTriggeredDueToAbusiveContent,
+        QuietUiReason::kEnabledInPrefs},
+       {false, true, true, false},
+       QuietUiReason::kTriggeredDueToAbusiveContent},
+      // If all selectors return a normal ui, it should use a normal ui.
+      {{base::nullopt, base::nullopt}, {false, true}, base::nullopt},
+
+      // Use a bunch of selectors both async and sync.
+      {{base::nullopt, base::nullopt, base::nullopt, base::nullopt,
+        base::nullopt, QuietUiReason::kTriggeredDueToAbusiveRequests,
+        base::nullopt, QuietUiReason::kEnabledInPrefs},
+       {false, true, false, true, true, true, false, false},
+       QuietUiReason::kTriggeredDueToAbusiveRequests},
+      // Use a bunch of selectors all sync.
+      {{base::nullopt, base::nullopt, base::nullopt, base::nullopt,
+        base::nullopt, QuietUiReason::kTriggeredDueToAbusiveRequests,
+        base::nullopt, QuietUiReason::kEnabledInPrefs},
+       {false, false, false, false, false, false, false, false},
+       QuietUiReason::kTriggeredDueToAbusiveRequests},
+      // Use a bunch of selectors all async.
+      {{base::nullopt, base::nullopt, base::nullopt, base::nullopt,
+        base::nullopt, QuietUiReason::kTriggeredDueToAbusiveRequests,
+        base::nullopt, QuietUiReason::kEnabledInPrefs},
+       {true, true, true, true, true, true, true, true},
+       QuietUiReason::kTriggeredDueToAbusiveRequests},
+  };
+
+  for (const auto& test : kTests) {
+    manager_->clear_notification_permission_ui_selector_for_testing();
+    for (size_t i = 0; i < test.quiet_ui_reasons.size(); ++i) {
+      MockNotificationPermissionUiSelector::CreateForManager(
+          manager_, test.quiet_ui_reasons[i],
+          test.simulate_delayed_decision[i]);
+    }
+
+    MockPermissionRequest request(
+        "foo", PermissionRequestType::PERMISSION_NOTIFICATIONS,
+        PermissionRequestGestureType::GESTURE);
+
+    manager_->AddRequest(web_contents()->GetMainFrame(), &request);
+    WaitForBubbleToBeShown();
+
+    EXPECT_TRUE(prompt_factory_->is_visible());
+    EXPECT_TRUE(
+        prompt_factory_->RequestTypeSeen(request.GetPermissionRequestType()));
+    if (test.expected_reason.has_value()) {
+      EXPECT_EQ(test.expected_reason, manager_->ReasonForUsingQuietUi());
+    } else {
+      EXPECT_FALSE(manager_->ShouldCurrentRequestUseQuietUI());
+    }
+
+    Accept();
+    EXPECT_TRUE(request.granted());
+  }
+}
+
 }  // namespace permissions

components/permissions/permissions_client.cc

@@ -68,10 +68,10 @@ PermissionRequest::IconId PermissionsClient::GetOverrideIconId(
 #endif
 }
 
-std::unique_ptr<NotificationPermissionUiSelector>
-PermissionsClient::CreateNotificationPermissionUiSelector(
+std::vector<std::unique_ptr<NotificationPermissionUiSelector>>
+PermissionsClient::CreateNotificationPermissionUiSelectors(
     content::BrowserContext* browser_context) {
-  return nullptr;
+  return std::vector<std::unique_ptr<NotificationPermissionUiSelector>>();
 }
 
 void PermissionsClient::OnPromptResolved(

components/permissions/permissions_client.h

@@ -121,11 +121,14 @@ class PermissionsClient {
   // used.
   virtual PermissionRequest::IconId GetOverrideIconId(ContentSettingsType type);
 
-  // Allows the embedder to provide a selector for chossing the UI to use for
-  // notification permission requests. If the embedder returns null here, the
-  // normal UI will be used.
-  virtual std::unique_ptr<NotificationPermissionUiSelector>
-  CreateNotificationPermissionUiSelector(
+  // Allows the embedder to provide a list of selectors for choosing the UI to
+  // use for notification permission requests. If the embedder returns an empty
+  // list, the normal UI will be used always. Then for each request, if none of
+  // the returned selectors prescribe the quiet UI, the normal UI will be used.
+  // Otherwise the quiet UI will be used. Selectors at lower indices have higher
+  // priority when determining the quiet UI flavor.
+  virtual std::vector<std::unique_ptr<NotificationPermissionUiSelector>>
+  CreateNotificationPermissionUiSelectors(
       content::BrowserContext* browser_context);
 
   using QuietUiReason = NotificationPermissionUiSelector::QuietUiReason;