Show a single "camera & mic" notification when a vm is accessing both

The old implementation is based on an older version of UX mock which
shows two notifications (one camera and one mic) in this situation.

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

chrome/app/chromeos_strings.grdp

@@ -5462,18 +5462,24 @@
   </message>
 
   <!-- Strings for camera/mic permission -->
-  <message name="IDS_CAMERA_NOTIFICATION_SOURCE" desc="The name of the source of a camera notification ">
+  <message name="IDS_CAMERA_NOTIFICATION_SOURCE" desc="The name of the source of a camera notification">
     Camera
   </message>
-  <message name="IDS_MIC_NOTIFICATION_SOURCE" desc="The name of the source of a mic notification ">
+  <message name="IDS_MIC_NOTIFICATION_SOURCE" desc="The name of the source of a mic notification">
     Microphone
   </message>
+  <message name="IDS_CAMERA_MIC_NOTIFICATION_SOURCE" desc="The name of the source of a 'camera and mic' notification">
+    Camera &amp; microphone
+  </message>
   <message name="IDS_APP_USING_CAMERA_NOTIFICATION_MESSAGE" desc="Message shown in notification when an app is using the camera">
     <ph name="APP_NAME">$1<ex>Parallels Desktop</ex></ph> is using your camera
   </message>
   <message name="IDS_APP_USING_MIC_NOTIFICATION_MESSAGE" desc="Message shown in notification when an app is using the microphone">
     <ph name="APP_NAME">$1<ex>Parallels Desktop</ex></ph> is using your microphone
   </message>
+  <message name="IDS_APP_USING_CAMERA_MIC_NOTIFICATION_MESSAGE" desc="Message shown in notification when an app is using both the camera and microphone">
+    <ph name="APP_NAME">$1<ex>Parallels Desktop</ex></ph> is using your camera and microphone
+  </message>
 
   <!-- Strings for Account Manager error screen -->
   <message name="IDS_ACCOUNT_MANAGER_SECONDARY_ACCOUNTS_DISABLED_TITLE" desc="Title for the Chrome OS Account Manager error screen when addition of secondary Google Accounts is disabled.">

chrome/app/chromeos_strings_grdp/IDS_APP_USING_CAMERA_MIC_NOTIFICATION_MESSAGE.png.sha1

+f65644bc7341e4420ba57bf9141974664506f461
\ No newline at end of file

chrome/app/chromeos_strings_grdp/IDS_CAMERA_MIC_NOTIFICATION_SOURCE.png.sha1

+f65644bc7341e4420ba57bf9141974664506f461
\ No newline at end of file

chrome/browser/chromeos/camera_mic/vm_camera_mic_manager.cc

@@ -25,6 +25,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "components/vector_icons/vector_icons.h"
+#include "content/public/browser/browser_thread.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/message_center/public/cpp/message_center_constants.h"
 #include "ui/message_center/public/cpp/notification.h"
@@ -48,33 +49,17 @@ void OpenPluginVmSettings(Profile* profile) {
                                 AppManagementEntryPoint::kNotificationPluginVm);
 }
 
-std::string GetNotificationId(VmCameraMicManager::VmType vm,
-                              VmCameraMicManager::DeviceType device) {
-  std::string id = kNotificationIdPrefix;
-
-  switch (vm) {
-    case VmCameraMicManager::VmType::kCrostiniVm:
-      id.append("-crostini");
-      break;
-    case VmCameraMicManager::VmType::kPluginVm:
-      id.append("-pluginvm");
-      break;
-  }
-
-  switch (device) {
-    case VmCameraMicManager::DeviceType::kCamera:
-      id.append("-camera");
-      break;
-    case VmCameraMicManager::DeviceType::kMic:
-      id.append("-mic");
-      break;
-  }
-
-  return id;
-}
-
 }  // namespace
 
