Code Health: Clean up stale NotificationContentImage

Fixed: 356624364
Change-Id: I91db3ebf4ea3c5a9dbc0d97cddb3fad78ed1e911
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6090091
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1398301}

chrome/browser/notifications/platform_notification_service_interactive_uitest.cc

@@ -1012,54 +1012,6 @@ IN_PROC_BROWSER_TEST_F(PlatformNotificationServiceBrowserTest,
 }
 #endif  // BUILDFLAG(ENABLE_BACKGROUND_MODE)
 
-class PlatformNotificationServiceWithoutContentImageBrowserTest
-    : public PlatformNotificationServiceBrowserTest {
- public:
-  // InProcessBrowserTest overrides.
-  void SetUpInProcessBrowserTestFixture() override {
-    scoped_feature_list_.InitWithFeatures(
-        {}, {features::kNotificationContentImage});
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-};
-
-IN_PROC_BROWSER_TEST_F(
-    PlatformNotificationServiceWithoutContentImageBrowserTest,
-    KillSwitch) {
-  GrantNotificationPermissionForTest();
-
-  EXPECT_EQ("ok", RunScript("DisplayPersistentAllOptionsNotification()"));
-
-  std::vector<message_center::Notification> notifications =
-      GetDisplayedNotifications(true /* is_persistent */);
-  ASSERT_EQ(1u, notifications.size());
-
-  // Since the kNotificationContentImage kill switch has disabled images, the
-  // notification should be shown without an image.
-  EXPECT_TRUE(notifications[0].image().IsEmpty());
-}
-
-IN_PROC_BROWSER_TEST_F(
-    PlatformNotificationServiceWithoutContentImageBrowserTest,
-    KillSwitch_NonPersistentNotifications) {
-  GrantNotificationPermissionForTest();
-
-  EXPECT_EQ("ok", RunScript(
-                      R"(DisplayNonPersistentNotification('Title2', {
-          image: 'icon.png'
-        }))"));
-
-  std::vector<message_center::Notification> notifications =
-      GetDisplayedNotifications(false /* is_persistent */);
-  ASSERT_EQ(1u, notifications.size());
-
-  // Since the kNotificationContentImage kill switch has disabled images, the
-  // notification should be shown without an image.
-  EXPECT_TRUE(notifications[0].image().IsEmpty());
-}
-
 class PlatformNotificationServiceIncomingCallTest
     : public PlatformNotificationServiceBrowserTest {
  public:

content/browser/notifications/blink_notification_service_impl.cc

@@ -34,9 +34,6 @@ namespace content {
 
 namespace {
 
-const char kBadMessageImproperNotificationImage[] =
-    "Received an unexpected message with image while notification images are "
-    "disabled.";
 const char kBadMessageInvalidNotificationTriggerTimestamp[] =
     "Received an invalid notification trigger timestamp.";
 const char kBadMessageInvalidNotificationActionButtons[] =
@@ -255,16 +252,6 @@ bool BlinkNotificationServiceImpl::ValidateNotificationDataAndResources(
     OnConnectionError();
     return false;
   }
-
-  if (!notification_resources.image.drawsNothing() &&
-      !base::FeatureList::IsEnabled(features::kNotificationContentImage)) {
-    receiver_.ReportBadMessage(kBadMessageImproperNotificationImage);
-    // The above ReportBadMessage() closes |binding_| but does not trigger its
-    // connection error handler, so we need to call the error handler explicitly
-    // here to do some necessary work.
-    OnConnectionError();
-    return false;
-  }
   return true;
 }
 

content/browser/notifications/blink_notification_service_impl_unittest.cc

@@ -57,9 +57,6 @@ namespace {
 
 const char kTestOrigin[] = "https://example.com";
 const char kTestServiceWorkerUrl[] = "https://example.com/sw.js";
-const char kBadMessageImproperNotificationImage[] =
-    "Received an unexpected message with image while notification images are "
-    "disabled.";
 const char kBadMessageInvalidNotificationTriggerTimestamp[] =
     "Received an invalid notification trigger timestamp.";
 
@@ -545,23 +542,6 @@ TEST_F(BlinkNotificationServiceImplTest,
   EXPECT_EQ(1u, GetDisplayedNotifications().size());
 }
 
-TEST_F(BlinkNotificationServiceImplTest,
-       DisplayNonPersistentNotificationWithContentImageSwitchOff) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndDisableFeature(
-      features::kNotificationContentImage);
-  SetPermissionStatus(blink::mojom::PermissionStatus::GRANTED);
-
-  ASSERT_TRUE(bad_messages_.empty());
-  blink::NotificationResources resources;
-  resources.image = gfx::test::CreateBitmap(200, 100, SK_ColorMAGENTA);
-  DisplayNonPersistentNotification(
-      "token", blink::PlatformNotificationData(), resources,
-      non_persistent_notification_listener_.GetRemote());
-  EXPECT_EQ(1u, bad_messages_.size());
-  EXPECT_EQ(kBadMessageImproperNotificationImage, bad_messages_[0]);
-}
-
 TEST_F(BlinkNotificationServiceImplTest,
        DisplayPersistentNotificationWithContentImageSwitchOn) {
   SetPermissionStatus(blink::mojom::PermissionStatus::GRANTED);
@@ -583,25 +563,6 @@ TEST_F(BlinkNotificationServiceImplTest,
   EXPECT_EQ(1u, GetDisplayedNotifications().size());
 }
 
-TEST_F(BlinkNotificationServiceImplTest,
-       DisplayPersistentNotificationWithContentImageSwitchOff) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndDisableFeature(
-      features::kNotificationContentImage);
-  SetPermissionStatus(blink::mojom::PermissionStatus::GRANTED);
-
-  scoped_refptr<ServiceWorkerRegistration> registration;
-  RegisterServiceWorker(&registration);
-
-  ASSERT_TRUE(bad_messages_.empty());
-  blink::NotificationResources resources;
-  resources.image = gfx::test::CreateBitmap(200, 100, SK_ColorMAGENTA);
-  DisplayPersistentNotificationSync(
-      registration->id(), blink::PlatformNotificationData(), resources);
-  EXPECT_EQ(1u, bad_messages_.size());
-  EXPECT_EQ(kBadMessageImproperNotificationImage, bad_messages_[0]);
-}
-
 TEST_F(BlinkNotificationServiceImplTest,
        DisplayPersistentNotificationWithPermission) {
   SetPermissionStatus(blink::mojom::PermissionStatus::GRANTED);

content/child/runtime_features.cc

@@ -256,8 +256,6 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
            raw_ref(media::kMediaCastOverlayButton)},
           {wf::EnableMediaEngagementBypassAutoplayPolicies,
            raw_ref(media::kMediaEngagementBypassAutoplayPolicies)},
-          {wf::EnableNotificationContentImage,
-           raw_ref(features::kNotificationContentImage), kSetOnlyIfOverridden},
           {wf::EnablePaymentApp, raw_ref(features::kServiceWorkerPaymentApps)},
           {wf::EnablePaymentRequest, raw_ref(features::kWebPayments)},
           {wf::EnablePeriodicBackgroundSync,

content/public/common/content_features.cc

@@ -668,11 +668,6 @@ BASE_FEATURE(kNetworkServiceInProcess,
 #endif
 );
 
-// Kill switch for Web Notification content images.
-BASE_FEATURE(kNotificationContentImage,
-             "NotificationContentImage",
-             base::FEATURE_ENABLED_BY_DEFAULT);
-
 // Feature which holdbacks NoStatePrefetch on all surfaces.
 BASE_FEATURE(kNoStatePrefetchHoldback,
              "NoStatePrefetchHoldback",

content/public/common/content_features.h

@@ -176,7 +176,6 @@ CONTENT_EXPORT extern const base::FeatureParam<MBIMode> kMBIModeParam;
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kNavigationNetworkResponseQueue);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kNetworkQualityEstimatorWebHoldback);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kNetworkServiceInProcess);
-CONTENT_EXPORT BASE_DECLARE_FEATURE(kNotificationContentImage);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kNoStatePrefetchHoldback);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kOriginIsolationHeader);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kOverscrollHistoryNavigation);