capture_mode_polish: Clean up Selfie Cam feature and flag

Fixed: 1347276
Test: Existing tests
Change-Id: Icf48c83aa854ad085c4a7a7ed816e69bbede7bb6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3785433
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1028382}

ash/accelerators/accelerator_controller_impl.cc

@@ -432,8 +432,8 @@ bool CanHandleFocusCameraPreview() {
     return false;
 
   auto* camera_controller = controller->camera_controller();
-  auto* preview_widget =
-      camera_controller ? camera_controller->camera_preview_widget() : nullptr;
+  DCHECK(camera_controller);
+  auto* preview_widget = camera_controller->camera_preview_widget();
   return preview_widget && preview_widget->IsVisible();
 }
 

ash/ash_strings.grd

@@ -4099,9 +4099,6 @@ Here are some things you can try to get started.
       <message name="IDS_ASH_SCREEN_CAPTURE_BUTTON_DELETE" desc="The delete button label for the screen capture notification.">
         Delete
       </message>
-      <message name="IDS_ASH_SCREEN_CAPTURE_FOLDER_SELECTION_USER_NUDGE" desc="The message shown in a toast widget to nudge the user and alert them to check out the new settings that allow them to change where captured files are saved.">
-        You can now change where screen captures are saved
-      </message>
       <message name="IDS_ASH_SCREEN_CAPTURE_LABEL_FULLSCREEN_IMAGE_CAPTURE_CLAMSHELL" desc="The capture label message which shows in the middle of the screen when in fullscreen image capture mode in clamshell mode.">
         Click anywhere to capture full screen
       </message>

ash/capture_mode/capture_mode_camera_unittests.cc

@@ -25,7 +25,6 @@
 #include "ash/capture_mode/fake_folder_selection_dialog_factory.h"
 #include "ash/capture_mode/fake_video_source_provider.h"
 #include "ash/capture_mode/test_capture_mode_delegate.h"
-#include "ash/constants/ash_features.h"
 #include "ash/display/window_tree_host_manager.h"
 #include "ash/public/cpp/capture_mode/capture_mode_test_api.h"
 #include "ash/public/cpp/window_properties.h"
@@ -37,16 +36,11 @@
 #include "ash/system/unified/unified_system_tray.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/window_state.h"
-#include "ash/wm/wm_event.h"
-#include "base/bind.h"
-#include "base/files/file_path.h"
-#include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/system/system_monitor.h"
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
-#include "base/test/scoped_feature_list.h"
 #include "base/timer/timer.h"
 #include "cc/paint/skia_paint_canvas.h"
 #include "media/base/video_facing.h"
@@ -180,10 +174,7 @@ gfx::Rect GetTooSmallToFitCameraRegion() {
 
 class CaptureModeCameraTest : public AshTestBase {
  public:
-  CaptureModeCameraTest() {
-    scoped_feature_list_.InitAndEnableFeature(
-        features::kCaptureModeSelfieCamera);
-  }
+  CaptureModeCameraTest() = default;
   CaptureModeCameraTest(const CaptureModeCameraTest&) = delete;
   CaptureModeCameraTest& operator=(const CaptureModeCameraTest&) = delete;
   ~CaptureModeCameraTest() override = default;
@@ -388,7 +379,6 @@ class CaptureModeCameraTest : public AshTestBase {
   }
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<aura::Window> window_;
 };
 

ash/capture_mode/capture_mode_controller.cc

@@ -14,7 +14,6 @@
 #include "ash/capture_mode/capture_mode_util.h"
 #include "ash/constants/ash_features.h"
 #include "ash/constants/notifier_catalogs.h"
-#include "ash/display/window_tree_host_manager.h"
 #include "ash/projector/projector_controller_impl.h"
 #include "ash/public/cpp/capture_mode/recording_overlay_view.h"
 #include "ash/public/cpp/holding_space/holding_space_client.h"
@@ -26,7 +25,6 @@
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/message_center/message_view_factory.h"
-#include "ash/system/status_area_widget.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ -38,14 +36,11 @@
 #include "base/location.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/scoped_refptr.h"
-#include "base/metrics/histogram_functions.h"
 #include "base/notreached.h"
 #include "base/strings/stringprintf.h"
-#include "base/task/bind_post_task.h"
 #include "base/task/current_thread.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
-#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
@@ -105,13 +100,6 @@ constexpr char kCustomCapturePathPrefName[] =
 constexpr char kUsesDefaultCapturePathPrefName[] =
     "ash.capture_mode.uses_default_capture_path";
 
-// The name of a boolean pref that determines whether we can show the folder
-// selection user nudge. When this pref is false, it means that we showed the
-// nudge at some point and the user interacted with the capture mode session UI
-// in such a way that the nudge no longer needs to be displayed again.
-constexpr char kCanShowFolderSelectionNudge[] =
-    "ash.capture_mode.can_show_folder_selection_nudge";
-
 // The name of a boolean pref that determines whether we can show the selfie
 // camera user nudge. When this pref is false, it means that we showed the
 // nudge at some point and the user interacted with the capture mode session UI
@@ -373,19 +361,13 @@ base::FilePath GetTempDir() {
   return temp_dir;
 }
 
-std::unique_ptr<CaptureModeCameraController> MaybeCreateCameraController(
-    CaptureModeDelegate* delegate) {
-  if (!features::IsCaptureModeSelfieCameraEnabled())
-    return nullptr;
-  return std::make_unique<CaptureModeCameraController>(delegate);
-}
-
 }  // namespace
 
 CaptureModeController::CaptureModeController(
     std::unique_ptr<CaptureModeDelegate> delegate)
     : delegate_(std::move(delegate)),
