Skip to content

Commit

Permalink
[Merge to M-103][Reland] capture_selfie_cam: Enable feature by default.
Browse files Browse the repository at this point in the history
The original CL was reverted due to https://crbug.com/1325697
which was actually caused by http://b/230917107 which
was the root cause for a crash hang on shutdown in the
camera backend. That bug was fixed, so we can now reland
this CL.

As a result of the above mentioned fix, we need to
refresh the cameras list on every capture mode
session start. Patchset#6 contains the diff from
original CL.

Verified on DUT by running:
- tast run <DUT> wmp.CaptureSelfieCamSelection
- tast run <DUT> lacros.Migrate.copy
- tast run <DUT> camera.CCAUIPreview
- tast run <DUT> CCAUIRecordVideo

Fixed: 1327717

Original CL description:

This CL enabled the selfie cam feature of Capture
Mode by default in preparation for launch in M-103.
Also adds some code needed to pass all tests when
this feature is enabled.

(cherry picked from commit e98dbdd)

Fixed: 1324256
Test: Existing tests.
Change-Id: I2914d12c738ad3e8fd59756adf37d556009b26ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3657948
Reviewed-by: Min Chen <minch@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1006016}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3658448
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/branch-heads/5060@{#154}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Ahmed Fakhry authored and Chromium LUCI CQ committed May 21, 2022
1 parent 78e5159 commit 433de09
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 8 deletions.
4 changes: 4 additions & 0 deletions ash/capture_mode/capture_mode_camera_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,10 @@ void CaptureModeCameraController::ToggleCameraPreviewSize() {
MaybeUpdatePreviewWidget(/*animate=*/true);
}

void CaptureModeCameraController::OnCaptureSessionStarted() {
GetCameraDevices();
}

void CaptureModeCameraController::OnRecordingStarted(
bool is_in_projector_mode) {
// Check if there's a camera disconnection that happened before recording
Expand Down
6 changes: 6 additions & 0 deletions ash/capture_mode/capture_mode_camera_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,12 @@ class ASH_EXPORT CaptureModeCameraController
// `is_camera_preview_collapsed_` when the resize button is pressed.
void ToggleCameraPreviewSize();

// Called when a capture session gets started so we can refresh the cameras
// list, since the cros-camera service might have not been running when we
// tried to refresh the cameras at the beginning. (See
// http://b/230917107#comment12 for more details).
void OnCaptureSessionStarted();

void OnRecordingStarted(bool is_in_projector_mode);
void OnRecordingEnded();

Expand Down
1 change: 0 additions & 1 deletion ash/capture_mode/capture_mode_camera_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,6 @@ class CaptureModeCameraTest : public AshTestBase {

private:
base::test::ScopedFeatureList scoped_feature_list_;
base::SystemMonitor system_monitor_;
std::unique_ptr<aura::Window> window_;
};

Expand Down
3 changes: 3 additions & 0 deletions ash/capture_mode/capture_mode_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,9 @@ void CaptureModeController::OnDlpRestrictionCheckedAtSessionInit(
capture_mode_session_ =
std::make_unique<CaptureModeSession>(this, for_projector);
capture_mode_session_->Initialize();

if (camera_controller_)
camera_controller_->OnCaptureSessionStarted();
}

void CaptureModeController::OnDlpRestrictionCheckedAtVideoEnd(
Expand Down
5 changes: 5 additions & 0 deletions ash/capture_mode/capture_mode_menu_group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,11 @@ bool CaptureModeMenuGroup::IsOptionEnabled(int option_id) const {
void CaptureModeMenuGroup::AppendHighlightableItems(
std::vector<CaptureModeSessionFocusCycler::HighlightableView*>&
highlightable_items) {
// The camera menu group can be hidden if there are no cameras connected. In
// this case no items in this group should be highlightable.
if (!GetVisible())
return;

highlightable_items.push_back(menu_header_);
for (auto* option : options_) {
if (option->GetEnabled())
Expand Down
2 changes: 1 addition & 1 deletion ash/constants/ash_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ const base::Feature kCameraPrivacySwitchNotifications{

// Controls whether the selfie camera feature is enabled for Capture Mode.
const base::Feature kCaptureModeSelfieCamera{"CaptureModeSelfieCamera",
base::FEATURE_DISABLED_BY_DEFAULT};
base::FEATURE_ENABLED_BY_DEFAULT};

// If enabled, allow eSIM installation bypass the non-cellular internet
// connectivity check.
Expand Down
4 changes: 3 additions & 1 deletion ash/test/ash_test_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "base/run_loop.h"
#include "base/system/sys_info.h"
#include "base/system/system_monitor.h"
#include "chromeos/ash/components/dbus/rgbkbd/rgbkbd_client.h"
#include "chromeos/dbus/audio/cras_audio_client.h"
#include "chromeos/dbus/power/power_policy_controller.h"
Expand Down Expand Up @@ -91,7 +92,8 @@ class AshTestHelper::PowerPolicyControllerInitializer {
};

AshTestHelper::AshTestHelper(ui::ContextFactory* context_factory)
: AuraTestHelper(context_factory) {
: AuraTestHelper(context_factory),
system_monitor_(std::make_unique<base::SystemMonitor>()) {
views::ViewsTestHelperAura::SetFallbackTestViewsDelegateFactory(
&MakeTestViewsDelegate);

Expand Down
18 changes: 13 additions & 5 deletions ash/test/ash_test_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,23 @@ class PrefService;

namespace aura {
class Window;
}
} // namespace aura

namespace base {
class SystemMonitor;
} // namespace base

namespace display {
class Display;
}
} // namespace display

namespace ui {
class ContextFactory;
}
} // namespace ui

namespace views {
class TestViewsDelegate;
}
} // namespace views

namespace ash {

Expand All @@ -49,7 +53,7 @@ class TestWallpaperControllerClient;

namespace input_method {
class MockInputMethodManager;
}
} // namespace input_method

// A helper class that does common initialization required for Ash. Creates a
// root window and an ash::Shell instance with a test delegate.
Expand Down Expand Up @@ -141,6 +145,10 @@ class AshTestHelper : public aura::test::AuraTestHelper {
class BluezDBusManagerInitializer;
class PowerPolicyControllerInitializer;

// Must be constructed so that `base::SystemMonitor::Get()` returns a valid
// instance.
std::unique_ptr<base::SystemMonitor> system_monitor_;

std::unique_ptr<base::test::ScopedCommandLine> command_line_ =
std::make_unique<base::test::ScopedCommandLine>();
std::unique_ptr<chromeos::system::ScopedFakeStatisticsProvider>
Expand Down

0 comments on commit 433de09

Please sign in to comment.