Remove feature parameters to specify UI flavors.

Now that we have settled on using an animated icon on desktop, and a
mini infobar on Android, remove the ability to specify the UI flavor
using either variation parameters or FeatureEntry::FeatureParams.

The CL also removes now-obsolete string literals, and cleans up tests:
  -- PermissionRequestNotificationAndroidTest.ShouldShowAsNotification
     already codified the expectation that the UI flavor variation
     parameters are ignored, this test is now removed.
  -- PermissionRequestManagerBrowserTest_StaticIcon.* verified behavior
     that was orthogonal to whether the quiet prompt is shown as static
     or animated icon, and the test for the latter still exists, so the
     former is now removed.

Bug: 1030216
Change-Id: I32c27d327a8704572bb617cd0800338587f58dab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948428Reviewed-by: Kamila Hasanbega <hkamila@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720917}

chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionFieldTrial.java

@@ -19,9 +19,6 @@ import java.lang.annotation.RetentionPolicy;
  * Provides Field Trial support for the permissions field trial
  */
 public class PermissionFieldTrial {
-    // Keep in sync with "chrome/browser/permissions/permission_features.h"
-    private static final String QUIET_NOTIFICATION_PROMPTS_MINI_INFOBAR = "mini_infobar";
-
     @IntDef({UIFlavor.NONE, UIFlavor.MINI_INFOBAR})
     @Retention(RetentionPolicy.SOURCE)
     public @interface UIFlavor {

chrome/browser/about_flags.cc

@@ -1313,8 +1313,6 @@ const FeatureEntry::FeatureVariation
 #if defined(OS_ANDROID)
 const FeatureEntry::FeatureParam kQuietNotificationPromptsForceMiniInfobars[] =
     {
-        {kQuietNotificationPromptsUIFlavorParameterName,
-         kQuietNotificationPromptsMiniInfobar},
         {kQuietNotificationPromptsActivationParameterName,
          kQuietNotificationPromptsActivationAlways},
 };
@@ -1323,34 +1321,21 @@ const FeatureEntry::FeatureParam kQuietNotificationPromptsForceMiniInfobars[] =
 // adaptively after 3 consecutive denies (or when enabled in settings). In
 // addition to that, expose an option to force-enable "mini-infobars".
 const FeatureEntry::FeatureVariation kQuietNotificationPromptsVariations[] = {
-    {"(force mini-infobars)", kQuietNotificationPromptsForceMiniInfobars,
+    {"(forced)", kQuietNotificationPromptsForceMiniInfobars,
      base::size(kQuietNotificationPromptsForceMiniInfobars), nullptr},
 };
 #else   // OS_ANDROID
-const FeatureEntry::FeatureParam
-    kQuietNotificationPromptsForceStaticIconNotificationsPrompt[] = {
-        {kQuietNotificationPromptsUIFlavorParameterName,
-         kQuietNotificationPromptsStaticIcon},
-        {kQuietNotificationPromptsActivationParameterName,
-         kQuietNotificationPromptsActivationAlways},
-};
 const FeatureEntry::FeatureParam
     kQuietNotificationPromptsForceAnimatedIconNotificationsPrompt[] = {
-        {kQuietNotificationPromptsUIFlavorParameterName,
-         kQuietNotificationPromptsAnimatedIcon},
         {kQuietNotificationPromptsActivationParameterName,
          kQuietNotificationPromptsActivationAlways},
 };
 
-// The "default" option that only shows "Enabled" will be the static icon,
-// triggered after 3 consecutive denies.
+// The default "Enabled" option has the semantics of showing "animated icon"
+// adaptively after 3 consecutive denies (or when enabled in settings). In
+// addition to that, expose an option to force-enable "animated icons".
 const FeatureEntry::FeatureVariation kQuietNotificationPromptsVariations[] = {
-    {"(force static-icon)",
-     kQuietNotificationPromptsForceStaticIconNotificationsPrompt,
-     base::size(kQuietNotificationPromptsForceStaticIconNotificationsPrompt),
-     nullptr},
-    {"(force animated-icon)",
-     kQuietNotificationPromptsForceAnimatedIconNotificationsPrompt,
+    {"(forced)", kQuietNotificationPromptsForceAnimatedIconNotificationsPrompt,
      base::size(kQuietNotificationPromptsForceAnimatedIconNotificationsPrompt),
      nullptr},
 };

chrome/browser/permissions/permission_features.cc

@@ -9,19 +9,6 @@
 #include "base/metrics/field_trial_params.h"
 #include "chrome/common/chrome_features.h"
 
-// Keep in sync with "PermissionFieldTrial.java"
-const char kQuietNotificationPromptsUIFlavorParameterName[] = "ui_flavour";
-
-#if defined(OS_ANDROID)
-const char kQuietNotificationPromptsQuietNotification[] = "quiet_notification";
-const char kQuietNotificationPromptsHeadsUpNotification[] =
-    "heads_up_notification";
-const char kQuietNotificationPromptsMiniInfobar[] = "mini_infobar";
-#else   // OS_ANDROID
-const char kQuietNotificationPromptsStaticIcon[] = "static_icon";
-const char kQuietNotificationPromptsAnimatedIcon[] = "animated_icon";
-#endif  // OS_ANDROID
-
 const char kQuietNotificationPromptsActivationParameterName[] = "activation";
 const char kQuietNotificationPromptsActivationNever[] = "never";
 const char kQuietNotificationPromptsActivationAdaptive[] = "adaptive";
@@ -35,17 +22,7 @@ QuietNotificationsPromptConfig::UIFlavorToUse() {
 #if defined(OS_ANDROID)
   return UIFlavor::MINI_INFOBAR;
 #else   // OS_ANDROID
-  std::string ui_flavor = base::GetFieldTrialParamValueByFeature(
-      features::kQuietNotificationPrompts,
-      kQuietNotificationPromptsUIFlavorParameterName);
-
-  if (ui_flavor == kQuietNotificationPromptsStaticIcon) {
-    return UIFlavor::STATIC_ICON;
-  } else if (ui_flavor == kQuietNotificationPromptsAnimatedIcon) {
-    return UIFlavor::ANIMATED_ICON;
-  } else {
-    return UIFlavor::STATIC_ICON;
-  }
+  return UIFlavor::ANIMATED_ICON;
 #endif  // !OS_ANDROID
 }
 

chrome/browser/permissions/permission_features.h

@@ -7,17 +7,6 @@
 
 #include "build/build_config.h"
 
-extern const char kQuietNotificationPromptsUIFlavorParameterName[];
-
-#if defined(OS_ANDROID)
-extern const char kQuietNotificationPromptsQuietNotification[];
-extern const char kQuietNotificationPromptsHeadsUpNotification[];
-extern const char kQuietNotificationPromptsMiniInfobar[];
-#else   // OS_ANDROID
-extern const char kQuietNotificationPromptsStaticIcon[];
-extern const char kQuietNotificationPromptsAnimatedIcon[];
-#endif  // OS_ANDROID
-
 extern const char kQuietNotificationPromptsActivationParameterName[];
 extern const char kQuietNotificationPromptsActivationNever[];
 extern const char kQuietNotificationPromptsActivationAdaptive[];

chrome/browser/permissions/permission_prompt_android_unittest.cc

@@ -24,11 +24,8 @@ class PermissionPromptAndroidTest : public ChromeRenderViewHostTestHarness {
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
     // Ensure that the test uses the mini-infobar variant.
-    base::FieldTrialParams params;
-    params[kQuietNotificationPromptsUIFlavorParameterName] =
-        kQuietNotificationPromptsMiniInfobar;
-    scoped_feature_list_.InitAndEnableFeatureWithParameters(
-        features::kQuietNotificationPrompts, params);
+    scoped_feature_list_.InitAndEnableFeature(
+        features::kQuietNotificationPrompts);
     AdaptiveNotificationPermissionUiSelector::GetForProfile(profile())
         ->set_should_show_quiet_ui_for_testing(true);
 

chrome/browser/permissions/permission_request_manager_browsertest.cc

@@ -697,68 +697,18 @@ IN_PROC_BROWSER_TEST_F(PermissionRequestManagerBrowserTest,
       "PermissionRequestManager"));
 }
 
-// For browser tests feature overrides need to be enabled during SetUp.
-// Have a separate fixture based on which UI flavor needs to be enabled.
-class PermissionRequestManagerBrowserTest_StaticIcon
-    : public PermissionRequestManagerBrowserTest {
- public:
-  void SetUp() override {
-    base::FieldTrialParams params;
-    params[kQuietNotificationPromptsUIFlavorParameterName] =
-        kQuietNotificationPromptsStaticIcon;
-    base::test::ScopedFeatureList scoped_feature_list;
-    scoped_feature_list.InitAndEnableFeatureWithParameters(
-        features::kQuietNotificationPrompts, params);
-
-    PermissionRequestManagerBrowserTest::SetUp();
-  }
-};
-
 class PermissionRequestManagerBrowserTest_AnimatedIcon
     : public PermissionRequestManagerBrowserTest {
  public:
   void SetUp() override {
-    base::FieldTrialParams params;
-    params[kQuietNotificationPromptsUIFlavorParameterName] =
-        kQuietNotificationPromptsAnimatedIcon;
     base::test::ScopedFeatureList scoped_feature_list;
-    scoped_feature_list.InitAndEnableFeatureWithParameters(
-        features::kQuietNotificationPrompts, params);
+    scoped_feature_list.InitAndEnableFeature(
+        features::kQuietNotificationPrompts);
 
     PermissionRequestManagerBrowserTest::SetUp();
   }
 };
 
-// Re-enable when 1016233 is fixed.
-// Quiet permission requests are cancelled when a new request is made.
-IN_PROC_BROWSER_TEST_F(PermissionRequestManagerBrowserTest_StaticIcon,
-                       DISABLED_QuietPendingRequestsKilledOnNewRequest) {
-  // First add a quiet permission request. Ensure that this request is decided
-  // by the end of this test.
-  MockPermissionRequest request_quiet(
-      "quiet", PermissionRequestType::PERMISSION_NOTIFICATIONS,
-      PermissionRequestGestureType::UNKNOWN);
-  GetPermissionRequestManager()->AddRequest(&request_quiet);
-  base::RunLoop().RunUntilIdle();
-
-  // Add a second permission request. This ones should cause the initial
-  // request to be cancelled.
-  MockPermissionRequest request_loud(
-      "loud", PermissionRequestType::PERMISSION_GEOLOCATION,
-      PermissionRequestGestureType::UNKNOWN);
-  GetPermissionRequestManager()->AddRequest(&request_loud);
-  base::RunLoop().RunUntilIdle();
-
-  // The first dialog should now have been decided.
-  EXPECT_TRUE(request_quiet.finished());
-  EXPECT_EQ(1u, GetPermissionRequestManager()->Requests().size());
-
-  // Cleanup remaining request. And check that this was the last request.
-  GetPermissionRequestManager()->Closing();
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(0u, GetPermissionRequestManager()->Requests().size());
-}
-
 // Re-enable when 1016233 is fixed.
 // Quiet permission requests are cancelled when a new request is made.
 IN_PROC_BROWSER_TEST_F(PermissionRequestManagerBrowserTest_AnimatedIcon,

chrome/browser/permissions/permission_request_manager_unittest.cc

@@ -612,19 +612,11 @@ TEST_F(PermissionRequestManagerTest, UMAForTabSwitching) {
 
 TEST_F(PermissionRequestManagerTest,
        NotificationsUnderClientSideEmbargoAfterSeveralDenies) {
-  std::map<std::string, std::string> parameters;
-  parameters[kQuietNotificationPromptsUIFlavorParameterName] =
-#if defined(OS_ANDROID)
-      kQuietNotificationPromptsMiniInfobar;
-#else
-      kQuietNotificationPromptsAnimatedIcon;
-#endif
-  parameters[kQuietNotificationPromptsActivationParameterName] =
-      kQuietNotificationPromptsActivationAdaptive;
   base::test::ScopedFeatureList feature_list;
-  feature_list.InitWithFeaturesAndParameters(
-      {{features::kQuietNotificationPrompts, parameters}},
-      {features::kBlockRepeatedNotificationPermissionPrompts});
+  feature_list.InitWithFeatures(
+      {features::kQuietNotificationPrompts,
+       features::kBlockRepeatedNotificationPermissionPrompts},
+      {});
 
   auto* permission_ui_selector =
       AdaptiveNotificationPermissionUiSelector::GetForProfile(profile());

chrome/browser/permissions/permission_request_notification_android_unittest.cc

@@ -174,24 +174,3 @@ TEST_F(PermissionRequestNotificationAndroidTest, Closing_CallsDelegateClosing) {
           kExampleUrl),
       true);
 }
-
-TEST_F(PermissionRequestNotificationAndroidTest, ShouldShowAsNotification) {
-  EXPECT_FALSE(PermissionRequestNotificationAndroid::ShouldShowAsNotification(
-      profile(), ContentSettingsType::NOTIFICATIONS));
-  EXPECT_FALSE(PermissionRequestNotificationAndroid::ShouldShowAsNotification(
-      profile(), ContentSettingsType::GEOLOCATION));
-
-  base::FieldTrialParams params;
-  params[kQuietNotificationPromptsUIFlavorParameterName] =
-      kQuietNotificationPromptsHeadsUpNotification;
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
-      features::kQuietNotificationPrompts, params);
-  AdaptiveNotificationPermissionUiSelector::GetForProfile(profile())
-      ->set_should_show_quiet_ui_for_testing(true);
-
-  EXPECT_FALSE(PermissionRequestNotificationAndroid::ShouldShowAsNotification(
-      profile(), ContentSettingsType::NOTIFICATIONS));
-  EXPECT_FALSE(PermissionRequestNotificationAndroid::ShouldShowAsNotification(
-      profile(), ContentSettingsType::GEOLOCATION));
-}

chrome/browser/ui/content_settings/content_setting_image_model_unittest.cc

@@ -389,8 +389,6 @@ TEST_F(ContentSettingImageModelTest, NotificationsIconVisibility) {
 TEST_F(ContentSettingImageModelTest, NotificationsPrompt) {
 #if !defined(OS_ANDROID)
   std::map<std::string, std::string> parameters;
-  parameters[kQuietNotificationPromptsUIFlavorParameterName] =
-      kQuietNotificationPromptsAnimatedIcon;
   parameters[kQuietNotificationPromptsActivationParameterName] =
       kQuietNotificationPromptsActivationAlways;
   base::test::ScopedFeatureList feature_list;