scanner: Reorder enterprise policy access checks before other checks
This is currently a no-op in behaviour (as confirmed by tests), but will
make some upcoming feature access check metrics more understandable.
This CL also reorders some tests to match the new order of checks.
Bug: b:397105844
Change-Id: I6a6a2100d84137e0ca44a3ab970d444ed89d30fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6275100
Reviewed-by: Michelle Chen <michellegc@google.com>
Commit-Queue: Michael Cui <mlcui@google.com>
Cr-Commit-Position: refs/heads/main@{#1421694}
diff --git a/ash/scanner/scanner_controller.cc b/ash/scanner/scanner_controller.cc
index ce71b581..f095882a 100644
--- a/ash/scanner/scanner_controller.cc
+++ b/ash/scanner/scanner_controller.cc
@@ -423,6 +423,20 @@
}
bool ScannerController::CanShowUi() {
+ // Check enterprise policy.
+ const AccountId& account_id = session_controller_->GetActiveAccountId();
+ PrefService* prefs =
+ session_controller_->GetUserPrefServiceForUser(account_id);
+ // We assume a default value of 1 (allowed without model improvement) if the
+ // value is invalid, or the pref service isn't valid.
+ if (prefs != nullptr &&
+ prefs->GetInteger(prefs::kScannerEnterprisePolicyAllowed) ==
+ static_cast<int>(ScannerEnterprisePolicy::kDisallowed)) {
+ RecordScannerFeatureUserState(
+ ScannerFeatureUserState::kCanShowUiReturnedFalse);
+ return false;
+ }
+
ScannerProfileScopedDelegate* profile_scoped_delegate =
delegate_->GetProfileScopedDelegate();
@@ -446,20 +460,6 @@
return false;
}
- // Check enterprise policy.
- const AccountId& account_id = session_controller_->GetActiveAccountId();
- PrefService* prefs =
- session_controller_->GetUserPrefServiceForUser(account_id);
- // We assume a default value of 1 (allowed without model improvement) if the
- // value is invalid, or the pref service isn't valid.
- if (prefs != nullptr &&
- prefs->GetInteger(prefs::kScannerEnterprisePolicyAllowed) ==
- static_cast<int>(ScannerEnterprisePolicy::kDisallowed)) {
- RecordScannerFeatureUserState(
- ScannerFeatureUserState::kCanShowUiReturnedFalse);
- return false;
- }
-
if (consent_not_accepted) {
RecordScannerFeatureUserState(
ScannerFeatureUserState::kCanShowUiReturnedTrueWithoutConsent);
@@ -492,17 +492,6 @@
}
bool ScannerController::CanStartSession() {
- ScannerProfileScopedDelegate* profile_scoped_delegate =
- delegate_->GetProfileScopedDelegate();
-
- if (profile_scoped_delegate == nullptr) {
- return false;
- }
-
- if (!profile_scoped_delegate->CheckFeatureAccess().empty()) {
- return false;
- }
-
// Check enterprise policy.
const AccountId& account_id = session_controller_->GetActiveAccountId();
PrefService* prefs =
@@ -515,6 +504,17 @@
return false;
}
+ ScannerProfileScopedDelegate* profile_scoped_delegate =
+ delegate_->GetProfileScopedDelegate();
+
+ if (profile_scoped_delegate == nullptr) {
+ return false;
+ }
+
+ if (!profile_scoped_delegate->CheckFeatureAccess().empty()) {
+ return false;
+ }
+
return true;
}
diff --git a/ash/scanner/scanner_controller_unittest.cc b/ash/scanner/scanner_controller_unittest.cc
index 18434896..04d1e30 100644
--- a/ash/scanner/scanner_controller_unittest.cc
+++ b/ash/scanner/scanner_controller_unittest.cc
@@ -413,23 +413,6 @@
}
TEST_F(ScannerControllerTest,
- CanShowUiFalseWhenFeatureAccessCheckFailsMetrics) {
- base::HistogramTester histogram_tester;
- ScannerController* scanner_controller = Shell::Get()->scanner_controller();
- ASSERT_TRUE(scanner_controller);
- EXPECT_CALL(*GetFakeScannerProfileScopedDelegate(*scanner_controller),
- CheckFeatureAccess)
- .WillRepeatedly(Return(specialized_features::FeatureAccessFailureSet{
- specialized_features::FeatureAccessFailure::kDisabledInSettings}));
-
- ASSERT_FALSE(scanner_controller->CanShowUi());
-
- histogram_tester.ExpectBucketCount(
- "Ash.ScannerFeature.UserState",
- ScannerFeatureUserState::kCanShowUiReturnedFalse, 1);
-}
-
-TEST_F(ScannerControllerTest,
CanShowUiFalseWhenEnterprisePolicyDisallowedMetrics) {
base::HistogramTester histogram_tester;
ScannerController* scanner_controller = Shell::Get()->scanner_controller();
@@ -448,6 +431,23 @@
ScannerFeatureUserState::kCanShowUiReturnedFalse, 1);
}
+TEST_F(ScannerControllerTest,
+ CanShowUiFalseWhenFeatureAccessCheckFailsMetrics) {
+ base::HistogramTester histogram_tester;
+ ScannerController* scanner_controller = Shell::Get()->scanner_controller();
+ ASSERT_TRUE(scanner_controller);
+ EXPECT_CALL(*GetFakeScannerProfileScopedDelegate(*scanner_controller),
+ CheckFeatureAccess)
+ .WillRepeatedly(Return(specialized_features::FeatureAccessFailureSet{
+ specialized_features::FeatureAccessFailure::kDisabledInSettings}));
+
+ ASSERT_FALSE(scanner_controller->CanShowUi());
+
+ histogram_tester.ExpectBucketCount(
+ "Ash.ScannerFeature.UserState",
+ ScannerFeatureUserState::kCanShowUiReturnedFalse, 1);
+}
+
TEST_F(ScannerControllerTest, CanShowUiTrueWithoutConsentMetrics) {
base::HistogramTester histogram_tester;
ScannerController* scanner_controller = Shell::Get()->scanner_controller();