VmCameraMicManager: Move notification controlling logic into VmInfo

This CL does not change the logic, but moves notification controlling
logic into VmInfo. The reason is that we are going to implement some
notification debounce logic into each VmInfo. This makes thing a bit
simpler. It also merges `VmNotificationObserver` into `VmInfo` because
we don't really need a separate class.

A few notes:

* `vm_info_map_`'s type is changed from `base::flat_map` to `std::map`
  because VmInfo is not copyable now.
* VmCameraMicManager::{Open,Close}Notification() is moved into `VmInfo`
  with only some trivial changes.

Bug: b/167491603
Change-Id: Ib41f68c67887e71563b90aeda37ef54ee60b1138
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2641827
Commit-Queue: Jason Lin <lxj@google.com>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846029}

chrome/browser/chromeos/camera_mic/vm_camera_mic_manager.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/camera_mic/vm_camera_mic_manager.h"
 
+#include <tuple>
 #include <utility>
 
 #include "ash/public/cpp/notification_utils.h"
@@ -43,16 +44,6 @@ const char kNotificationIdPrefix[] = "vm_camera_mic_manager";
 const base::TimeDelta kObserverTimerDelay =
     base::TimeDelta::FromMilliseconds(100);
 
-void OpenCrostiniSettings(Profile* profile) {
-  chrome::SettingsWindowManager::GetInstance()->ShowOSSettings(
-      profile, chromeos::settings::mojom::kCrostiniDetailsSubpagePath);
-}
-
-void OpenPluginVmSettings(Profile* profile) {
-  chrome::ShowAppManagementPage(profile, plugin_vm::kPluginVmShelfAppId,
-                                AppManagementEntryPoint::kNotificationPluginVm);
-}
-
 }  // namespace
 
 constexpr VmCameraMicManager::NotificationType
@@ -64,16 +55,20 @@ constexpr VmCameraMicManager::NotificationType
 constexpr VmCameraMicManager::NotificationType
     VmCameraMicManager::kCameraAndMicNotification;
 