-      camera_controller_(MaybeCreateCameraController(delegate_.get())),
+      camera_controller_(
+          std::make_unique<CaptureModeCameraController>(delegate_.get())),
       blocking_task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
           // A task priority of BEST_EFFORT is good enough for this runner,
           // since it's used for blocking file IO such as saving the screenshots
@@ -465,10 +447,7 @@ void CaptureModeController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
                                  /*default_value=*/base::FilePath());
   registry->RegisterBooleanPref(kUsesDefaultCapturePathPrefName,
                                 /*default_value=*/false);
-  const auto* pref_name = features::IsCaptureModeSelfieCameraEnabled()
-                              ? kCanShowCameraNudge
-                              : kCanShowFolderSelectionNudge;
-  registry->RegisterBooleanPref(pref_name, /*default_value=*/true);
+  registry->RegisterBooleanPref(kCanShowCameraNudge, /*default_value=*/true);
 }
 
 bool CaptureModeController::IsActive() const {
@@ -536,10 +515,8 @@ void CaptureModeController::SetUserCaptureRegion(const gfx::Rect& region,
   if (!user_capture_region_.IsEmpty() && by_user)
     last_capture_region_update_time_ = base::TimeTicks::Now();
 
-  if (camera_controller_ && !is_recording_in_progress() &&
-      source_ == CaptureModeSource::kRegion) {
+  if (!is_recording_in_progress() && source_ == CaptureModeSource::kRegion)
     camera_controller_->MaybeReparentPreviewWidget();
-  }
 }
 
 bool CaptureModeController::CanShowUserNudge() const {
@@ -568,17 +545,11 @@ bool CaptureModeController::CanShowUserNudge() const {
 
   auto* pref_service = session_controller->GetActivePrefService();
   DCHECK(pref_service);
-  const auto* pref_name = features::IsCaptureModeSelfieCameraEnabled()
-                              ? kCanShowCameraNudge
-                              : kCanShowFolderSelectionNudge;
-  return pref_service->GetBoolean(pref_name);
+  return pref_service->GetBoolean(kCanShowCameraNudge);
 }
 
 void CaptureModeController::DisableUserNudgeForever() {
-  const auto* pref_name = features::IsCaptureModeSelfieCameraEnabled()
-                              ? kCanShowCameraNudge
-                              : kCanShowFolderSelectionNudge;
-  GetActiveUserPrefService()->SetBoolean(pref_name, false);
+  GetActiveUserPrefService()->SetBoolean(kCanShowCameraNudge, false);
 }
 
 void CaptureModeController::SetUsesDefaultCaptureFolder(bool value) {
@@ -789,8 +760,7 @@ void CaptureModeController::OnActiveUserSessionChanged(
     const AccountId& account_id) {
   EndSessionOrRecording(EndRecordingReason::kActiveUserChange);
 
-  if (camera_controller_)
-    camera_controller_->OnActiveUserSessionChanged();
+  camera_controller_->OnActiveUserSessionChanged();
 
   // Remove the previous notification when switching to another user.
   auto* message_center = message_center::MessageCenter::Get();
@@ -808,8 +778,7 @@ void CaptureModeController::OnChromeTerminating() {
   // Order here matters. We may shutdown while a session with a camera is active
   // before recording starts, we need to inform the camera controller first to
   // destroy the camera preview first.
-  if (camera_controller_)
-    camera_controller_->OnShuttingDown();
+  camera_controller_->OnShuttingDown();
   EndSessionOrRecording(EndRecordingReason::kShuttingDown);
 }
 
@@ -1656,8 +1625,7 @@ void CaptureModeController::OnDlpRestrictionCheckedAtSessionInit(
       std::make_unique<CaptureModeSession>(this, for_projector);
   capture_mode_session_->Initialize();
 
-  if (camera_controller_)
-    camera_controller_->OnCaptureSessionStarted();
+  camera_controller_->OnCaptureSessionStarted();
 }
 
 void CaptureModeController::OnDlpRestrictionCheckedAtVideoEnd(

ash/capture_mode/capture_mode_controller.h

@@ -460,8 +460,7 @@ class ASH_EXPORT CaptureModeController
 
   std::unique_ptr<CaptureModeDelegate> delegate_;
 
-  // Controls the selfie camera feature of capture mode. This is only available
-  // when the feature `kCaptureModeSelfieCamera` is enabled.
+  // Controls the selfie camera feature of capture mode.
   std::unique_ptr<CaptureModeCameraController> camera_controller_;
 
   CaptureModeType type_ = CaptureModeType::kImage;

ash/capture_mode/capture_mode_session.cc

@@ -330,9 +330,9 @@ void UpdateFloatingPanelBoundsIfNeeded() {
 }
 
 views::Widget* GetCameraPreviewWidget() {
-  auto* camera_controller = CaptureModeController::Get()->camera_controller();
-  return camera_controller ? camera_controller->camera_preview_widget()
-                           : nullptr;
+  return CaptureModeController::Get()
+      ->camera_controller()
+      ->camera_preview_widget();
 }
 
 bool ShouldPassEventToCameraPreview(ui::LocatedEvent* event) {
@@ -348,8 +348,7 @@ bool ShouldPassEventToCameraPreview(ui::LocatedEvent* event) {
   if (!camera_preview_widget || !camera_preview_widget->IsVisible())
     return false;
 
-  auto* camera_controller = controller->camera_controller();
-  if (camera_controller && camera_controller->is_drag_in_progress())
+  if (controller->camera_controller()->is_drag_in_progress())
     return true;
 
   // If the event is targeted on the camera preview, even it's not located
@@ -684,9 +683,8 @@ void CaptureModeSession::Initialize() {
       controller_->type());
   MaybeCreateUserNudge();
 
-  auto* camera_controller = controller_->camera_controller();
-  if (is_in_projector_mode_ && camera_controller)
-    camera_controller->MaybeSelectFirstCamera();
+  if (is_in_projector_mode_)
+    controller_->camera_controller()->MaybeSelectFirstCamera();
 
   Shell::Get()->AddShellObserver(this);
 }
@@ -727,10 +725,8 @@ void CaptureModeSession::Shutdown() {
 
     // Kill the camera preview when the capture mode session ends without
     // starting any recording.
-    if (controller_->camera_controller() &&
-        !controller_->is_recording_in_progress()) {
+    if (!controller_->is_recording_in_progress())
       controller_->camera_controller()->SetShouldShowPreview(false);
-    }
   }
 
   Shell::Get()->RemoveShellObserver(this);
@@ -1062,8 +1058,7 @@ void CaptureModeSession::OnPaintLayer(const ui::PaintContext& context) {
   // UIs (capture bar, capture label), but we should still paint the layer to
   // indicate the capture surface where user can drag camera preview on.
   if (!is_all_uis_visible_ &&
-      !(controller_->camera_controller() &&
-        controller_->camera_controller()->is_drag_in_progress())) {
+      !controller_->camera_controller()->is_drag_in_progress()) {
     return;
   }
 
@@ -1088,9 +1083,8 @@ void CaptureModeSession::OnKeyEvent(ui::KeyEvent* event) {
   if (event->type() != ui::ET_KEY_PRESSED)
     return;
 
-  auto* camera_controller = controller_->camera_controller();
   auto* camera_preview_view =
-      camera_controller ? camera_controller->camera_preview_view() : nullptr;
+      controller_->camera_controller()->camera_preview_view();
   if (camera_preview_view && camera_preview_view->MaybeHandleKeyEvent(event)) {
     event->StopPropagation();
     return;
@@ -1264,8 +1258,7 @@ void CaptureModeSession::OnDisplayMetricsChanged(
   // it if the source is `kRegion`.
   // `CaptureWindowObserver::OnWindowBoundsChanged` will take care of it if the
   // source is `kWindow`.
-  if (controller_->camera_controller() &&
-      controller_->source() == CaptureModeSource::kFullscreen &&
+  if (controller_->source() == CaptureModeSource::kFullscreen &&
       !controller_->is_recording_in_progress()) {
     controller_->camera_controller()->MaybeUpdatePreviewWidget();
   }
@@ -2795,15 +2788,15 @@ void CaptureModeSession::UpdateRegionVertically(bool up, int event_flags) {
 }
 
 void CaptureModeSession::MaybeReparentCameraPreviewWidget() {
-  auto* camera_controller = controller_->camera_controller();
-  if (camera_controller && !controller_->is_recording_in_progress())
-    camera_controller->MaybeReparentPreviewWidget();
+  if (!controller_->is_recording_in_progress())
+    controller_->camera_controller()->MaybeReparentPreviewWidget();
 }
 
 void CaptureModeSession::MaybeUpdateCameraPreviewBounds() {
-  auto* camera_controller = controller_->camera_controller();
-  if (camera_controller && !controller_->is_recording_in_progress())
-    camera_controller->MaybeUpdatePreviewWidget(/*animate=*/false);
+  if (!controller_->is_recording_in_progress()) {
+    controller_->camera_controller()->MaybeUpdatePreviewWidget(
+        /*animate=*/false);
+  }
 }
 
 bool CaptureModeSession::IsEventTargetedOnCaptureBar(

ash/capture_mode/capture_mode_session_focus_cycler.cc

@@ -64,14 +64,15 @@ std::vector<aura::Window*> GetWindowListIgnoreModalForActiveDesk() {
 }
 
 views::Widget* GetCameraPreviewWidget() {
-  auto* camera_controller = CaptureModeController::Get()->camera_controller();
-  return camera_controller ? camera_controller->camera_preview_widget()
-                           : nullptr;
+  return CaptureModeController::Get()
+      ->camera_controller()
+      ->camera_preview_widget();
 }
 
 CameraPreviewView* GetCameraPreviewView() {
-  auto* camera_controller = CaptureModeController::Get()->camera_controller();
-  return camera_controller ? camera_controller->camera_preview_view() : nullptr;
+  return CaptureModeController::Get()
+      ->camera_controller()
+      ->camera_preview_view();
 }
 
 // Returns true if the `value` is within the inclusive range of `low` and

ash/capture_mode/capture_mode_settings_view.cc

@@ -13,7 +13,6 @@
 #include "ash/capture_mode/capture_mode_metrics.h"
 #include "ash/capture_mode/capture_mode_session.h"
 #include "ash/capture_mode/capture_mode_toggle_button.h"
-#include "ash/constants/ash_features.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_provider.h"
@@ -78,8 +77,7 @@ CaptureModeSettingsView::CaptureModeSettingsView(CaptureModeSession* session,
         kAudioMicrophone);
   }
 
-  if (features::IsCaptureModeSelfieCameraEnabled() &&
-      !controller->is_recording_in_progress()) {
+  if (!controller->is_recording_in_progress()) {
     separator_1_ = AddChildView(std::make_unique<views::Separator>());
     separator_1_->SetColorId(ui::kColorAshSystemUIMenuSeparator);
     auto* camera_controller = controller->camera_controller();
@@ -129,8 +127,7 @@ CaptureModeSettingsView::CaptureModeSettingsView(CaptureModeSession* session,
 }
 
 CaptureModeSettingsView::~CaptureModeSettingsView() {
-  if (features::IsCaptureModeSelfieCameraEnabled())
-    CaptureModeController::Get()->camera_controller()->RemoveObserver(this);
+  CaptureModeController::Get()->camera_controller()->RemoveObserver(this);
 }
 
 gfx::Rect CaptureModeSettingsView::GetBounds(
@@ -189,10 +186,8 @@ CaptureModeSettingsView::GetHighlightableItems() {
       highlightable_items;
   DCHECK(audio_input_menu_group_);
   audio_input_menu_group_->AppendHighlightableItems(highlightable_items);
-  if (features::IsCaptureModeSelfieCameraEnabled()) {
-    DCHECK(camera_menu_group_);
-    camera_menu_group_->AppendHighlightableItems(highlightable_items);
-  }
+  DCHECK(camera_menu_group_);
+  camera_menu_group_->AppendHighlightableItems(highlightable_items);
   if (save_to_menu_group_)
     save_to_menu_group_->AppendHighlightableItems(highlightable_items);
   return highlightable_items;