CameraMicTrayItemView: updates tooltip/a11y messages

... according to the comment from the UX deck: https://docs.google.com/presentation/d/1MWWejK-y2vnBN3Wg__Kd_LIiAcIF3VctzMowMI3wM4w/edit?disco=AAAAKz06f_U

For each VM, there are actually 3 types of notifications: "camera
(only)", "mic (only)", and "camera and mic". We have two types of tray
items: "camera", and "mic".

The "camera" tray item is displayed when there is a "camera" and/or
"camera and mic" notifications.

The non-trivial part of this cl is that the "camera" tray item now used
a different message depending on whether there is a "camera and mic"
notification or not. To do this, VmCameraMicManager now exposes the
exact type of the notification, and ash will pass this information down
to CameraMicTrayItemView.

Bug: b/167491603
Change-Id: I7acd1aa87d463930406b917da7dfd0ddbacf799b
Fixed: b/177270109
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622361
Commit-Queue: Jason Lin <lxj@google.com>
Reviewed-by: Daniel Ng <danielng@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842949}

ash/ash_strings.grd

@@ -2998,10 +2998,13 @@ Here are some things you can try to get started.
 
       <!-- For CameraMicTrayItemView -->
       <message name="IDS_ASH_CAMERA_MIC_VM_USING_CAMERA" desc="Tooltip message shown at the systray camera indicator when a VM is using the camera">
-        A virtual machine is using your camera
+        An application is using your camera
+      </message>
+      <message name="IDS_ASH_CAMERA_MIC_VM_USING_CAMERA_AND_MIC" desc="Tooltip message shown at the systray camera indicator when a VM is using both the camera and mic">
+        An application is using your camera and microphone
       </message>
       <message name="IDS_ASH_CAMERA_MIC_VM_USING_MIC" desc="Tooltip message shown at the systray microphone indicator when a VM is using the microphone">
-        A virtual machine is using your microphone
+        An application is using your microphone
       </message>
 
       <message name="IDS_ASH_MESSAGE_CENTER_UNLOCK_TO_PERFORM_ACTION" desc="The short message to encourage user to unlock the device so that Chrome OS can perform the notification action selected by user after unlocking.">

ash/ash_strings_grd/IDS_ASH_CAMERA_MIC_VM_USING_CAMERA.png.sha1

-d591db48d6c57816890d85213d98f10f6c322a6b
\ No newline at end of file
+27e7bb4609c6891d64bc47c2e232c6d410634d7e
\ No newline at end of file

ash/ash_strings_grd/IDS_ASH_CAMERA_MIC_VM_USING_CAMERA_AND_MIC.png.sha1

+b5feac7cdbfe35bc8a2bc68a9d41106327310574
\ No newline at end of file

ash/ash_strings_grd/IDS_ASH_CAMERA_MIC_VM_USING_MIC.png.sha1

-22591a040bd7307c93d9a3c760ed8c976a0b6e22
\ No newline at end of file
+aba716d721efb63bb24f6bd83a3f8d9c609556a4
\ No newline at end of file

ash/media/media_controller_impl.cc

