Only mute web notifications

Other types of notifications shown by Chrome are typically features
explicitly triggered by the user and should still be shown even if the
user is currently recording the whole screen.

Bug: 1131375
Change-Id: I06bc335f8b556fb9105d99314ec1070a653e7810
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437397
Commit-Queue: Richard Knoll <knollr@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814283}

chrome/browser/notifications/notification_display_queue.cc

@@ -60,6 +60,12 @@ class ScreenCaptureNotificationBlocker
 };
 #endif  // !defined(OS_ANDROID)
 
+bool IsWebNotification(NotificationHandler::Type notification_type) {
+  return notification_type == NotificationHandler::Type::WEB_PERSISTENT ||
+         notification_type == NotificationHandler::Type::WEB_NON_PERSISTENT ||
+         notification_type == NotificationHandler::Type::EXTENSION;
+}
+
 }  // namespace
 
 NotificationDisplayQueue::NotificationDisplayQueue(
@@ -84,11 +90,10 @@ void NotificationDisplayQueue::OnBlockingStateChanged() {
   MaybeDisplayQueuedNotifications();
 }
 
-bool NotificationDisplayQueue::ShouldEnqueueNotifications() {
-  return std::any_of(blockers_.begin(), blockers_.end(),
-                     [](const std::unique_ptr<NotificationBlocker>& blocker) {
-                       return blocker->ShouldBlockNotifications();
-                     });
+bool NotificationDisplayQueue::ShouldEnqueueNotifications(
+    NotificationHandler::Type notification_type) const {
+  return IsWebNotification(notification_type) &&
+         IsAnyNotificationBlockerActive();
 }
 
 void NotificationDisplayQueue::EnqueueNotification(
@@ -112,7 +117,8 @@ void NotificationDisplayQueue::RemoveQueuedNotification(
     queued_notifications_.erase(it);
 }
 
-std::set<std::string> NotificationDisplayQueue::GetQueuedNotificationIds() {
+std::set<std::string> NotificationDisplayQueue::GetQueuedNotificationIds()
+    const {
   std::set<std::string> notification_ids;
   for (const QueuedNotification& queued : queued_notifications_)
     notification_ids.insert(queued.notification.id());
@@ -136,7 +142,7 @@ void NotificationDisplayQueue::SetNotificationBlockers(
 }
 
 void NotificationDisplayQueue::MaybeDisplayQueuedNotifications() {
-  if (ShouldEnqueueNotifications())
+  if (IsAnyNotificationBlockerActive())
     return;
 
   std::vector<QueuedNotification> queued_notifications =
@@ -150,6 +156,13 @@ void NotificationDisplayQueue::MaybeDisplayQueuedNotifications() {
   }
 }
 
+bool NotificationDisplayQueue::IsAnyNotificationBlockerActive() const {
+  return std::any_of(blockers_.begin(), blockers_.end(),
+                     [](const std::unique_ptr<NotificationBlocker>& blocker) {
+                       return blocker->ShouldBlockNotifications();
+                     });
+}
+
 NotificationDisplayQueue::QueuedNotification::QueuedNotification(
     NotificationHandler::Type notification_type,
     const message_center::Notification& notification,

chrome/browser/notifications/notification_display_queue.h

@@ -37,9 +37,11 @@ class NotificationDisplayQueue : public NotificationBlocker::Observer {
   // NotificationBlocker::Observer:
   void OnBlockingStateChanged() override;
 
-  // Returns if we should currently queue up notifications. This is the case if
-  // at least one NotificationBlocker is active.
-  bool ShouldEnqueueNotifications();
+  // Returns if we should currently queue up |notification_type| notifications.
+  // This is the case if at least one NotificationBlocker is active and
+  // |notification_type| is a Web Notification.
+  bool ShouldEnqueueNotifications(
+      NotificationHandler::Type notification_type) const;
 
   // Enqueue the passed |notification| to be shown once no blocker is active
   // anymore. If there is already a notification with the same id queued, it
@@ -54,7 +56,7 @@ class NotificationDisplayQueue : public NotificationBlocker::Observer {
   void RemoveQueuedNotification(const std::string& notification_id);
 
   // Returns a set of the currently queued notification ids.
-  std::set<std::string> GetQueuedNotificationIds();
+  std::set<std::string> GetQueuedNotificationIds() const;
 
   // Sets the list of |blockers| to be used and observers their state.
   void SetNotificationBlockers(NotificationBlockers blockers);
@@ -64,6 +66,9 @@ class NotificationDisplayQueue : public NotificationBlocker::Observer {
   // free all queued notifications if no blocker is active anymore.
   void MaybeDisplayQueuedNotifications();
 
+  // Checks if any notification blocker is currently active.
+  bool IsAnyNotificationBlockerActive() const;
+
   // Represents a queued notification.
   struct QueuedNotification {
     QueuedNotification(NotificationHandler::Type notification_type,

chrome/browser/notifications/notification_display_queue_unittest.cc

@@ -113,16 +113,25 @@ class NotificationDisplayQueueTest : public testing::Test {
 
 TEST_F(NotificationDisplayQueueTest, ShouldEnqueueWithoutBlockers) {
   queue().SetNotificationBlockers({});
-  EXPECT_FALSE(queue().ShouldEnqueueNotifications());
+  EXPECT_FALSE(queue().ShouldEnqueueNotifications(
+      NotificationHandler::Type::WEB_PERSISTENT));
 }
 
 TEST_F(NotificationDisplayQueueTest, ShouldEnqueueWithAllowingBlocker) {
-  EXPECT_FALSE(queue().ShouldEnqueueNotifications());
+  EXPECT_FALSE(queue().ShouldEnqueueNotifications(
+      NotificationHandler::Type::WEB_PERSISTENT));
 }
 
 TEST_F(NotificationDisplayQueueTest, ShouldEnqueueWithBlockingBlocker) {
   notification_blocker().SetShouldBlockNotifications(true);
-  EXPECT_TRUE(queue().ShouldEnqueueNotifications());
+  EXPECT_TRUE(queue().ShouldEnqueueNotifications(
+      NotificationHandler::Type::WEB_PERSISTENT));
+}
+
+TEST_F(NotificationDisplayQueueTest, ShouldEnqueueForNonWebNotification) {
+  notification_blocker().SetShouldBlockNotifications(true);
+  EXPECT_FALSE(
+      queue().ShouldEnqueueNotifications(NotificationHandler::Type::TRANSIENT));
 }
 
 TEST_F(NotificationDisplayQueueTest, EnqueueNotification) {

chrome/browser/notifications/notification_display_service_impl.cc

@@ -196,7 +196,7 @@ void NotificationDisplayServiceImpl::Display(
   for (auto& observer : observers_)
     observer.OnNotificationDisplayed(notification, metadata.get());
 
-  if (notification_queue_.ShouldEnqueueNotifications()) {
+  if (notification_queue_.ShouldEnqueueNotifications(notification_type)) {
     notification_queue_.EnqueueNotification(notification_type, notification,
                                             std::move(metadata));
   } else {

chrome/browser/notifications/notification_display_service_impl_unittest.cc

@@ -81,8 +81,7 @@ message_center::Notification CreateNotification(const std::string& id) {
       /*message=*/base::string16(), /*icon=*/gfx::Image(),
       /*display_source=*/base::string16(),
       /*origin_url=*/GURL(), message_center::NotifierId(),
-      message_center::RichNotificationData(),
-      base::MakeRefCounted<message_center::NotificationDelegate>());
+      message_center::RichNotificationData(), /*delegate=*/nullptr);
 }
 
 }  // namespace
@@ -151,12 +150,12 @@ class NotificationDisplayServiceImplTest : public testing::Test {
   }
 
   void DisplayNotification(const std::string id) {
-    service_->Display(NotificationHandler::Type::TRANSIENT,
+    service_->Display(NotificationHandler::Type::WEB_PERSISTENT,
                       CreateNotification(id), /*metadata=*/nullptr);
   }
 
   void CloseNotification(const std::string id) {
-    service_->Close(NotificationHandler::Type::TRANSIENT, id);
+    service_->Close(NotificationHandler::Type::WEB_PERSISTENT, id);
   }
 
  private: