Show media indicators to the right user in the list.

Changes the RequestCaptureState() to send media capture state bundled
with account id, so that receiver can identify which one represents
which user's status.

Test: manual
Test: ash_unittest --gtest_filter=MultiProfileMediaTrayItemTest.*
Bug: 879269
Change-Id: I4e808d7e88d7904673869f282b41d7c37ecc8b12
Reviewed-on: https://chromium-review.googlesource.com/1203756
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589822}

ash/media_controller.cc

@@ -27,7 +27,7 @@ void MediaController::SetClient(mojom::MediaClientAssociatedPtrInfo client) {
 }
 
 void MediaController::NotifyCaptureState(
-    const std::vector<mojom::MediaCaptureState>& capture_states) {
+    const base::flat_map<AccountId, mojom::MediaCaptureState>& capture_states) {
   for (auto& observer : observers_)
     observer.OnMediaCaptureChanged(capture_states);
 }

ash/media_controller.h

@@ -8,6 +8,7 @@
 #include "ash/public/interfaces/media.mojom.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "components/account_id/interfaces/account_id.mojom.h"
 #include "mojo/public/cpp/bindings/associated_binding.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 
@@ -18,7 +19,8 @@ class MediaCaptureObserver {
  public:
   // Called when media capture state has changed.
   virtual void OnMediaCaptureChanged(
-      const std::vector<mojom::MediaCaptureState>& capture_states) = 0;
+      const base::flat_map<AccountId, mojom::MediaCaptureState>&
+          capture_states) = 0;
 
  protected:
   virtual ~MediaCaptureObserver() {}
@@ -40,7 +42,8 @@ class MediaController : public mojom::MediaController {
   // mojom::MediaController:
   void SetClient(mojom::MediaClientAssociatedPtrInfo client) override;
   void NotifyCaptureState(
-      const std::vector<mojom::MediaCaptureState>& capture_states) override;
+      const base::flat_map<AccountId, mojom::MediaCaptureState>& capture_states)
+      override;
 
   // Methods that forward to |client_|.
   void HandleMediaNextTrack();

ash/public/interfaces/media.mojom

@@ -4,6 +4,8 @@
 
 module ash.mojom;
 