-class VmCameraMicManager::VmInfo {
+class VmCameraMicManager::VmInfo : public message_center::NotificationObserver {
  public:
-  VmInfo() = default;
-  VmInfo(const VmInfo&) = default;
+  VmInfo(Profile* profile, VmType vm_type, int name_id)
+      : profile_(profile), vm_type_(vm_type), name_id_(name_id) {}
   ~VmInfo() = default;
 
+  VmType vm_type() const { return vm_type_; }
+  int name_id() const { return name_id_; }
   NotificationType notification_type() const { return notification_type_; }
 
   void SetMicActive(bool active) {
-    notification_type_.set(static_cast<size_t>(DeviceType::kMic), active);
+    target_notification_type_.set(static_cast<size_t>(DeviceType::kMic),
+                                  active);
+    UpdateNotification();
   }
 
   void SetCameraAccessing(bool accessing) {
@@ -87,16 +82,123 @@ class VmCameraMicManager::VmInfo {
 
  private:
   void OnCameraUpdated() {
-    notification_type_.set(static_cast<size_t>(DeviceType::kCamera),
+    target_notification_type_.set(static_cast<size_t>(DeviceType::kCamera),
                                   camera_accessing_ && !camera_privacy_is_on_);
+    UpdateNotification();
+  }
+
+  void UpdateNotification() {
+    if (notification_type_ == target_notification_type_) {
+      return;
+    }
+
+    // We always show 0 or 1 notifications for a VM, so here we just need to
+    // close the previous one if it exists and open the new one if necessary.
+    if (notification_type_ != kNoNotification) {
+      CloseNotification(notification_type_);
+    }
+    if (target_notification_type_ != kNoNotification) {
+      OpenNotification(target_notification_type_);
+    }
+
+    notification_type_ = target_notification_type_;
+  }
+
+  void OpenNotification(NotificationType type) const {
+    DCHECK_NE(type, kNoNotification);
+    if (!base::FeatureList::IsEnabled(
+            features::kVmCameraMicIndicatorsAndNotifications)) {
+      return;
+    }
+
+    const gfx::VectorIcon* source_icon = nullptr;
+    int message_id;
+    if (type[static_cast<size_t>(DeviceType::kCamera)]) {
+      source_icon = &::vector_icons::kVideocamIcon;
+      if (type[static_cast<size_t>(DeviceType::kMic)]) {
+        message_id = IDS_APP_USING_CAMERA_MIC_NOTIFICATION_MESSAGE;
+      } else {
+        message_id = IDS_APP_USING_CAMERA_NOTIFICATION_MESSAGE;
+      }
+    } else {
+      DCHECK_EQ(type, kMicNotification);
+      source_icon = &::vector_icons::kMicIcon;
+      message_id = IDS_APP_USING_MIC_NOTIFICATION_MESSAGE;
+    }
+
+    message_center::RichNotificationData rich_notification_data;
+    rich_notification_data.vector_small_image = source_icon;
+    rich_notification_data.pinned = true;
+    rich_notification_data.buttons.emplace_back(
+        l10n_util::GetStringUTF16(IDS_INTERNAL_APP_SETTINGS));
+    rich_notification_data.fullscreen_visibility =
+        message_center::FullscreenVisibility::OVER_USER;
+
+    message_center::Notification notification(
+        message_center::NOTIFICATION_TYPE_SIMPLE,
+        GetNotificationId(vm_type_, type),
+        /*title=*/
+        l10n_util::GetStringFUTF16(message_id,
+                                   l10n_util::GetStringUTF16(name_id_)),
+        /*message=*/base::string16(),
+        /*icon=*/gfx::Image(),
+        /*display_source=*/
+        l10n_util::GetStringUTF16(IDS_CHROME_OS_NOTIFICATION_SOURCE),
+        /*origin_url=*/GURL(),
+        message_center::NotifierId(
+            message_center::NotifierType::SYSTEM_COMPONENT,
+            ash::kVmCameraMicNotifierId),
+        rich_notification_data,
+        base::MakeRefCounted<message_center::ThunkNotificationDelegate>(
+            weak_ptr_factory_.GetWeakPtr()));
+
+    NotificationDisplayService::GetForProfile(profile_)->Display(
+        NotificationHandler::Type::TRANSIENT, notification,
+        /*metadata=*/nullptr);
+  }
+
+  void CloseNotification(NotificationType type) const {
+    DCHECK_NE(type, kNoNotification);
+    if (!base::FeatureList::IsEnabled(
+            features::kVmCameraMicIndicatorsAndNotifications)) {
+      return;
+    }
+    NotificationDisplayService::GetForProfile(profile_)->Close(
+        NotificationHandler::Type::TRANSIENT,
+        GetNotificationId(vm_type_, type));
+  }
+
+  // message_center::NotificationObserver:
+  //
+  // This open the settings page if the button is clicked on the notification.
+  void Click(const base::Optional<int>& button_index,
+             const base::Optional<base::string16>& reply) override {
+    switch (vm_type_) {
+      case VmType::kCrostiniVm:
+        chrome::SettingsWindowManager::GetInstance()->ShowOSSettings(
+            profile_, chromeos::settings::mojom::kCrostiniDetailsSubpagePath);
+        break;
+      case VmType::kPluginVm:
+        chrome::ShowAppManagementPage(
+            profile_, plugin_vm::kPluginVmShelfAppId,
+            AppManagementEntryPoint::kNotificationPluginVm);
+        break;
+    }
   }
 
+  Profile* const profile_;
+  const VmType vm_type_;
+  const int name_id_;
+
   bool camera_accessing_ = false;
   // We don't actually need to store this separately for each VM, but this
   // makes code simpler.
   bool camera_privacy_is_on_ = false;
 
   NotificationType notification_type_;
+  NotificationType target_notification_type_;
+
+  base::WeakPtrFactory<VmInfo> weak_ptr_factory_{this};
 };
 
 VmCameraMicManager* VmCameraMicManager::Get() {
@@ -117,14 +219,14 @@ void VmCameraMicManager::OnPrimaryUserSessionStarted(Profile* primary_profile) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   primary_profile_ = primary_profile;
-  crostini_vm_notification_observer_.Initialize(
-      primary_profile_, base::BindRepeating(OpenCrostiniSettings));
-  plugin_vm_notification_observer_.Initialize(
-      primary_profile_, base::BindRepeating(OpenPluginVmSettings));
 
-  for (VmType vm : {VmType::kCrostiniVm, VmType::kPluginVm}) {
-    vm_info_map_[vm] = {};
-  }
+  auto emplace_vm_info = [this](VmType vm, int name_id) {
+    vm_info_map_.emplace(std::piecewise_construct, std::forward_as_tuple(vm),
+                         std::forward_as_tuple(primary_profile_, vm, name_id));
+  };
+
+  emplace_vm_info(VmType::kCrostiniVm, IDS_CROSTINI_LINUX);
+  emplace_vm_info(VmType::kPluginVm, IDS_PLUGIN_VM_APP_NAME);
 
   // Only do the subscription in real ChromeOS environment.
   if (base::SysInfo::IsRunningOnChromeOS()) {
@@ -164,33 +266,18 @@ void VmCameraMicManager::MaybeSubscribeToCameraService(
       camera->AddCameraPrivacySwitchObserver(this));
 }
 
-void VmCameraMicManager::UpdateVmInfoAndNotifications(
-    VmType vm,
+void VmCameraMicManager::UpdateVmInfo(VmType vm,
                                       void (VmInfo::*updator)(bool),
                                       bool value) {
   auto it = vm_info_map_.find(vm);
   CHECK(it != vm_info_map_.end());
   auto& vm_info = it->second;
 
-  const NotificationType old_notification_type = vm_info.notification_type();
   (vm_info.*updator)(value);
-  const NotificationType new_notification_type = vm_info.notification_type();
-
-  if (old_notification_type == new_notification_type)
-    return;
 
   if (!observer_timer_.IsRunning()) {
     observer_timer_.Reset();
   }
-
-  // We always show 0 or 1 notifications for a VM, so here we just need to close
-  // the previous one if it exists and open the new one if necessary.
-  if (old_notification_type != kNoNotification) {
-    CloseNotification(vm, old_notification_type);
-  }
-  if (new_notification_type != kNoNotification) {
-    OpenNotification(vm, new_notification_type);
-  }
 }
 
 bool VmCameraMicManager::IsDeviceActive(DeviceType device) const {
@@ -228,7 +315,7 @@ void VmCameraMicManager::OnActiveClientChange(
 }
 
 void VmCameraMicManager::SetCameraAccessing(VmType vm, bool accessing) {
-  UpdateVmInfoAndNotifications(vm, &VmInfo::SetCameraAccessing, accessing);
+  UpdateVmInfo(vm, &VmInfo::SetCameraAccessing, accessing);
 }
 
 void VmCameraMicManager::OnCameraPrivacySwitchStatusChanged(
@@ -253,8 +340,8 @@ void VmCameraMicManager::OnCameraPrivacySwitchStatusChanged(
 void VmCameraMicManager::SetCameraPrivacyIsOn(bool is_on) {
   DCHECK(!vm_info_map_.empty());
   for (auto& vm_and_info : vm_info_map_) {
-    UpdateVmInfoAndNotifications(/*vm=*/vm_and_info.first,
-                                 &VmInfo::SetCameraPrivacyIsOn, is_on);
+    UpdateVmInfo(/*vm=*/vm_and_info.first, &VmInfo::SetCameraPrivacyIsOn,
+                 is_on);
   }
 }
 
@@ -290,103 +377,6 @@ std::string VmCameraMicManager::GetNotificationId(VmType vm,
   return id;
 }
 
-void VmCameraMicManager::OpenNotification(VmType vm, NotificationType type) {
-  DCHECK_NE(type, kNoNotification);
-  if (!base::FeatureList::IsEnabled(
-          features::kVmCameraMicIndicatorsAndNotifications)) {
-    return;
-  }
-
-  const gfx::VectorIcon* source_icon = nullptr;
-  int message_id;
-  if (type[static_cast<size_t>(DeviceType::kCamera)]) {
-    source_icon = &::vector_icons::kVideocamIcon;
-    if (type[static_cast<size_t>(DeviceType::kMic)]) {
-      message_id = IDS_APP_USING_CAMERA_MIC_NOTIFICATION_MESSAGE;
-    } else {
-      message_id = IDS_APP_USING_CAMERA_NOTIFICATION_MESSAGE;
-    }
-  } else {
-    DCHECK_EQ(type, kMicNotification);
-    source_icon = &::vector_icons::kMicIcon;
-    message_id = IDS_APP_USING_MIC_NOTIFICATION_MESSAGE;
-  }
-
-  int app_name_id;
-  base::WeakPtr<message_center::NotificationObserver> notification_observer_;
-  switch (vm) {
-    case VmCameraMicManager::VmType::kCrostiniVm:
-      app_name_id = IDS_CROSTINI_LINUX;
-      notification_observer_ = crostini_vm_notification_observer_.GetWeakPtr();
-      break;
-    case VmCameraMicManager::VmType::kPluginVm:
-      app_name_id = IDS_PLUGIN_VM_APP_NAME;
-      notification_observer_ = plugin_vm_notification_observer_.GetWeakPtr();
-      break;
-  }
-
-  message_center::RichNotificationData rich_notification_data;
-  rich_notification_data.vector_small_image = source_icon;
-  rich_notification_data.pinned = true;
-  rich_notification_data.buttons.emplace_back(
-      l10n_util::GetStringUTF16(IDS_INTERNAL_APP_SETTINGS));
-  rich_notification_data.fullscreen_visibility =
-      message_center::FullscreenVisibility::OVER_USER;
-
-  message_center::Notification notification(
-      message_center::NOTIFICATION_TYPE_SIMPLE, GetNotificationId(vm, type),
-      /*title=*/
-      l10n_util::GetStringFUTF16(message_id,
-                                 l10n_util::GetStringUTF16(app_name_id)),
-      /*message=*/base::string16(),
-      /*icon=*/gfx::Image(),
-      /*display_source=*/
-      l10n_util::GetStringUTF16(IDS_CHROME_OS_NOTIFICATION_SOURCE),
-      /*origin_url=*/GURL(),
-      message_center::NotifierId(message_center::NotifierType::SYSTEM_COMPONENT,
-                                 ash::kVmCameraMicNotifierId),
-      rich_notification_data,
-      base::MakeRefCounted<message_center::ThunkNotificationDelegate>(
-          std::move(notification_observer_)));
-
-  NotificationDisplayService::GetForProfile(primary_profile_)
-      ->Display(NotificationHandler::Type::TRANSIENT, notification,
-                /*metadata=*/nullptr);
-}
-
-void VmCameraMicManager::CloseNotification(VmType vm, NotificationType type) {
-  DCHECK_NE(type, kNoNotification);
-  if (!base::FeatureList::IsEnabled(
-          features::kVmCameraMicIndicatorsAndNotifications)) {
-    return;
-  }
-  NotificationDisplayService::GetForProfile(primary_profile_)
-      ->Close(NotificationHandler::Type::TRANSIENT,
-              GetNotificationId(vm, type));
-}
-
-VmCameraMicManager::VmNotificationObserver::VmNotificationObserver() = default;
-VmCameraMicManager::VmNotificationObserver::~VmNotificationObserver() = default;
-
-void VmCameraMicManager::VmNotificationObserver::Initialize(
-    Profile* profile,
-    OpenSettingsFunction open_settings) {
-  profile_ = profile;
-  open_settings_ = std::move(open_settings);
-}
-
-base::WeakPtr<message_center::NotificationObserver>
-VmCameraMicManager::VmNotificationObserver::GetWeakPtr() {
-  return weak_ptr_factory_.GetWeakPtr();
-}
-
-void VmCameraMicManager::VmNotificationObserver::Click(
-    const base::Optional<int>& button_index,
-    const base::Optional<base::string16>& reply) {
-  // We only have one button --- the settings button.
-  open_settings_.Run(profile_);
-}
-
 void VmCameraMicManager::OnNumberOfInputStreamsWithPermissionChanged() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
@@ -405,7 +395,7 @@ void VmCameraMicManager::OnNumberOfInputStreamsWithPermissionChanged() {
 }
 
 void VmCameraMicManager::SetMicActive(VmType vm, bool active) {
-  UpdateVmInfoAndNotifications(vm, &VmInfo::SetMicActive, active);
+  UpdateVmInfo(vm, &VmInfo::SetMicActive, active);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/camera_mic/vm_camera_mic_manager.h

@@ -84,27 +84,6 @@ class VmCameraMicManager : public media::CameraActiveClientObserver,
 
   class VmInfo;
 
-  class VmNotificationObserver : public message_center::NotificationObserver {
-   public:
-    using OpenSettingsFunction = base::RepeatingCallback<void(Profile*)>;
-
-    VmNotificationObserver();
-    ~VmNotificationObserver();
-
-    void Initialize(Profile* profile, OpenSettingsFunction open_settings);
-
-    base::WeakPtr<NotificationObserver> GetWeakPtr();
-
-    // message_center::NotificationObserver:
-    void Click(const base::Optional<int>& button_index,
-               const base::Optional<base::string16>& reply) override;
-
-   private:
-    Profile* profile_ = nullptr;
-    OpenSettingsFunction open_settings_;
-    base::WeakPtrFactory<VmNotificationObserver> weak_ptr_factory_{this};
-  };
-
   void MaybeSubscribeToCameraService(bool should_use_cros_camera_service);
 
   // media::CameraActiveClientObserver
@@ -124,18 +103,11 @@ class VmCameraMicManager : public media::CameraActiveClientObserver,
 
   static std::string GetNotificationId(VmType vm, NotificationType type);
 
-  void UpdateVmInfoAndNotifications(VmType vm,
-                                    void (VmInfo::*updator)(bool),
-                                    bool value);
+  void UpdateVmInfo(VmType vm, void (VmInfo::*updator)(bool), bool value);
   void NotifyActiveChanged();
 
-  void OpenNotification(VmType vm, NotificationType type);
-  void CloseNotification(VmType vm, NotificationType type);
-
   Profile* primary_profile_ = nullptr;
-  VmNotificationObserver crostini_vm_notification_observer_;
-  VmNotificationObserver plugin_vm_notification_observer_;
-  base::flat_map<VmType, VmInfo> vm_info_map_;
+  std::map<VmType, VmInfo> vm_info_map_;
 
   base::RetainingOneShotTimer observer_timer_;
   base::ObserverList<Observer> observers_;