+constexpr VmCameraMicManager::NotificationType
+    VmCameraMicManager::kNoNotification;
+constexpr VmCameraMicManager::NotificationType
+    VmCameraMicManager::kMicNotification;
+constexpr VmCameraMicManager::NotificationType
+    VmCameraMicManager::kCameraNotification;
+constexpr VmCameraMicManager::NotificationType
+    VmCameraMicManager::kCameraWithMicNotification;
+
 VmCameraMicManager* VmCameraMicManager::GetForProfile(Profile* profile) {
   return VmCameraMicManagerFactory::GetForProfile(profile);
 }
@@ -97,45 +82,66 @@ VmCameraMicManager::VmCameraMicManager(Profile* profile)
   DCHECK(ProfileHelper::IsPrimaryProfile(profile));
 
   for (VmType vm : {VmType::kCrostiniVm, VmType::kPluginVm}) {
-    for (DeviceType device : {DeviceType::kMic, DeviceType::kCamera}) {
-      active_map_[std::make_pair(vm, device)] = false;
-    }
+    notification_map_[vm] = {};
   }
 }
 
 VmCameraMicManager::~VmCameraMicManager() = default;
 
 void VmCameraMicManager::SetActive(VmType vm, DeviceType device, bool active) {
-  auto active_it = active_map_.find(std::make_pair(vm, device));
-  CHECK(active_it != active_map_.end());
-  if (active_it->second != active) {
-    if (!observer_timer_.IsRunning()) {
-      observer_timer_.Reset();
-    }
-    active_it->second = active;
-    if (active) {
-      OpenNotification(vm, device);
-    } else {
-      CloseNotification(vm, device);
-    }
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  NotificationType& notification_type = notification_map_[vm];
+  const NotificationType old_notification_type = notification_type;
+  notification_type.set(static_cast<size_t>(device), active);
+  if (old_notification_type == notification_type)
+    return;
+
+  if (!observer_timer_.IsRunning()) {
+    observer_timer_.Reset();
   }
-}
 
-bool VmCameraMicManager::GetActive(VmType vm, DeviceType device) const {
-  auto active_it = active_map_.find(std::make_pair(vm, device));
-  CHECK(active_it != active_map_.end());
-  return active_it->second;
+  // 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 (notification_type != kNoNotification) {
+    OpenNotification(vm, notification_type);
+  }
 }
 
 bool VmCameraMicManager::GetDeviceActive(DeviceType device) const {
-  for (auto& type_active : active_map_) {
-    if (type_active.first.second == device && type_active.second) {
+  for (const auto& vm_notification : notification_map_) {
+    const NotificationType& notification_type = vm_notification.second;
+    if (notification_type[static_cast<size_t>(device)]) {
       return true;
     }
   }
   return false;
 }
 
+bool VmCameraMicManager::IsNotificationActive(DeviceType device) const {
+  for (const auto& vm_notification : notification_map_) {
+    const NotificationType& notification_type = vm_notification.second;
+    switch (device) {
+      case DeviceType::kMic:
+        if (notification_type == kMicNotification) {
+          return true;
+        }
+        break;
+      case DeviceType::kCamera:
+        // Both the "camera only" and "camera and mic" notifications use the
+        // camera icon.
+        if (notification_type[static_cast<size_t>(DeviceType::kCamera)]) {
+          return true;
+        }
+        break;
+    }
+  }
+  return false;
+}
+
 void VmCameraMicManager::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
 }
@@ -150,29 +156,51 @@ void VmCameraMicManager::NotifyActiveChanged() {
   }
 }
 