+import "components/account_id/interfaces/account_id.mojom";
+
 // Describes whether media is currently being captured.
 enum MediaCaptureState {
   NONE = 0,
@@ -19,11 +21,10 @@ interface MediaController {
   SetClient(associated MediaClient client);
 
   // Called when the media capture state changes on the client, or in response
-  // to a RequestCaptureState() request. Returns an array of MediaCaptureState
-  // by index of the shell content. (These indexes are unstable, but are
-  // unlikely to change in practice, see comments in chrome's MediaClient::
-  // RequestCaptureState()).
-  NotifyCaptureState(array<MediaCaptureState> state);
+  // to a RequestCaptureState() request. Returns a map from AccountId to
+  // MediaCaptureState representing every user's state.
+  NotifyCaptureState(map<signin.mojom.AccountId,
+      MediaCaptureState> capture_states);
 };
 
 // This delegate allows the UI code in ash to forward UI commands.

ash/system/media_security/multi_profile_media_tray_item.cc

@@ -7,6 +7,7 @@
 #include "ash/media_controller.h"
 #include "ash/public/cpp/ash_view_ids.h"
 #include "ash/resources/vector_icons/vector_icons.h"
+#include "ash/session/session_controller.h"
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray_notifier.h"
 #include "ash/system/tray/tray_constants.h"
@@ -37,10 +38,17 @@ class MultiProfileMediaTrayView : public TrayItemView,
 
   // MediaCaptureObserver:
   void OnMediaCaptureChanged(
-      const std::vector<mojom::MediaCaptureState>& capture_states) override {
+      const base::flat_map<AccountId, mojom::MediaCaptureState>& capture_states)
+      override {
     // The user at 0 is the current active desktop user.
-    for (size_t index = 1; index < capture_states.size(); ++index) {
-      if (capture_states[index] != mojom::MediaCaptureState::NONE) {
+    const auto& current_account_id = Shell::Get()
+                                         ->session_controller()
+                                         ->GetUserSession(0)
+                                         ->user_info->account_id;
+    for (const auto& entry : capture_states) {
+      if (entry.first == current_account_id)
+        continue;
+      if (entry.second != mojom::MediaCaptureState::NONE) {
         SetVisible(true);
         return;
       }

ash/system/media_security/multi_profile_media_tray_item_unittest.cc

@@ -28,10 +28,13 @@ class MultiProfileMediaTrayItemTest : public AshTestBase {
   void SetMediaCaptureState(mojom::MediaCaptureState state) {
     // Create the fake update.
     SessionController* controller = Shell::Get()->session_controller();
-    std::vector<mojom::MediaCaptureState> v;
-    for (int i = 0; i < controller->NumberOfLoggedInUsers(); ++i)
-      v.push_back(state);
-    Shell::Get()->media_controller()->NotifyCaptureState(v);
+    base::flat_map<AccountId, mojom::MediaCaptureState> capture_states;
+    for (int i = 0; i < controller->NumberOfLoggedInUsers(); ++i) {
+      capture_states.emplace(
+          controller->GetUserSession(i)->user_info->account_id, state);
+    }
+    Shell::Get()->media_controller()->NotifyCaptureState(
+        std::move(capture_states));
   }
 
  private:

ash/system/unified/user_chooser_view.cc

@@ -269,12 +269,18 @@ UserChooserView::~UserChooserView() {
 }
 
 void UserChooserView::OnMediaCaptureChanged(
-    const std::vector<mojom::MediaCaptureState>& capture_states) {
+    const base::flat_map<AccountId, mojom::MediaCaptureState>& capture_states) {
   if (user_item_buttons_.size() != capture_states.size())
     return;
 
-  for (size_t i = 0; i < user_item_buttons_.size(); ++i)
-    user_item_buttons_[i]->SetCaptureState(capture_states[i]);
+  for (size_t i = 0; i < user_item_buttons_.size(); ++i) {
+    const mojom::UserSession* const user_session =
+        Shell::Get()->session_controller()->GetUserSession(i);
+    auto matched = capture_states.find(user_session->user_info->account_id);
+    if (matched != capture_states.end()) {
+      user_item_buttons_[i]->SetCaptureState(matched->second);
+    }
+  }
 }
 
 }  // namespace ash

ash/system/unified/user_chooser_view.h

@@ -31,7 +31,7 @@ class UserItemButton : public views::Button, public views::ButtonListener {
                  bool has_close_button);
   ~UserItemButton() override = default;
 
-  void SetCaptureState(mojom::MediaCaptureState capture_state);
+  void SetCaptureState(mojom::MediaCaptureState capture_states);
 
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
@@ -52,7 +52,8 @@ class UserChooserView : public views::View, public MediaCaptureObserver {
 
   // MediaCaptureObserver:
   void OnMediaCaptureChanged(
-      const std::vector<mojom::MediaCaptureState>& capture_states) override;
+      const base::flat_map<AccountId, mojom::MediaCaptureState>& capture_states)
+      override;
 
  private:
   std::vector<UserItemButton*> user_item_buttons_;

ash/system/user/user_card_view.cc

@@ -342,16 +342,24 @@ void UserCardView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
 }
 
 void UserCardView::OnMediaCaptureChanged(
-    const std::vector<mojom::MediaCaptureState>& capture_states) {
+    const base::flat_map<AccountId, mojom::MediaCaptureState>& capture_states) {
   mojom::MediaCaptureState state = mojom::MediaCaptureState::NONE;
+  auto account_id = Shell::Get()
+                        ->session_controller()
+                        ->GetUserSession(user_index_)
+                        ->user_info->account_id;
   if (is_active_user()) {
     int cumulative_state = 0;
     // The active user reports media capture states for all /other/ users.
-    for (size_t i = 1; i < capture_states.size(); ++i)
-      cumulative_state |= static_cast<int32_t>(capture_states[i]);
+    for (const auto& entry : capture_states) {
+      if (entry.first == account_id)
+        continue;
+      cumulative_state |= static_cast<int32_t>(entry.second);
+    }
     state = static_cast<mojom::MediaCaptureState>(cumulative_state);
   } else {
-    state = capture_states[user_index_];
+    auto matched = capture_states.find(account_id);
+    state = matched->second;
   }
 
   int res_id = 0;

ash/system/user/user_card_view.h

@@ -37,7 +37,8 @@ class UserCardView : public views::View, public MediaCaptureObserver {
 
   // MediaCaptureObserver:
   void OnMediaCaptureChanged(
-      const std::vector<mojom::MediaCaptureState>& capture_states) override;
+      const base::flat_map<AccountId, mojom::MediaCaptureState>& capture_states)
+      override;
 
  private:
   // Creates the content for the public mode.

chrome/browser/ui/ash/media_client.cc

@@ -191,20 +191,14 @@ void MediaClient::HandleMediaPrevTrack() {
 }
 
 void MediaClient::RequestCaptureState() {
-  // TODO(erg): Ash doesn't have stable user indexes. Given the asynchronous
-  // nature of sending messages over mojo pipes, this could theoretically cause
-  // bad data, as one side thinks the vector is [user1, user2] while the other
-  // thinks [user2, user1]. However, since parts of this system are already
-  // asynchronous (see OnRequestUpdate's PostTask()), we're not worrying about
-  // this right now.
-  std::vector<MediaCaptureState> state;
+  base::flat_map<AccountId, MediaCaptureState> capture_states;
   for (user_manager::User* user :
-       user_manager::UserManager::Get()->GetLoggedInUsers()) {
-    state.push_back(GetMediaCaptureStateOfAllWebContents(
-        chromeos::ProfileHelper::Get()->GetProfileByUser(user)));
+       user_manager::UserManager::Get()->GetLRULoggedInUsers()) {
+    capture_states[user->GetAccountId()] = GetMediaCaptureStateOfAllWebContents(
+        chromeos::ProfileHelper::Get()->GetProfileByUser(user));
   }
 
-  media_controller_->NotifyCaptureState(std::move(state));
+  media_controller_->NotifyCaptureState(std::move(capture_states));
 }
 
 void MediaClient::SuspendMediaSessions() {