0

scanner: Disable feature in pinned mode

To do so, we need to use `ScreenPinningController` which typically lives
on the `Shell` instance.

As some `ScannerController` tests do not have a `Shell`, we need to
dependency-inject the `ScreenPinningController` into
`ScannerController`. To manually dependency inject a
`ScreenPinningController` in these tests without a `Shell`, we need to
construct it… but its constructor calls `Shell::Get()` to get the
display manager.

Instead, pass in a pointer to a `ScreenPinningController`, instead, and
assert that if that pointer is null, then we are in a test.

Note that this CL intentionally does not add metrics for this new way of
returning false from `CanShowUi` - this will be added in a follow-up CL.

Bug: b:403423199
Change-Id: I6a6a2100ff889f5c20468d8858b25c1496af7bc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6365376
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Michael Cui <mlcui@google.com>
Cr-Commit-Position: refs/heads/main@{#1435793}
This commit is contained in:
Michael Cui
2025-03-20 17:45:00 -07:00
committed by Chromium LUCI CQ
parent 8c716a7f7b
commit 3a43a07970
5 changed files with 154 additions and 11 deletions

@ -5,6 +5,7 @@
#include "ash/scanner/scanner_controller.h"
#include <cstddef>
#include <functional>
#include <limits>
#include <memory>
#include <optional>
@ -33,6 +34,7 @@
#include "ash/shell.h"
#include "ash/shell_delegate.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/wm/screen_pinning_controller.h"
#include "base/check.h"
#include "base/check_is_test.h"
#include "base/containers/span.h"
@ -430,9 +432,17 @@ std::unique_ptr<manta::proto::ScannerOutput> CreateMockScannerOutput(
}
} // namespace
ScannerController::ScannerController(std::unique_ptr<ScannerDelegate> delegate,
SessionControllerImpl& session_controller)
: delegate_(std::move(delegate)), session_controller_(session_controller) {}
ScannerController::ScannerController(
std::unique_ptr<ScannerDelegate> delegate,
SessionControllerImpl& session_controller,
const ScreenPinningController* screen_pinning_controller)
: delegate_(std::move(delegate)),
session_controller_(session_controller),
screen_pinning_controller_(screen_pinning_controller) {
if (screen_pinning_controller_ == nullptr) {
CHECK_IS_TEST();
}
}
ScannerController::~ScannerController() = default;
@ -479,6 +489,16 @@ void ScannerController::OnActiveUserSessionChanged(
}
bool ScannerController::CanShowUi() {
if (screen_pinning_controller_ == nullptr) {
CHECK_IS_TEST();
} else if (screen_pinning_controller_->IsPinned()) {
// TODO: crbug.com/403423199 - Record a metric here that `CanShowUi`
// returned false due to being in pinned mode.
RecordScannerFeatureUserState(
ScannerFeatureUserState::kCanShowUiReturnedFalse);
return false;
}
// Check enterprise policy.
const AccountId& account_id = session_controller_->GetActiveAccountId();
PrefService* prefs =
@ -586,6 +606,14 @@ bool ScannerController::CanShowUi() {
}
bool ScannerController::CanShowFeatureSettingsToggle() {
if (screen_pinning_controller_ == nullptr) {
CHECK_IS_TEST();
} else if (screen_pinning_controller_->IsPinned()) {
return false;
}
// Intentionally ignore enterprise policy here, as we still want to show the
// settings toggle (as disabled).
ScannerProfileScopedDelegate* profile_scoped_delegate =
delegate_->GetProfileScopedDelegate();
@ -607,6 +635,11 @@ bool ScannerController::CanShowFeatureSettingsToggle() {
}
bool ScannerController::CanStartSession() {
if (screen_pinning_controller_ == nullptr) {
CHECK_IS_TEST();
} else if (screen_pinning_controller_->IsPinned()) {
return false;
}
// Check enterprise policy.
const AccountId& account_id = session_controller_->GetActiveAccountId();
PrefService* prefs =

@ -15,6 +15,7 @@
#include "ash/scanner/scanner_action_view_model.h"
#include "ash/scanner/scanner_session.h"
#include "base/functional/callback.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
#include "base/memory/ref_counted_memory.h"
#include "base/memory/scoped_refptr.h"
@ -30,6 +31,7 @@ namespace ash {
class ScannerCommandDelegateImpl;
class ScannerDelegate;
class ScreenPinningController;
class SessionControllerImpl;
// This is the top level controller used for Scanner. It acts as a mediator
@ -38,8 +40,12 @@ class ASH_EXPORT ScannerController : public SessionObserver {
public:
using OnActionFinishedCallback = base::OnceCallback<void(bool success)>;
explicit ScannerController(std::unique_ptr<ScannerDelegate> delegate,
SessionControllerImpl& session_controller);
// `screen_pinning_controller` must outlive this class.
// It must only be null in tests.
explicit ScannerController(
std::unique_ptr<ScannerDelegate> delegate,
SessionControllerImpl& session_controller,
const ScreenPinningController* screen_pinning_controller);
ScannerController(const ScannerController&) = delete;
ScannerController& operator=(const ScannerController&) = delete;
~ScannerController() override;
@ -131,6 +137,9 @@ class ASH_EXPORT ScannerController : public SessionObserver {
// External dependencies not owned by this class:
// Session controller, stored in `Shell`. Always outlives this class.
raw_ref<SessionControllerImpl> session_controller_;
// Screen pinning controller, stored in `Shell`. Always outlives this class.
// If this pointer is null, then we are in a test.
const raw_ptr<const ScreenPinningController> screen_pinning_controller_;
// Holds mock ScannerOutput data for testing.
std::vector<std::string> mock_scanner_responses_for_testing_;

@ -12,8 +12,10 @@
#include <utility>
#include <vector>
#include "ash/accelerators/accelerator_controller_impl.h"
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_pref_names.h"
#include "ash/public/cpp/accelerator_actions.h"
#include "ash/public/cpp/scanner/scanner_delegate.h"
#include "ash/public/cpp/scanner/scanner_enums.h"
#include "ash/public/cpp/scanner/scanner_feedback_info.h"
@ -32,8 +34,11 @@
#include "ash/system/toast/toast_overlay.h"
#include "ash/test/ash_test_base.h"
#include "ash/test_shell_delegate.h"
#include "ash/wm/screen_pinning_controller.h"
#include "ash/wm/window_util.h"
#include "base/check.h"
#include "base/containers/span.h"
#include "base/functional/bind.h"
#include "base/memory/ref_counted_memory.h"
#include "base/memory/scoped_refptr.h"
#include "base/strings/string_split.h"
@ -65,6 +70,7 @@
#include "ui/message_center/fake_message_center.h"
#include "ui/message_center/message_center.h"
#include "ui/views/view_utils.h"
#include "ui/wm/core/window_util.h"
#include "url/gurl.h"
namespace ash {
@ -270,6 +276,22 @@ TEST_F(ScannerControllerTest,
EXPECT_FALSE(scanner_controller->StartNewSession());
}
TEST_F(ScannerControllerTest, CannotStartSessionInPinnedMode) {
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
ASSERT_TRUE(scanner_controller);
ON_CALL(*GetFakeScannerProfileScopedDelegate(*scanner_controller),
CheckFeatureAccess)
.WillByDefault(Return(specialized_features::FeatureAccessFailureSet{}));
std::unique_ptr<aura::Window> pinned_window = CreateAppWindow();
wm::ActivateWindow(pinned_window.get());
window_util::PinWindow(pinned_window.get(), /*trusted=*/false);
ASSERT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
EXPECT_FALSE(scanner_controller->CanStartSession());
EXPECT_FALSE(scanner_controller->StartNewSession());
}
TEST_F(ScannerControllerTest, CanShowFeatureSettingsToggleIfNoChecksFail) {
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
ASSERT_TRUE(scanner_controller);
@ -280,6 +302,21 @@ TEST_F(ScannerControllerTest, CanShowFeatureSettingsToggleIfNoChecksFail) {
EXPECT_TRUE(scanner_controller->CanShowFeatureSettingsToggle());
}
TEST_F(ScannerControllerTest, DoesNotShowFeatureSettingsToggleInPinnedMode) {
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
ASSERT_TRUE(scanner_controller);
ON_CALL(*GetFakeScannerProfileScopedDelegate(*scanner_controller),
CheckFeatureAccess)
.WillByDefault(Return(specialized_features::FeatureAccessFailureSet{}));
std::unique_ptr<aura::Window> pinned_window = CreateAppWindow();
wm::ActivateWindow(pinned_window.get());
window_util::PinWindow(pinned_window.get(), /*trusted=*/false);
ASSERT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
EXPECT_FALSE(scanner_controller->CanShowFeatureSettingsToggle());
}
TEST_F(ScannerControllerTest, CanShowUiIfConsentNotAcceptedOnly) {
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
ASSERT_TRUE(scanner_controller);
@ -372,6 +409,42 @@ TEST_F(ScannerControllerDisabledTest, CanShowUiForShellFalseWhenNoController) {
EXPECT_FALSE(ScannerController::CanShowUiForShell());
}
TEST_F(ScannerControllerTest, CannotShowUiInPinnedMode) {
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
ASSERT_TRUE(scanner_controller);
ON_CALL(*GetFakeScannerProfileScopedDelegate(*scanner_controller),
CheckFeatureAccess)
.WillByDefault(Return(specialized_features::FeatureAccessFailureSet{}));
std::unique_ptr<aura::Window> pinned_window = CreateAppWindow();
wm::ActivateWindow(pinned_window.get());
window_util::PinWindow(pinned_window.get(), /*trusted=*/false);
ASSERT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
EXPECT_FALSE(scanner_controller->CanShowUi());
EXPECT_FALSE(ScannerController::CanShowUiForShell());
}
TEST_F(ScannerControllerTest, CanShowUiAfterExitingPinnedMode) {
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
ASSERT_TRUE(scanner_controller);
ON_CALL(*GetFakeScannerProfileScopedDelegate(*scanner_controller),
CheckFeatureAccess)
.WillByDefault(Return(specialized_features::FeatureAccessFailureSet{}));
std::unique_ptr<aura::Window> pinned_window = CreateAppWindow();
wm::ActivateWindow(pinned_window.get());
window_util::PinWindow(pinned_window.get(), /*trusted=*/false);
ASSERT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
ASSERT_FALSE(scanner_controller->CanShowUi());
ASSERT_FALSE(ScannerController::CanShowUiForShell());
Shell::Get()->accelerator_controller()->PerformActionIfEnabled(
AcceleratorAction::kUnpin, {});
EXPECT_TRUE(scanner_controller->CanShowUi());
EXPECT_TRUE(ScannerController::CanShowUiForShell());
}
TEST(ScannerControllerNoFixtureTest, CanShowUiForShellFalseWhenNoShellMetrics) {
base::HistogramTester histogram_tester;
@ -402,6 +475,27 @@ TEST_F(ScannerControllerDisabledTest,
ScannerFeatureUserState::kCanShowUiReturnedFalse, 1);
}
TEST_F(ScannerControllerTest, CanShowUiForShellFalseWhenPinnedMetrics) {
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
ASSERT_TRUE(scanner_controller);
ON_CALL(*GetFakeScannerProfileScopedDelegate(*scanner_controller),
CheckFeatureAccess)
.WillByDefault(Return(specialized_features::FeatureAccessFailureSet{}));
std::unique_ptr<aura::Window> pinned_window = CreateAppWindow();
wm::ActivateWindow(pinned_window.get());
window_util::PinWindow(pinned_window.get(), /*trusted=*/false);
ASSERT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
// This must be after pinning, as `SunfishScannerFeatureWatcher` may
// automatically call `CanShowUi` when the pinned state changes.
base::HistogramTester histogram_tester;
ASSERT_FALSE(scanner_controller->CanShowUi());
histogram_tester.ExpectBucketCount(
"Ash.ScannerFeature.UserState",
ScannerFeatureUserState::kCanShowUiReturnedFalse, 1);
}
TEST(ScannerControllerNoFixtureTest,
CanShowUiFalseWhenNoProfileScopedDelegateMetrics) {
base::HistogramTester histogram_tester;
@ -410,7 +504,8 @@ TEST(ScannerControllerNoFixtureTest,
EXPECT_CALL(*mock_delegate, GetProfileScopedDelegate())
.WillRepeatedly(Return(nullptr));
ScannerController scanner_controller(std::move(mock_delegate),
session_controller);
session_controller,
/*screen_pinning_controller=*/nullptr);
ASSERT_FALSE(scanner_controller.CanShowUi());
@ -1566,7 +1661,8 @@ TEST(ScannerControllerNoFixtureTest, RunningNewContactActionOpensUrl) {
_, _))
.Times(1);
ScannerController scanner_controller(std::make_unique<FakeScannerDelegate>(),
session_controller);
session_controller,
/*screen_pinning_controller=*/nullptr);
ScannerSession* session = scanner_controller.StartNewSession();
ASSERT_TRUE(session);
FakeScannerProfileScopedDelegate& delegate =

@ -1035,6 +1035,9 @@ Shell::~Shell() {
float_controller_.reset();
pip_controller_.reset();
// `scanner_controller_` depends on `session_controller_` (destroyed
// implicitly) and `screen_pinning_controller_` (destroyed below).
scanner_controller_.reset();
screen_pinning_controller_.reset();
multidevice_notification_presenter_.reset();
@ -1837,9 +1840,11 @@ void Shell::Init(
}
if (features::IsScannerEnabled()) {
// Depends on `session_controller_` (instantiated in the constructor).
// Depends on `session_controller_` (instantiated in the constructor) and
// `screen_pinning_controller_` (initialised above).
scanner_controller_ = std::make_unique<ScannerController>(
shell_delegate_->CreateScannerDelegate(), *session_controller_);
shell_delegate_->CreateScannerDelegate(), *session_controller_,
screen_pinning_controller_.get());
}
if (features::IsTilingWindowResizeEnabled()) {

@ -1142,8 +1142,6 @@ class ASH_EXPORT Shell : public SessionObserver,
std::unique_ptr<PipController> pip_controller_;
std::unique_ptr<PrivacyScreenController> privacy_screen_controller_;
std::unique_ptr<PolicyRecommendationRestorer> policy_recommendation_restorer_;
// Must be after `session_controller_`.
std::unique_ptr<ScannerController> scanner_controller_;
std::unique_ptr<ScreenSwitchCheckController> screen_switch_check_controller_;
std::unique_ptr<ShelfConfig> shelf_config_;
std::unique_ptr<ShelfController> shelf_controller_;
@ -1221,6 +1219,8 @@ class ASH_EXPORT Shell : public SessionObserver,
std::unique_ptr<DisplayConfigurationObserver> display_configuration_observer_;
std::unique_ptr<ScreenPinningController> screen_pinning_controller_;
// Must be after `session_controller_` and `screen_pinning_controller_`.
std::unique_ptr<ScannerController> scanner_controller_;
std::unique_ptr<PeripheralBatteryListener> peripheral_battery_listener_;
std::unique_ptr<PeripheralBatteryNotifier> peripheral_battery_notifier_;