[Reland] GD: Don't always auto select a camera

Original CL was reverted because of a failing test that was
not caught by the CQ (since ash_unittests were removed from CQ,
see b/333572800).

This CL relands the CL with a fix.

In Game-Dashboard-initiated sessions, we auto-select the first
available camera (if none is currently selected). However, we
should stop doing that once a user makes an explicit camera
selection. All subsequent sessions should just continue using the
system-wide most recent camera selection.

Bug: b/324964571
Test: Manually, unit tests.
Change-Id: I632288b157b1964704060971e7700bc2062230fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5464172
Reviewed-by: Prameet Shah <phshah@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1289353}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5463216
Cr-Commit-Position: refs/heads/main@{#1289727}

ash/capture_mode/capture_mode_behavior.cc

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "ash/capture_mode/base_capture_mode_session.h"
+#include "ash/capture_mode/capture_mode_camera_controller.h"
 #include "ash/capture_mode/capture_mode_constants.h"
 #include "ash/capture_mode/capture_mode_controller.h"
 #include "ash/capture_mode/capture_mode_metrics.h"
@@ -230,7 +231,11 @@ class GameDashboardBehavior : public CaptureModeBehavior,
   bool ShouldDemoToolsSettingsBeIncluded() const override { return false; }
   bool ShouldGifBeSupported() const override { return false; }
   bool ShouldShowUserNudge() const override { return false; }
-  bool ShouldAutoSelectFirstCamera() const override { return true; }
+  bool ShouldAutoSelectFirstCamera() const override {
+    return !CaptureModeController::Get()
+                ->camera_controller()
+                ->did_user_ever_change_camera();
+  }
 
   std::unique_ptr<CaptureModeBarView> CreateCaptureModeBarView() override {
     return std::make_unique<GameCaptureBarView>();

ash/capture_mode/capture_mode_camera_controller.cc

@@ -455,7 +455,8 @@ std::string CaptureModeCameraController::GetDisplayNameOfSelectedCamera()
   return std::string();
 }
 
-void CaptureModeCameraController::SetSelectedCamera(CameraId camera_id) {
+void CaptureModeCameraController::SetSelectedCamera(CameraId camera_id,
+                                                    bool by_user) {
   // When cameras are disabled by policy, we don't allow any camera selection.
   if (IsCameraDisabledByPolicy()) {
     LOG(WARNING) << "Camera is disabled by policy. Selecting camera: "
@@ -466,6 +467,13 @@ void CaptureModeCameraController::SetSelectedCamera(CameraId camera_id) {
   if (selected_camera_ == camera_id)
     return;
 
+  did_user_ever_change_camera_ |= by_user;
+
+  // If camera auto-selection is on, and a camera change happened (either by
+  // user or due to disconnection), calling `MaybeRevertAutoCameraSelection()`
+  // should be a no-op, and the camera should not be restored to off.
+  did_make_camera_auto_selection_ = false;
+
   selected_camera_ = std::move(camera_id);
   camera_reconnect_timer_.Stop();
 

ash/capture_mode/capture_mode_camera_controller.h

@@ -163,6 +163,9 @@ class ASH_EXPORT CaptureModeCameraController
   bool is_camera_preview_collapsed() const {
     return is_camera_preview_collapsed_;
   }
+  bool did_user_ever_change_camera() const {
+    return did_user_ever_change_camera_;
+  }
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
@@ -185,8 +188,9 @@ class ASH_EXPORT CaptureModeCameraController
 
   // Sets the currently selected camera to the whose ID is the given
   // `camera_id`. If `camera_id` is invalid (see CameraId::is_valid()), this
-  // clears the selected camera.
-  void SetSelectedCamera(CameraId camera_id);
+  // clears the selected camera. `by_user` is true if the selection was made
+  // explicitly by the user, false otherwise.
+  void SetSelectedCamera(CameraId camera_id, bool by_user = false);
 
   // Sets `should_show_preview_` to the given `value`, and refreshes the state
   // of the camera preview.
@@ -426,6 +430,10 @@ class ASH_EXPORT CaptureModeCameraController
   // selection.
   bool did_make_camera_auto_selection_ = false;
 
+  // True if the user ever made an explicit camera selection (i.e. from the
+  // capture mode settings menu).
+  bool did_user_ever_change_camera_ = false;
+
   base::WeakPtrFactory<CaptureModeCameraController> weak_ptr_factory_{this};
 };
 

ash/capture_mode/capture_mode_game_dashboard_unittests.cc

@@ -702,6 +702,11 @@ TEST_F(GameDashboardCaptureModeTest, GameCaptureModeRecordInstantlyTest) {
                 ? AudioRecordingMode::kSystemAndMicrophone
                 : AudioRecordingMode::kMicrophone);
 
+  auto* camera_controller = controller->camera_controller();
+  ASSERT_TRUE(camera_controller->camera_preview_widget());
+  const CameraId camera_id(kDefaultCameraModelId, 1);
+  EXPECT_EQ(camera_id, camera_controller->selected_camera());
+
   // Update the audio recording mode and demo tools configs for the
   // game-dashboard initiated capture mode.
   controller->SetAudioRecordingMode(AudioRecordingMode::kOff);
@@ -728,13 +733,110 @@ TEST_F(GameDashboardCaptureModeTest, GameCaptureModeRecordInstantlyTest) {
 
   // Verify that selfie camera is visible and is parented correctly to the game
   // window.
-  const auto* camera_controller = controller->camera_controller();
   const auto* camera_preview_widget =
       camera_controller->camera_preview_widget();
   ASSERT_TRUE(camera_preview_widget);
   EXPECT_EQ(camera_preview_widget->GetNativeWindow()->parent(), game_window());
 }
 
+TEST_F(GameDashboardCaptureModeTest, UserTurnsOffCamera) {
+  AddDefaultCamera();
+
+  // By default, the first available camera should be auto selected.
+  auto* controller = StartGameCaptureModeSession();
+  auto* camera_controller = controller->camera_controller();
+  ASSERT_TRUE(camera_controller->camera_preview_widget());
+  const CameraId camera_id(kDefaultCameraModelId, 1);
+  EXPECT_EQ(camera_id, camera_controller->selected_camera());
+
+  // Now, open the settings menu, and select the "camera off" option.
+  LeftClickOn(GetSettingsButton());
+  CaptureModeSettingsTestApi test_api;
+  CaptureModeMenuGroup* camera_menu_group = test_api.GetCameraMenuGroup();
+  ASSERT_TRUE(camera_menu_group);
+  EXPECT_TRUE(camera_menu_group->GetVisible());
+  auto* off_option = test_api.GetCameraOption(kCameraOff);
+  EXPECT_TRUE(off_option);
+  LeftClickOn(off_option);
+
+  // No camera should be selected, and the preview widget should be closed.
+  EXPECT_FALSE(camera_controller->selected_camera().is_valid());
+  EXPECT_FALSE(camera_controller->camera_preview_widget());
+
+  // Close the session and start a new one. No camera should be auto selected.
+  controller->Stop();
+  StartGameCaptureModeSession();
+  EXPECT_FALSE(camera_controller->selected_camera().is_valid());
+  EXPECT_FALSE(camera_controller->camera_preview_widget());
+  controller->Stop();
+
+  // Start a new default session and select a camera, then end the session.
+  StartCaptureSession(CaptureModeSource::kFullscreen, CaptureModeType::kVideo);
+  camera_controller->SetSelectedCamera(camera_id);
+  EXPECT_TRUE(camera_controller->camera_preview_widget());
+  controller->Stop();
+
+  // When we start a new Game Dashboard session next, the camera selected from
+  // the previous default session will remain selected.
+  StartGameCaptureModeSession();
+  EXPECT_EQ(camera_id, camera_controller->selected_camera());
+  EXPECT_TRUE(camera_controller->camera_preview_widget());
+  controller->Stop();
+}
+
+TEST_F(GameDashboardCaptureModeTest, StartWithNoCamera) {
+  // Initially there's no camera connected, so the Game Dashboard auto camera
+  // selection won't work.
+  auto* controller = StartGameCaptureModeSession();
+  auto* camera_controller = controller->camera_controller();
+  EXPECT_FALSE(camera_controller->selected_camera().is_valid());
+  EXPECT_FALSE(camera_controller->camera_preview_widget());
+
+  // Now stop the current session, connect a camera, and start a new Game
+  // Dashboard session. This new session should be able to auto-select the
+  // camera.
+  controller->Stop();
+  AddDefaultCamera();
+  StartGameCaptureModeSession();
+  EXPECT_TRUE(camera_controller->camera_preview_widget());
+  const CameraId camera_id(kDefaultCameraModelId, 1);
+  EXPECT_EQ(camera_id, camera_controller->selected_camera());
+  controller->Stop();
+
+  // The next Game Dashboard session should still launch with a camera.
+  StartGameCaptureModeSession();
+  EXPECT_TRUE(camera_controller->camera_preview_widget());
+  EXPECT_EQ(camera_id, camera_controller->selected_camera());
+}
+
+TEST_F(GameDashboardCaptureModeTest, CameraAutoSelectionDisabledOnChange) {
+  const std::string device_id_1 = "/dev/video0";
+  const std::string display_name_1 = "Integrated Webcam";
+
+  const std::string device_id_2 = "/dev/video1";
+  const std::string display_name_2 = "Integrated Webcam 1";
+
+  AddFakeCamera(device_id_1, display_name_1, display_name_1);
+  AddFakeCamera(device_id_2, display_name_2, display_name_2);
+
+  // The first Game Dashboard session, the first camera will be auto selected.
+  auto* controller = StartGameCaptureModeSession();
+  auto* camera_controller = controller->camera_controller();
+  EXPECT_TRUE(camera_controller->camera_preview_widget());
+  const CameraId camera_id1(display_name_1, 1);
+  EXPECT_EQ(camera_id1, camera_controller->selected_camera());
+
+  // Now, simulate a change by the user to select a different camera while the
+  // session is still running.
+  const CameraId camera_id2(display_name_2, 1);
+  camera_controller->SetSelectedCamera(camera_id2);
+  EXPECT_EQ(camera_id2, camera_controller->selected_camera());
+
+  // Stop the session and expect that the camera remains selected.
+  controller->Stop();
+  EXPECT_EQ(camera_id2, camera_controller->selected_camera());
+}
+
 TEST_F(GameDashboardCaptureModeTest, NoDimmingOfGameDashboardWidgets) {
   auto* controller = CaptureModeController::Get();
   controller->StartRecordingInstantlyForGameDashboard(game_window());

ash/capture_mode/capture_mode_settings_view.cc

@@ -363,14 +363,14 @@ void CaptureModeSettingsView::OnOptionSelected(int option_id) const {
       controller->SetUsesDefaultCaptureFolder(false);
       break;
     case kCameraOff:
-      camera_controller->SetSelectedCamera(CameraId());
+      camera_controller->SetSelectedCamera(CameraId(), /*by_user=*/true);
       break;
     default:
       DCHECK(!camera_controller->IsCameraDisabledByPolicy());
       DCHECK_GE(option_id, kCameraDevicesBegin);
       const CameraId* camera_id = FindCameraIdByOptionId(option_id);
       DCHECK(camera_id);
-      camera_controller->SetSelectedCamera(*camera_id);
+      camera_controller->SetSelectedCamera(*camera_id, /*by_user=*/true);
       break;
   }
 }