-void VmCameraMicManager::OpenNotification(VmType vm, DeviceType device) {
+std::string VmCameraMicManager::GetNotificationId(VmType vm,
+                                                  NotificationType type) {
+  std::string id = kNotificationIdPrefix;
+
+  switch (vm) {
+    case VmType::kCrostiniVm:
+      id.append("-crostini");
+      break;
+    case VmType::kPluginVm:
+      id.append("-pluginvm");
+      break;
+  }
+
+  id.append(type.to_string());
+
+  return id;
+}
+
+void VmCameraMicManager::OpenNotification(VmType vm, NotificationType type) {
+  DCHECK_NE(type, kNoNotification);
   if (!base::FeatureList::IsEnabled(
           features::kVmCameraMicIndicatorsAndNotifications)) {
     return;
   }
 
-  int source_id;
+  // TODO(b/167491603): Rename this notifier id and remove useless ones.
+  const char* notifier_id = ash::kVmCameraNotifierId;
+
   const gfx::VectorIcon* source_icon = nullptr;
-  const char* notifier_id = nullptr;
+  int source_id;
   int message_id;
-  switch (device) {
-    case VmCameraMicManager::DeviceType::kCamera:
+  if (type[static_cast<size_t>(DeviceType::kCamera)]) {
+    source_icon = &::vector_icons::kVideocamIcon;
+    if (type[static_cast<size_t>(DeviceType::kMic)]) {
+      source_id = IDS_CAMERA_MIC_NOTIFICATION_SOURCE;
+      message_id = IDS_APP_USING_CAMERA_MIC_NOTIFICATION_MESSAGE;
+    } else {
       source_id = IDS_CAMERA_NOTIFICATION_SOURCE;
-      source_icon = &::vector_icons::kVideocamIcon;
-      notifier_id = ash::kVmCameraNotifierId;
       message_id = IDS_APP_USING_CAMERA_NOTIFICATION_MESSAGE;
-      break;
-    case VmCameraMicManager::DeviceType::kMic:
-      source_id = IDS_MIC_NOTIFICATION_SOURCE;
-      source_icon = &::vector_icons::kMicIcon;
-      notifier_id = ash::kVmMicNotifierId;
-      message_id = IDS_APP_USING_MIC_NOTIFICATION_MESSAGE;
-      break;
+    }
+  } else {
+    DCHECK_EQ(type, kMicNotification);
+    source_icon = &::vector_icons::kMicIcon;
+    source_id = IDS_MIC_NOTIFICATION_SOURCE;
+    message_id = IDS_APP_USING_MIC_NOTIFICATION_MESSAGE;
   }
 
   int app_name_id;
@@ -188,8 +216,6 @@ void VmCameraMicManager::OpenNotification(VmType vm, DeviceType device) {
       break;
   }
 
-  // TODO(b/167491603): check if NotificationPriority should be higher than
-  // default.
   message_center::RichNotificationData rich_notification_data;
   rich_notification_data.vector_small_image = source_icon;
   rich_notification_data.pinned = true;
@@ -197,7 +223,7 @@ void VmCameraMicManager::OpenNotification(VmType vm, DeviceType device) {
       l10n_util::GetStringUTF16(IDS_INTERNAL_APP_SETTINGS));
 
   message_center::Notification notification(
-      message_center::NOTIFICATION_TYPE_SIMPLE, GetNotificationId(vm, device),
+      message_center::NOTIFICATION_TYPE_SIMPLE, GetNotificationId(vm, type),
       /*title=*/
       l10n_util::GetStringFUTF16(message_id,
                                  l10n_util::GetStringUTF16(app_name_id)),
@@ -216,13 +242,14 @@ void VmCameraMicManager::OpenNotification(VmType vm, DeviceType device) {
       /*metadata=*/nullptr);
 }
 
-void VmCameraMicManager::CloseNotification(VmType vm, DeviceType device) {
+void VmCameraMicManager::CloseNotification(VmType vm, NotificationType type) {
+  DCHECK_NE(type, kNoNotification);
   if (!base::FeatureList::IsEnabled(
           features::kVmCameraMicIndicatorsAndNotifications)) {
     return;
   }
   NotificationDisplayService::GetForProfile(profile_)->Close(
-      NotificationHandler::Type::TRANSIENT, GetNotificationId(vm, device));
+      NotificationHandler::Type::TRANSIENT, GetNotificationId(vm, type));
 }
 
 VmCameraMicManager::VmNotificationObserver::VmNotificationObserver(

chrome/browser/chromeos/camera_mic/vm_camera_mic_manager.h

@@ -5,7 +5,9 @@
 #ifndef CHROME_BROWSER_CHROMEOS_CAMERA_MIC_VM_CAMERA_MIC_MANAGER_H_
 #define CHROME_BROWSER_CHROMEOS_CAMERA_MIC_VM_CAMERA_MIC_MANAGER_H_
 
+#include <bitset>
 #include <memory>
+
 #include "base/callback_forward.h"
 #include "base/containers/flat_map.h"
 #include "base/memory/weak_ptr.h"
@@ -32,6 +34,7 @@ class VmCameraMicManager : public KeyedService {
   enum class DeviceType {
     kMic,
     kCamera,
+    kMaxValue = kCamera,
   };
 
   class Observer : public base::CheckedObserver {
@@ -51,13 +54,37 @@ class VmCameraMicManager : public KeyedService {
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  void SetActive(VmType vm, DeviceType device, bool active);
-  bool GetActive(VmType vm, DeviceType device) const;
   // Return true if any of the VMs is using the device.
+  //
+  // TODO(b/167491603): Rename to `IsDeviceActive()` for consistency.
   bool GetDeviceActive(DeviceType device) const;
 
+  // When a VM is using both camera and mic, we only show a single "camera and
+  // mic" notification, which is considered a camera notification but not a mic
+  // notification because it uses the camera icon. So, if only "camera only" or
+  // "camera and mic" notifications are shown, this function returns true for
+  // `kCamera` but false for `kMic`. If a "mic only" notification is shown, this
+  // function returns true for `kMic`.
+  //
+  // TODO(b/167491603): We need to switch the indicator displaying logic to use
+  // this function instead.
+  bool IsNotificationActive(DeviceType device) const;
+
  private:
-  using ActiveMap = base::flat_map<std::pair<VmType, DeviceType>, bool>;
+  friend class VmCameraMicManagerTest;
+
+  using NotificationType =
+      std::bitset<static_cast<size_t>(DeviceType::kMaxValue) + 1>;
+  using NotificationMap = base::flat_map<VmType, NotificationType>;
+
+  static constexpr NotificationType kNoNotification{};
+  static constexpr NotificationType kMicNotification{
+      1 << static_cast<size_t>(DeviceType::kMic)};
+  static constexpr NotificationType kCameraNotification{
+      1 << static_cast<size_t>(DeviceType::kCamera)};
+  static constexpr NotificationType kCameraWithMicNotification{
+      (1 << static_cast<size_t>(DeviceType::kMic)) |
+      (1 << static_cast<size_t>(DeviceType::kCamera))};
 
   class VmNotificationObserver : public message_center::NotificationObserver {
    public:
@@ -79,15 +106,18 @@ class VmCameraMicManager : public KeyedService {
     base::WeakPtrFactory<VmNotificationObserver> weak_ptr_factory_{this};
   };
 
+  static std::string GetNotificationId(VmType vm, NotificationType type);
+
+  void SetActive(VmType vm, DeviceType device, bool active);
   void NotifyActiveChanged();
 
-  void OpenNotification(VmType vm, DeviceType device);
-  void CloseNotification(VmType vm, DeviceType device);
+  void OpenNotification(VmType vm, NotificationType type);
+  void CloseNotification(VmType vm, NotificationType type);
 
   Profile* const profile_;
   VmNotificationObserver crostini_vm_notification_observer_;
   VmNotificationObserver plugin_vm_notification_observer_;
-  ActiveMap active_map_;
+  NotificationMap notification_map_;
 
   base::RetainingOneShotTimer observer_timer_;
   base::ObserverList<Observer> observers_;