@@ -93,9 +93,10 @@ void MediaControllerImpl::NotifyCaptureState(
 }
 
 void MediaControllerImpl::NotifyVmMediaNotificationState(bool camera,
-                                                         bool mic) {
+                                                         bool mic,
+                                                         bool camera_and_mic) {
   for (auto& observer : observers_)
-    observer.OnVmMediaNotificationChanged(camera, mic);
+    observer.OnVmMediaNotificationChanged(camera, mic, camera_and_mic);
 }
 
 void MediaControllerImpl::HandleMediaPlayPause() {

ash/media/media_controller_impl.h

@@ -26,8 +26,13 @@ class MediaCaptureObserver {
   // Called when media capture state has changed.
   virtual void OnMediaCaptureChanged(
       const base::flat_map<AccountId, MediaCaptureState>& capture_states) {}
-  // Called when a VM's media notifications have changed.
-  virtual void OnVmMediaNotificationChanged(bool camera, bool mic) {}
+  // Called when VMs' media capture notifications change. Each VM can have 0 or
+  // 1 media notification. It can either be a "camera", "mic", or "camera and
+  // mic" notification. Each of the argument is true if a notification of the
+  // corresponding type is active.
+  virtual void OnVmMediaNotificationChanged(bool camera,
+                                            bool mic,
+                                            bool camera_and_mic) {}
 
  protected:
   virtual ~MediaCaptureObserver() {}
@@ -57,7 +62,9 @@ class ASH_EXPORT MediaControllerImpl
   void SetForceMediaClientKeyHandling(bool enabled) override;
   void NotifyCaptureState(const base::flat_map<AccountId, MediaCaptureState>&
                               capture_states) override;
-  void NotifyVmMediaNotificationState(bool camera, bool mic) override;
+  void NotifyVmMediaNotificationState(bool camera,
+                                      bool mic,
+                                      bool camera_and_mic) override;
 
   // If media session accelerators are enabled then these methods will use the
   // media session service to control playback. Otherwise it will forward to

ash/public/cpp/media_controller.h

@@ -41,9 +41,16 @@ class ASH_PUBLIC_EXPORT MediaController {
   // MediaCaptureState representing every user's state.
   virtual void NotifyCaptureState(
       const base::flat_map<AccountId, MediaCaptureState>& capture_states) = 0;
-  // Called when a VM's media notifications change. There is no `AccountId` in
-  // the argument because only the primary account/profile can launch a VM.
-  virtual void NotifyVmMediaNotificationState(bool camera, bool mic) = 0;
+  // Called when VMs' media capture notifications change. Each VM can have 0 or
+  // 1 media notification. It can either be a "camera", "mic", or "camera and
+  // mic" notification. Each of the argument is true if a notification of the
+  // corresponding type is active.
+  //
+  // There is no `AccountId` in the argument because only the primary
+  // account/profile can launch a VM.
+  virtual void NotifyVmMediaNotificationState(bool camera,
+                                              bool mic,
+                                              bool camera_and_mic) = 0;
 
  protected:
   MediaController();

ash/system/unified/camera_mic_tray_item_view.cc

@@ -59,10 +59,13 @@ CameraMicTrayItemView::~CameraMicTrayItemView() {
 }
 
 void CameraMicTrayItemView::OnVmMediaNotificationChanged(bool camera,
-                                                         bool mic) {
+                                                         bool mic,
+                                                         bool camera_and_mic) {
   switch (type_) {
     case Type::kCamera:
-      active_ = camera;
+      active_ = camera || camera_and_mic;
+      with_mic_ = camera_and_mic;
+      FetchMessage();
       break;
     case Type::kMic:
       active_ = mic;
@@ -110,7 +113,9 @@ void CameraMicTrayItemView::HandleLocaleChange() {
 void CameraMicTrayItemView::FetchMessage() {
   switch (type_) {
     case Type::kCamera:
-      message_ = l10n_util::GetStringUTF16(IDS_ASH_CAMERA_MIC_VM_USING_CAMERA);
+      message_ = l10n_util::GetStringUTF16(
+          with_mic_ ? IDS_ASH_CAMERA_MIC_VM_USING_CAMERA_AND_MIC
+                    : IDS_ASH_CAMERA_MIC_VM_USING_CAMERA);
       break;
     case Type::kMic:
       message_ = l10n_util::GetStringUTF16(IDS_ASH_CAMERA_MIC_VM_USING_MIC);

ash/system/unified/camera_mic_tray_item_view.h

@@ -43,7 +43,9 @@ class ASH_EXPORT CameraMicTrayItemView : public TrayItemView,
   void HandleLocaleChange() override;
 
   // MediaCaptureObserver:
-  void OnVmMediaNotificationChanged(bool camera, bool mic) override;
+  void OnVmMediaNotificationChanged(bool camera,
+                                    bool mic,
+                                    bool camera_and_mic) override;
 
  private:
   void Update();
@@ -51,6 +53,7 @@ class ASH_EXPORT CameraMicTrayItemView : public TrayItemView,
 
   const Type type_;
   bool active_ = false;
+  bool with_mic_ = false;  // Only for `type_ == kCamera`.
   bool is_primary_session_ = false;
   base::string16 message_;
 };

ash/system/unified/camera_mic_tray_item_view_unittest.cc

@@ -8,10 +8,12 @@
 #include <utility>
 
 #include "ash/public/cpp/media_controller.h"
+#include "ash/strings/grit/ash_strings.h"
 #include "ash/test/ash_test_base.h"
 #include "base/test/scoped_feature_list.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace ash {
 
@@ -19,17 +21,15 @@ namespace {
 using Type = CameraMicTrayItemView::Type;
 }  // namespace
 
-class CameraMicTrayItemViewTest : public AshTestBase,
-                                  public testing::WithParamInterface<Type> {
+class BaseCameraMicTrayItemViewTest : public AshTestBase {
  public:
-  // AshTestBase:
-  void SetUp() override {
+  void SetUpWithType(Type type) {
     scoped_feature_list_.InitAndEnableFeature(
         chromeos::features::kVmCameraMicIndicatorsAndNotifications);
     AshTestBase::SetUp();
 
     camera_mic_tray_item_view_ =
-        std::make_unique<CameraMicTrayItemView>(GetPrimaryShelf(), GetParam());
+        std::make_unique<CameraMicTrayItemView>(GetPrimaryShelf(), type);
 
     // Relogin to make sure `OnActiveUserSessionChanged` is triggered.
     ClearLogin();
@@ -46,30 +46,59 @@ class CameraMicTrayItemViewTest : public AshTestBase,
   std::unique_ptr<CameraMicTrayItemView> camera_mic_tray_item_view_;
 };
 
-TEST_P(CameraMicTrayItemViewTest, OnVmMediaNotificationChanged) {
+class CameraMicTrayItemViewTest : public BaseCameraMicTrayItemViewTest,
+                                  public testing::WithParamInterface<Type> {
+ public:
+  // AshTestBase:
+  void SetUp() override { SetUpWithType(GetParam()); }
+};
+
+TEST_P(CameraMicTrayItemViewTest, GetVisible) {
   Type type = GetParam();
   EXPECT_FALSE(camera_mic_tray_item_view_->GetVisible());
 
-  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(/*camera=*/true,
-                                                           /*mic=*/false);
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/true,
+      /*mic=*/false,
+      /*camera_and_mic=*/false);
+  EXPECT_EQ(camera_mic_tray_item_view_->GetVisible(), type == Type::kCamera);
+
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/false,
+      /*mic=*/false,
+      /*camera_and_mic=*/true);
   EXPECT_EQ(camera_mic_tray_item_view_->GetVisible(), type == Type::kCamera);
 
-  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(/*camera=*/false,
-                                                           /*mic=*/true);
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/false,
+      /*mic=*/true,
+      /*camera_and_mic=*/false);
   EXPECT_EQ(camera_mic_tray_item_view_->GetVisible(), type == Type::kMic);
 
-  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(/*camera=*/true,
-                                                           /*mic=*/true);
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/true,
+      /*mic=*/true,
+      /*camera_and_mic=*/false);
+  EXPECT_TRUE(camera_mic_tray_item_view_->GetVisible());
+
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/true,
+      /*mic=*/true,
+      /*camera_and_mic=*/true);
   EXPECT_TRUE(camera_mic_tray_item_view_->GetVisible());
 
-  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(/*camera=*/false,
-                                                           /*mic=*/false);
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/false,
+      /*mic=*/false,
+      /*camera_and_mic=*/false);
   EXPECT_FALSE(camera_mic_tray_item_view_->GetVisible());
 }
 
 TEST_P(CameraMicTrayItemViewTest, HideForNonPrimaryUser) {
-  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(/*camera=*/true,
-                                                           /*mic=*/true);
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/true,
+      /*mic=*/true,
+      /*camera_and_mic=*/true);
   EXPECT_TRUE(camera_mic_tray_item_view_->GetVisible());
 
   SimulateUserLogin("user2@test.com");
@@ -80,4 +109,36 @@ INSTANTIATE_TEST_SUITE_P(All,
                          CameraMicTrayItemViewTest,
                          testing::Values(Type::kCamera, Type::kMic));
 
+// For testing that the camera tray item switch the message depending on whether
+// the "camera and mic" notification is active.
+class CameraMicTrayItemViewMessageTest : public BaseCameraMicTrayItemViewTest {
+  // AshTestBase:
+  void SetUp() override { SetUpWithType(Type::kCamera); }
+};
+
+TEST_F(CameraMicTrayItemViewMessageTest, Message) {
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/true,
+      /*mic=*/false,
+      /*camera_and_mic=*/false);
+  EXPECT_EQ(camera_mic_tray_item_view_->GetAccessibleNameString(),
+            l10n_util::GetStringUTF16(IDS_ASH_CAMERA_MIC_VM_USING_CAMERA));
+
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/false,
+      /*mic=*/false,
+      /*camera_and_mic=*/true);
+  EXPECT_EQ(
+      camera_mic_tray_item_view_->GetAccessibleNameString(),
+      l10n_util::GetStringUTF16(IDS_ASH_CAMERA_MIC_VM_USING_CAMERA_AND_MIC));
+
+  camera_mic_tray_item_view_->OnVmMediaNotificationChanged(
+      /*camera=*/true,
+      /*mic=*/false,
+      /*camera_and_mic=*/true);
+  EXPECT_EQ(
+      camera_mic_tray_item_view_->GetAccessibleNameString(),
+      l10n_util::GetStringUTF16(IDS_ASH_CAMERA_MIC_VM_USING_CAMERA_AND_MIC));
+}
+
 }  // namespace ash

chrome/browser/chromeos/camera_mic/vm_camera_mic_manager.cc

@@ -62,7 +62,7 @@ constexpr VmCameraMicManager::NotificationType
 constexpr VmCameraMicManager::NotificationType
     VmCameraMicManager::kCameraNotification;
 constexpr VmCameraMicManager::NotificationType
-    VmCameraMicManager::kCameraWithMicNotification;
+    VmCameraMicManager::kCameraAndMicNotification;
 
 VmCameraMicManager* VmCameraMicManager::Get() {
   static base::NoDestructor<VmCameraMicManager> manager;
@@ -169,23 +169,11 @@ bool VmCameraMicManager::IsDeviceActive(DeviceType device) const {
   return false;
 }
 
-bool VmCameraMicManager::IsNotificationActive(DeviceType device) const {
+bool VmCameraMicManager::IsNotificationActive(
+    NotificationType notification) const {
   for (const auto& vm_info : vm_info_map_) {
-    const NotificationType& notification_type =
-        vm_info.second.notification_type();
-    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;
+    if (vm_info.second.notification_type() == notification) {
+      return true;
     }
   }
   return false;

chrome/browser/chromeos/camera_mic/vm_camera_mic_manager.h

@@ -43,6 +43,16 @@ class VmCameraMicManager : public media::CameraActiveClientObserver,
     kMaxValue = kCamera,
   };
 
+  using NotificationType =
+      std::bitset<static_cast<size_t>(DeviceType::kMaxValue) + 1>;
+  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 kCameraAndMicNotification{
+      (1 << static_cast<size_t>(DeviceType::kMic)) |
+      (1 << static_cast<size_t>(DeviceType::kCamera))};
+
   class Observer : public base::CheckedObserver {
    public:
     virtual void OnVmCameraMicActiveChanged(VmCameraMicManager*) {}
@@ -64,27 +74,13 @@ class VmCameraMicManager : public media::CameraActiveClientObserver,
   // Return true if any of the VMs is using the device. Note that if the camera
   // privacy switch is on, this always returns false for `kCamera`.
   bool IsDeviceActive(DeviceType device) const;
+  // Return true if any of the VMs is displaying the `notification`.
+  bool IsNotificationActive(NotificationType notification) 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`.
-  bool IsNotificationActive(DeviceType device) const;
  private:
   friend class VmCameraMicManagerTest;
 
-  using NotificationType =
-      std::bitset<static_cast<size_t>(DeviceType::kMaxValue) + 1>;
   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 VmInfo {
    public:

chrome/browser/ui/ash/media_client_impl.cc

@@ -313,8 +313,12 @@ void MediaClientImpl::OnVmCameraMicActiveChanged(
     vm_media_capture_state_ |= MediaCaptureState::kAudio;
 
   media_controller_->NotifyVmMediaNotificationState(
-      manager->IsNotificationActive(DeviceType::kCamera),
-      manager->IsNotificationActive(DeviceType::kMic));
+      manager->IsNotificationActive(
+          chromeos::VmCameraMicManager::kCameraNotification),
+      manager->IsNotificationActive(
+          chromeos::VmCameraMicManager::kMicNotification),
+      manager->IsNotificationActive(
+          chromeos::VmCameraMicManager::kCameraAndMicNotification));
 }
 
 void MediaClientImpl::OnCameraPrivacySwitchStatusChanged(

chrome/browser/ui/ash/media_client_impl_unittest.cc

@@ -30,7 +30,9 @@ class TestMediaController : public ash::MediaController {
       const base::flat_map<AccountId, ash::MediaCaptureState>& capture_states)
       override {}
 
-  void NotifyVmMediaNotificationState(bool camera, bool mic) override {}
+  void NotifyVmMediaNotificationState(bool camera,
+                                      bool mic,
+                                      bool camera_and_mic) override {}
 
   bool force_media_client_key_handling() const {
     return force_media_client_key_handling_;