Extract ScreenCaptureNotificationBlocker into new file

This class will get more logic to show / hide notifications so
extracting it into a separate file so we can test it easily. Also moves
construction into the NotificationDisplayService so we can handle
dependencies there.

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

chrome/browser/BUILD.gn

@@ -3569,6 +3569,8 @@ static_library("browser") {
       "notifications/notification_system_observer.h",
       "notifications/profile_notification.cc",
       "notifications/profile_notification.h",
+      "notifications/screen_capture_notification_blocker.cc",
+      "notifications/screen_capture_notification_blocker.h",
       "obsolete_system/obsolete_system.h",
       "page_load_metrics/observers/session_restore_page_load_metrics_observer.cc",
       "page_load_metrics/observers/session_restore_page_load_metrics_observer.h",

chrome/browser/notifications/notification_display_queue.cc

@@ -7,59 +7,10 @@
 #include <algorithm>
 #include <utility>
 
-#include "base/bind.h"
-#include "base/containers/flat_set.h"
-#include "base/feature_list.h"
-#include "build/build_config.h"
-#include "chrome/browser/browser_features.h"
-#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
-#include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 
 namespace {
 
-#if !defined(OS_ANDROID)
-// This notification blocker listens to the events when the user starts
-// capturing a display. It will block notifications while such a capture is
-// ongoing. Note that this does not include casting the whole display and only
-// covers capturing via WebContents.
-class ScreenCaptureNotificationBlocker
-    : public NotificationBlocker,
-      public MediaStreamCaptureIndicator::Observer {
- public:
-  ScreenCaptureNotificationBlocker() {
-    observer_.Add(MediaCaptureDevicesDispatcher::GetInstance()
-                      ->GetMediaStreamCaptureIndicator()
-                      .get());
-  }
-
-  // NotificationBlocker:
-  bool ShouldBlockNotifications() override {
-    return !capturing_web_contents_.empty();
-  }
-
-  // MediaStreamCaptureIndicator::Observer:
-  void OnIsCapturingDisplayChanged(content::WebContents* web_contents,
-                                   bool is_capturing_display) override {
-    if (is_capturing_display)
-      capturing_web_contents_.insert(web_contents);
-    else
-      capturing_web_contents_.erase(web_contents);
-
-    NotifyBlockingStateChanged();
-  }
-
- private:
-  ScopedObserver<MediaStreamCaptureIndicator,
-                 MediaStreamCaptureIndicator::Observer>
-      observer_{this};
-
-  // Storing raw pointers here is fine because we never access them. We're only
-  // interested in the current set of WebContents that captures a display.
-  base::flat_set<content::WebContents*> capturing_web_contents_;
-};
-#endif  // !defined(OS_ANDROID)
-
 bool IsWebNotification(NotificationHandler::Type notification_type) {
   return notification_type == NotificationHandler::Type::WEB_PERSISTENT ||
          notification_type == NotificationHandler::Type::WEB_NON_PERSISTENT ||
@@ -70,19 +21,7 @@ bool IsWebNotification(NotificationHandler::Type notification_type) {
 
 NotificationDisplayQueue::NotificationDisplayQueue(
     NotificationDisplayService* notification_display_service)
-    : notification_display_service_(notification_display_service) {
-  NotificationBlockers blockers;
-
-#if !defined(OS_ANDROID)
-  // TODO(knollr): Also block notifications while casting a screen.
-  if (base::FeatureList::IsEnabled(
-          features::kMuteNotificationsDuringScreenShare)) {
-    blockers.push_back(std::make_unique<ScreenCaptureNotificationBlocker>());
-  }
-#endif  // !defined(OS_ANDROID)
-
-  SetNotificationBlockers(std::move(blockers));
-}
+    : notification_display_service_(notification_display_service) {}
 
 NotificationDisplayQueue::~NotificationDisplayQueue() = default;
 
@@ -141,6 +80,12 @@ void NotificationDisplayQueue::SetNotificationBlockers(
   MaybeDisplayQueuedNotifications();
 }
 
+void NotificationDisplayQueue::AddNotificationBlocker(
+    std::unique_ptr<NotificationBlocker> blocker) {
+  notification_blocker_observer_.Add(blocker.get());
+  blockers_.push_back(std::move(blocker));
+}
+
 void NotificationDisplayQueue::MaybeDisplayQueuedNotifications() {
   if (IsAnyNotificationBlockerActive())
     return;

chrome/browser/notifications/notification_display_queue.h

@@ -58,9 +58,12 @@ class NotificationDisplayQueue : public NotificationBlocker::Observer {
   // Returns a set of the currently queued notification ids.
   std::set<std::string> GetQueuedNotificationIds() const;
 
-  // Sets the list of |blockers| to be used and observers their state.
+  // Sets the list of |blockers| to be used and observes their state.
   void SetNotificationBlockers(NotificationBlockers blockers);
 
+  // Adds |blocker| to the list of blockers to be used and observes its state.
+  void AddNotificationBlocker(std::unique_ptr<NotificationBlocker> blocker);
+
  private:
   // Called when the state of a notification blocker changes. Will display and
   // free all queued notifications if no blocker is active anymore.

chrome/browser/notifications/notification_display_service_impl.cc

@@ -39,6 +39,10 @@
 #include "chrome/browser/nearby_sharing/nearby_notification_handler.h"
 #endif
 
+#if !defined(OS_ANDROID)
+#include "chrome/browser/notifications/screen_capture_notification_blocker.h"
+#endif
+
 namespace {
 
 void OperationCompleted() {
@@ -92,6 +96,12 @@ NotificationDisplayServiceImpl::NotificationDisplayServiceImpl(Profile* profile)
     AddNotificationHandler(NotificationHandler::Type::ANNOUNCEMENT,
                            std::make_unique<AnnouncementNotificationHandler>());
 
+    if (base::FeatureList::IsEnabled(
+            features::kMuteNotificationsDuringScreenShare)) {
+      notification_queue_.AddNotificationBlocker(
+          std::make_unique<ScreenCaptureNotificationBlocker>());
+    }
+
 #endif
 
 #if defined(OS_CHROMEOS)

chrome/browser/notifications/screen_capture_notification_blocker.cc

+// 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.
+
+#include "chrome/browser/notifications/screen_capture_notification_blocker.h"
+
+#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
+
+ScreenCaptureNotificationBlocker::ScreenCaptureNotificationBlocker() {
+  observer_.Add(MediaCaptureDevicesDispatcher::GetInstance()
+                    ->GetMediaStreamCaptureIndicator()
+                    .get());
+}
+
+ScreenCaptureNotificationBlocker::~ScreenCaptureNotificationBlocker() = default;
+
+bool ScreenCaptureNotificationBlocker::ShouldBlockNotifications() {
+  return !capturing_web_contents_.empty();
+}
+
+void ScreenCaptureNotificationBlocker::OnIsCapturingDisplayChanged(
+    content::WebContents* web_contents,
+    bool is_capturing_display) {
+  if (is_capturing_display)
+    capturing_web_contents_.insert(web_contents);
+  else
+    capturing_web_contents_.erase(web_contents);
+
+  NotifyBlockingStateChanged();
+}

chrome/browser/notifications/screen_capture_notification_blocker.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_NOTIFICATIONS_SCREEN_CAPTURE_NOTIFICATION_BLOCKER_H_
+#define CHROME_BROWSER_NOTIFICATIONS_SCREEN_CAPTURE_NOTIFICATION_BLOCKER_H_
+
+#include "base/containers/flat_set.h"
+#include "base/gtest_prod_util.h"
+#include "base/scoped_observer.h"
+#include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
+#include "chrome/browser/notifications/notification_blocker.h"
+
+namespace content {
+class WebContents;
+}  // namespace content
+
+// This notification blocker listens to the events when the user starts
+// capturing a display. It will block notifications while such a capture is
+// ongoing. Note that this does not include casting the whole display and only
+// covers capturing via WebContents.
+// TODO(crbug.com/1131375): Also block notifications while casting a screen.
+class ScreenCaptureNotificationBlocker
+    : public NotificationBlocker,
+      public MediaStreamCaptureIndicator::Observer {
+ public:
+  ScreenCaptureNotificationBlocker();
+  ScreenCaptureNotificationBlocker(const ScreenCaptureNotificationBlocker&) =
+      delete;
+  ScreenCaptureNotificationBlocker& operator=(
+      const ScreenCaptureNotificationBlocker&) = delete;
+  ~ScreenCaptureNotificationBlocker() override;
+
+  // NotificationBlocker:
+  bool ShouldBlockNotifications() override;
+
+  // MediaStreamCaptureIndicator::Observer:
+  void OnIsCapturingDisplayChanged(content::WebContents* web_contents,
+                                   bool is_capturing_display) override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(ScreenCaptureNotificationBlockerTest,
+                           ObservesMediaStreamCaptureIndicator);
+
+  ScopedObserver<MediaStreamCaptureIndicator,
+                 MediaStreamCaptureIndicator::Observer>
+      observer_{this};
+
+  // Storing raw pointers here is fine because we never access them. We're only
+  // interested in the current set of WebContents that captures a display.
+  base::flat_set<content::WebContents*> capturing_web_contents_;
+};
+
+#endif  // CHROME_BROWSER_NOTIFICATIONS_SCREEN_CAPTURE_NOTIFICATION_BLOCKER_H_

chrome/browser/notifications/screen_capture_notification_blocker_unittest.cc

+// 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.
+
+#include "chrome/browser/notifications/screen_capture_notification_blocker.h"
+
+#include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
+#include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/test_web_contents_factory.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class ScreenCaptureNotificationBlockerTest : public testing::Test {
+ public:
+  ScreenCaptureNotificationBlockerTest() = default;
+  ~ScreenCaptureNotificationBlockerTest() override = default;
+
+  ScreenCaptureNotificationBlocker& blocker() { return blocker_; }
+
+  content::WebContents* CreateWebContents() {
+    return web_contents_factory_.CreateWebContents(&profile_);
+  }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfile profile_;
+  content::TestWebContentsFactory web_contents_factory_;
+  ScreenCaptureNotificationBlocker blocker_;
+};
+
+TEST_F(ScreenCaptureNotificationBlockerTest, ShouldNotBlockWhenNotCapturing) {
+  EXPECT_FALSE(blocker().ShouldBlockNotifications());
+}
+
+TEST_F(ScreenCaptureNotificationBlockerTest, ShouldBlockWhenCapturing) {
+  blocker().OnIsCapturingDisplayChanged(CreateWebContents(), true);
+  EXPECT_TRUE(blocker().ShouldBlockNotifications());
+}
+
+TEST_F(ScreenCaptureNotificationBlockerTest, ShouldBlockWhenCapturingMutliple) {
+  content::WebContents* contents_1 = CreateWebContents();
+  content::WebContents* contents_2 = CreateWebContents();
+
+  blocker().OnIsCapturingDisplayChanged(contents_1, true);
+  blocker().OnIsCapturingDisplayChanged(contents_2, true);
+  EXPECT_TRUE(blocker().ShouldBlockNotifications());
+
+  blocker().OnIsCapturingDisplayChanged(contents_1, false);
+  EXPECT_TRUE(blocker().ShouldBlockNotifications());
+
+  blocker().OnIsCapturingDisplayChanged(contents_2, false);
+  EXPECT_FALSE(blocker().ShouldBlockNotifications());
+}
+
+TEST_F(ScreenCaptureNotificationBlockerTest, CapturingTwice) {
+  content::WebContents* contents = CreateWebContents();
+
+  // Calling changed twice with the same contents should ignore the second call.
+  blocker().OnIsCapturingDisplayChanged(contents, true);
+  blocker().OnIsCapturingDisplayChanged(contents, true);
+  EXPECT_TRUE(blocker().ShouldBlockNotifications());
+
+  blocker().OnIsCapturingDisplayChanged(contents, false);
+  EXPECT_FALSE(blocker().ShouldBlockNotifications());
+}
+
+TEST_F(ScreenCaptureNotificationBlockerTest, StopUnknownContents) {
+  content::WebContents* contents = CreateWebContents();
+  blocker().OnIsCapturingDisplayChanged(contents, false);
+  EXPECT_FALSE(blocker().ShouldBlockNotifications());
+}
+
+TEST_F(ScreenCaptureNotificationBlockerTest,
+       ObservesMediaStreamCaptureIndicator) {
+  MediaStreamCaptureIndicator* indicator =
+      MediaCaptureDevicesDispatcher::GetInstance()
+          ->GetMediaStreamCaptureIndicator()
+          .get();
+  EXPECT_TRUE(blocker().observer_.IsObserving(indicator));
+}

chrome/test/BUILD.gn

@@ -3950,6 +3950,7 @@ test("unit_tests") {
       "../browser/media/kaleidoscope/kaleidoscope_metrics_recorder_unittest.cc",
       "../browser/media/kaleidoscope/kaleidoscope_service_unittest.cc",
       "../browser/media/kaleidoscope/kaleidoscope_switches_unittest.cc",
+      "../browser/notifications/screen_capture_notification_blocker_unittest.cc",
       "../browser/password_manager/generated_password_leak_detection_pref_unittest.cc",
       "../browser/performance_manager/test_support/page_discarding_utils.cc",
       "../browser/performance_manager/test_support/page_discarding_utils.h",