0

Add metrics DemoMode.SignedIn.Account{Clean|Set}upResult

Add two new histograms DemoMode.SignedIn.AccountSetupResult and
DemoMode.SignedIn.AccountCleanupResult to report the demo account
request results, and tie the enum class
DemoSessionMetricsRecorder::ResultCode to it.

In some unit tests, we still need to utilize
{set|clean}up_failed_callback_for_testing to wait for the request
to report error, but the extra unnecessary parameter is removed.

ResultCode is moved from DemoLoginController to
DemoSessionMetricsRecorder because ash/ can't depend on
chrome/browser/ for DemoSessionMetricsRecorder to access ResultCode
in DemoLoginController.

TEST= unit tests DemoLoginControllerTest.*

Bug: 390232961
Change-Id: I3b499676a35f7336d700cf728db4e43e447565db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6177880
Commit-Queue: Haifan Wang <wanghaifan@google.com>
Reviewed-by: Xiqi Ruan <xiqiruan@chromium.org>
Reviewed-by: Daniel Classon <dclasson@google.com>
Cr-Commit-Position: refs/heads/main@{#1414460}
This commit is contained in:
HAIFAN WANG
2025-01-31 16:32:51 -08:00
committed by Chromium LUCI CQ
parent 5a917486a5
commit f3a48238b7
7 changed files with 208 additions and 94 deletions

@@ -49,6 +49,11 @@ constexpr char kHelpAppId[] = "nbljnnecbjbmifnoehiemkgefbnpoeak";
constexpr char kDemoModeSignedInShopperDwellTime[] =
"DemoMode.SignedIn.Shopper.DwellTime";
constexpr char kSetupDemoAccountRequestResult[] =
"DemoMode.SignedIn.Request.SetupResult";
constexpr char kCleanupDemoAccountRequestResult[] =
"DemoMode.SignedIn.Request.CleanupResult";
// How many periods to wait for user activity before discarding samples.
// This timeout is low because demo sessions tend to be very short. If we
// recorded samples for a full minute while the device is in between uses, we
@@ -610,6 +615,16 @@ void DemoSessionMetricsRecorder::ReportShopperSessionDwellTime() {
}
}
void DemoSessionMetricsRecorder::ReportDemoAccountSetupResult(
DemoAccountRequestResultCode result_code) {
base::UmaHistogramEnumeration(kSetupDemoAccountRequestResult, result_code);
}
void DemoSessionMetricsRecorder::ReportDemoAccountCleanupResult(
DemoAccountRequestResultCode result_code) {
base::UmaHistogramEnumeration(kCleanupDemoAccountRequestResult, result_code);
}
void DemoSessionMetricsRecorder::StartRecording() {
unique_apps_launched_recording_enabled_ = true;
timer_->Start(FROM_HERE, kSamplePeriod, this,

@@ -95,6 +95,25 @@ class ASH_EXPORT DemoSessionMetricsRecorder
kSystemTrayPowerButton = 2,
};
// The types of the result of the demo account setup or cleanup request.
// This enum is tied directly to a UMA enum
// `DemoModeSignedInAccountRequestResult`, and should always reflect it (do
// not change one without changing the other). Entries should never be
// modified or reordered. Entries can only be removed by deprecating it and
// its value should never be reused. New ones should be added to the end
// (right before the max value).
enum class DemoAccountRequestResultCode {
kSuccess = 0, // Demo account request success.
kResponseParsingError = 1, // Malformat Http response.
kInvalidCreds = 2, // Missing required credential for login.
kEmptyReponse = 3, // Empty Http response.
kNetworkError = 4, // Network error.
kRequestFailed = 5, // Server side error or out of quota.
kCannotObtainDMTokenAndClientID =
6, // Unable to obtain the DM Token and the Client ID.
kMaxValue = kCannotObtainDMTokenAndClientID,
};
static constexpr char kUserClicksAndPressesMetric[] =
"DemoMode.UserClicksAndPresses";
@@ -131,6 +150,12 @@ class ASH_EXPORT DemoSessionMetricsRecorder
// last user activity.
void ReportShopperSessionDwellTime();
// Records the result of demo account setup request.
void ReportDemoAccountSetupResult(DemoAccountRequestResultCode result_code);
// Records the result of demo account cleanup request.
void ReportDemoAccountCleanupResult(DemoAccountRequestResultCode result_code);
private:
// Starts the timer for periodic sampling.
void StartRecording();

@@ -381,12 +381,12 @@ void DemoLoginController::TriggerDemoAccountLoginFlow() {
}
void DemoLoginController::SetSetupFailedCallbackForTest(
base::OnceCallback<void(const ResultCode result_code)> callback) {
FailedRequestCallback callback) {
setup_failed_callback_for_testing_ = std::move(callback);
}
void DemoLoginController::SetCleanUpFailedCallbackForTest(
base::OnceCallback<void(const ResultCode result_code)> callback) {
FailedRequestCallback callback) {
clean_up_failed_callback_for_testing_ = std::move(callback);
}
@@ -453,6 +453,10 @@ void DemoLoginController::HandleSetupDemoAcountResponse(
return;
}
// Report success to the metrics.
DemoSessionMetricsRecorder::Get()->ReportDemoAccountSetupResult(
ResultCode::kSuccess);
UserLoginPermissionTracker::Get()->SetDemoUser(
gaia::CanonicalizeEmail(*email));
DCHECK_EQ(State::kSetupDemoAccountInProgress, state_);
@@ -469,7 +473,7 @@ void DemoLoginController::HandleSetupDemoAcountResponse(
}
void DemoLoginController::OnSetupDemoAccountError(
const DemoLoginController::ResultCode result_code) {
const ResultCode result_code) {
// TODO(crbug.com/372333479): Instruct how to do retry on failed according to
// the error code.
@@ -478,12 +482,15 @@ void DemoLoginController::OnSetupDemoAccountError(
DCHECK_EQ(State::kSetupDemoAccountInProgress, state_);
// Report error to the metrics.
DemoSessionMetricsRecorder::Get()->ReportDemoAccountSetupResult(result_code);
// Login public account session when set up failed.
state_ = State::kLoginToMGS;
configure_auto_login_callback_.Run();
if (setup_failed_callback_for_testing_) {
std::move(setup_failed_callback_for_testing_).Run(result_code);
std::move(setup_failed_callback_for_testing_).Run();
}
}
@@ -554,7 +561,11 @@ void DemoLoginController::MaybeCleanupPreviousDemoAccount() {
void DemoLoginController::OnCleanUpDemoAccountComplete(
std::unique_ptr<std::string> response_body) {
auto result = GetDemoAccountRequestResult(url_loader_.get(), *response_body);
if (result != ResultCode::kSuccess) {
if (result == ResultCode::kSuccess) {
// Report success to the metrics.
DemoSessionMetricsRecorder::Get()->ReportDemoAccountCleanupResult(result);
} else {
LogServerResponseError(*response_body, /*is_setup*/ false);
OnCleanUpDemoAccountError(result);
}
@@ -564,14 +575,17 @@ void DemoLoginController::OnCleanUpDemoAccountComplete(
}
void DemoLoginController::OnCleanUpDemoAccountError(
const DemoLoginController::ResultCode result_code) {
if (clean_up_failed_callback_for_testing_) {
std::move(clean_up_failed_callback_for_testing_).Run(result_code);
}
const ResultCode result_code) {
// Report error to the metrics.
DemoSessionMetricsRecorder::Get()->ReportDemoAccountCleanupResult(
result_code);
// TODO(crbug.com/364214790): Record metric for the failure.
LOG(ERROR) << "Failed to clean up demo account. Result code: "
<< static_cast<int>(result_code);
if (clean_up_failed_callback_for_testing_) {
std::move(clean_up_failed_callback_for_testing_).Run();
}
}
std::optional<base::Value::Dict> DemoLoginController::GetDeviceIdentifier(

@@ -8,6 +8,7 @@
#include <optional>
#include <string>
#include "ash/metrics/demo_session_metrics_recorder.h"
#include "base/functional/callback.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
@@ -22,17 +23,6 @@ namespace ash {
class DemoLoginController
: public policy::DeviceCloudPolicyManagerAsh::Observer {
public:
enum class ResultCode {
kSuccess = 0, // Demo account request success.
kResponseParsingError = 1, // Malformat Http response.
kInvalidCreds = 2, // Missing required credential for login.
kEmptyReponse = 3, // Empty Http response.
kNetworkError = 4, // Network error.
kRequestFailed = 5, // Server side error or out of quota.
kCannotObtainDMTokenAndClientID =
6, // Unbale to obtain the DM Token and the Client ID.
};
enum class State {
// `DemoLoginController` is not initialized properly.
kUnknown = 0,
@@ -56,8 +46,9 @@ class DemoLoginController
kLoginToMGS = 5,
};
using FailedRequestCallback =
base::OnceCallback<void(const ResultCode result_code)>;
using FailedRequestCallback = base::OnceCallback<void()>;
using ResultCode = DemoSessionMetricsRecorder::DemoAccountRequestResultCode;
explicit DemoLoginController(base::RepeatingClosure auto_login_mgs_callback);
DemoLoginController(const DemoLoginController&) = delete;
@@ -69,10 +60,8 @@ class DemoLoginController
// Trigger Demo account login flow.
void TriggerDemoAccountLoginFlow();
void SetSetupFailedCallbackForTest(
base::OnceCallback<void(const ResultCode result_code)> callback);
void SetCleanUpFailedCallbackForTest(
base::OnceCallback<void(const ResultCode result_code)> callback);
void SetSetupFailedCallbackForTest(FailedRequestCallback callback);
void SetCleanUpFailedCallbackForTest(FailedRequestCallback callback);
void SetDeviceCloudPolicyManagerForTesting(
policy::CloudPolicyManager* policy_manager);

@@ -6,10 +6,12 @@
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_pref_names.h"
#include "ash/metrics/demo_session_metrics_recorder.h"
#include "base/logging.h"
#include "base/memory/raw_ptr.h"
#include "base/run_loop.h"
#include "base/test/bind.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/mock_callback.h"
#include "base/test/mock_log.h"
#include "base/test/scoped_feature_list.h"
@@ -84,6 +86,11 @@ constexpr char kPublicAccountUserId[] = "public_session_user@localhost";
constexpr GaiaId::Literal kTestGaiaId("123");
constexpr char kTestEmail[] = "example@gmail.com";
constexpr char kSetupDemoAccountRequestResultHistogram[] =
"DemoMode.SignedIn.Request.SetupResult";
constexpr char kCleanupDemoAccountRequestResultHistogram[] =
"DemoMode.SignedIn.Request.CleanupResult";
} // namespace
class DemoLoginControllerTest : public testing::Test {
@@ -232,6 +239,7 @@ class DemoLoginControllerTest : public testing::Test {
EXPECT_TRUE(user_list[0]->IsDeviceLocalAccount());
}
base::HistogramTester histogram_tester_;
network::TestURLLoaderFactory test_url_loader_factory_;
private:
@@ -297,16 +305,21 @@ TEST_F(DemoLoginControllerTest, OnSetupDemoAccountSuccessFirstTime) {
TEST_F(DemoLoginControllerTest, InValidGaia) {
test_url_loader_factory_.AddResponse(GetSetupUrl().spec(), kInValidGaiaCreds);
base::RunLoop loop;
EXPECT_CALL(login_display_host(), CompleteLogin).Times(0);
base::RunLoop loop;
GetDemoLoginController()->SetSetupFailedCallbackForTest(
base::BindLambdaForTesting(
[&](const DemoLoginController::ResultCode result_code) {
EXPECT_EQ(result_code,
DemoLoginController::ResultCode::kInvalidCreds);
base::BindLambdaForTesting([&]() {
// Expect the setup request to fail by checking metrics.
histogram_tester_.ExpectTotalCount(
kSetupDemoAccountRequestResultHistogram, 1);
histogram_tester_.ExpectBucketCount(
kSetupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::
kInvalidCreds,
1);
loop.Quit();
}));
// Verify demo account login gets triggered by `ExistingUserController`.
ConfigureAutoLoginSetting();
loop.Run();
@@ -321,20 +334,17 @@ TEST_F(DemoLoginControllerTest,
// base::Value::Dict), causing the request to fail.
GetDemoLoginController()->SetDeviceCloudPolicyManagerForTesting(nullptr);
base::RunLoop loop;
EXPECT_CALL(login_display_host(), CompleteLogin).Times(0);
GetDemoLoginController()->SetSetupFailedCallbackForTest(
base::BindLambdaForTesting([&](const DemoLoginController::ResultCode
result_code) {
EXPECT_EQ(
result_code,
DemoLoginController::ResultCode::kCannotObtainDMTokenAndClientID);
loop.Quit();
}));
// Verify demo account login gets triggered by `ExistingUserController`.
ConfigureAutoLoginSetting();
loop.Run();
// Expect the setup request to fail by checking metrics.
histogram_tester_.ExpectTotalCount(kSetupDemoAccountRequestResultHistogram,
1);
histogram_tester_.ExpectBucketCount(
kSetupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::
kCannotObtainDMTokenAndClientID,
1);
}
TEST_F(DemoLoginControllerTest, ServerCleanUpSuccess) {
@@ -343,12 +353,6 @@ TEST_F(DemoLoginControllerTest, ServerCleanUpSuccess) {
local_state->SetString(prefs::kDemoAccountGaiaId, kTestGaiaId.ToString());
const std::string last_session_id = "device_id";
local_state->SetString(prefs::kDemoModeSessionIdentifier, last_session_id);
base::MockCallback<DemoLoginController::FailedRequestCallback>
cleanup_failed_callback;
// `cleanup_failed_callback` not called means no failure for the cleanup.
EXPECT_CALL(cleanup_failed_callback, Run(testing::_)).Times(0);
GetDemoLoginController()->SetCleanUpFailedCallbackForTest(
cleanup_failed_callback.Get());
// Verify demo account login gets triggered by `ExistingUserController`.
ConfigureAutoLoginSetting();
@@ -362,6 +366,13 @@ TEST_F(DemoLoginControllerTest, ServerCleanUpSuccess) {
local_state->GetString(prefs::kDemoModeSessionIdentifier);
EXPECT_NE(new_session_id, last_session_id);
// Expect the cleanup request to succeed by checking metrics.
histogram_tester_.ExpectTotalCount(kCleanupDemoAccountRequestResultHistogram,
1);
histogram_tester_.ExpectBucketCount(
kCleanupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::kSuccess, 1);
ExpectOnlyDeviceLocalAccountInUserList();
}
@@ -375,13 +386,17 @@ TEST_F(DemoLoginControllerTest, ServerCleanUpFailed) {
net::HTTP_UNAUTHORIZED);
base::RunLoop loop;
GetDemoLoginController()->SetCleanUpFailedCallbackForTest(
base::BindLambdaForTesting(
[&](const DemoLoginController::ResultCode result_code) {
EXPECT_EQ(result_code,
DemoLoginController::ResultCode::kRequestFailed);
base::BindLambdaForTesting([&]() {
// Expect the cleanup request to fail by checking metrics.
histogram_tester_.ExpectTotalCount(
kCleanupDemoAccountRequestResultHistogram, 1);
histogram_tester_.ExpectBucketCount(
kCleanupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::
kRequestFailed,
1);
loop.Quit();
}));
// Verify demo account login gets triggered by `ExistingUserController`.
ConfigureAutoLoginSetting();
loop.Run();
@@ -413,46 +428,59 @@ TEST_F(DemoLoginControllerTest,
const std::string last_session_id = "device_id";
local_state->SetString(prefs::kDemoModeSessionIdentifier, last_session_id);
// Right after the account cleanup failed, it'll try to set up the demo
// account regardless of the cleanup result. However, it's still
// unable to obtain the DM Token and the Client ID, so it will fail again and
// fall back to MGS.
base::RunLoop loop;
GetDemoLoginController()->SetCleanUpFailedCallbackForTest(
base::BindLambdaForTesting([&](const DemoLoginController::ResultCode
result_code) {
EXPECT_EQ(
result_code,
DemoLoginController::ResultCode::kCannotObtainDMTokenAndClientID);
loop.Quit();
}));
// Verify demo account login gets triggered by `ExistingUserController`.
ConfigureAutoLoginSetting();
loop.Run();
// Expect the test account to be removed even if the cleanup failed.
ExpectOnlyDeviceLocalAccountInUserList();
// Expect auto login managed guest session to start.
// Expect the cleanup request to fail by checking metrics.
histogram_tester_.ExpectTotalCount(kCleanupDemoAccountRequestResultHistogram,
1);
histogram_tester_.ExpectBucketCount(
kCleanupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::
kCannotObtainDMTokenAndClientID,
1);
// Right after the account cleanup failed, it'll try to set up the demo
// account regardless of the cleanup result. However, it's still
// unable to obtain the DM Token and the Client ID, so it will fail again and
// fall back to MGS. Therefore, we expect the auto login managed guest session
// to start.
EXPECT_TRUE(existing_user_controller()->IsAutoLoginTimerRunningForTesting());
// Also expect the setup request to fail for the same reason by checking
// metrics.
histogram_tester_.ExpectTotalCount(kSetupDemoAccountRequestResultHistogram,
1);
histogram_tester_.ExpectBucketCount(
kSetupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::
kCannotObtainDMTokenAndClientID,
1);
}
TEST_F(DemoLoginControllerTest, FallbackToMGS) {
// Mock setup failed by returning invalid credential.
test_url_loader_factory_.AddResponse(GetSetupUrl().spec(), kInValidGaiaCreds);
base::RunLoop loop;
EXPECT_CALL(login_display_host(), CompleteLogin).Times(0);
base::RunLoop loop;
GetDemoLoginController()->SetSetupFailedCallbackForTest(
base::BindLambdaForTesting(
[&](const DemoLoginController::ResultCode result_code) {
base::BindLambdaForTesting([&]() {
// Expect the setup request to fail by checking metrics.
histogram_tester_.ExpectTotalCount(
kSetupDemoAccountRequestResultHistogram, 1);
histogram_tester_.ExpectBucketCount(
kSetupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::
kInvalidCreds,
1);
loop.Quit();
}));
// Verify demo account login gets triggered by `ExistingUserController`.
ConfigureAutoLoginSetting();
loop.Run();
// Expect auto login managed guest session to start.
@@ -470,20 +498,24 @@ TEST_F(DemoLoginControllerTest, LogServerError) {
testing::HasSubstr("Setup response error: error code: 500; message: "
"Internal error encountered.; status: INTERNAL.")))
.Times(1);
base::RunLoop loop;
EXPECT_CALL(login_display_host(), CompleteLogin).Times(0);
base::RunLoop loop;
GetDemoLoginController()->SetSetupFailedCallbackForTest(
base::BindLambdaForTesting(
[&](const DemoLoginController::ResultCode result_code) {
EXPECT_EQ(result_code,
DemoLoginController::ResultCode::kRequestFailed);
base::BindLambdaForTesting([&]() {
// Expect the setup request to fail by checking metrics.
histogram_tester_.ExpectTotalCount(
kSetupDemoAccountRequestResultHistogram, 1);
histogram_tester_.ExpectBucketCount(
kSetupDemoAccountRequestResultHistogram,
DemoSessionMetricsRecorder::DemoAccountRequestResultCode::
kRequestFailed,
1);
loop.Quit();
log.StartCapturingLogs();
}));
// Trigger auto sign in:
ConfigureAutoLoginSetting();
loop.Run();
}

@@ -3749,6 +3749,19 @@ to ensure that the crash string is shown properly on the user-facing crash UI.
<int value="33" label="Success. No error."/>
</enum>
<enum name="DemoModeSignedInAccountRequestResult">
<summary>
The result of account cleanup request in demo mode signed in experience.
</summary>
<int value="0" label="Success."/>
<int value="1" label="Malformat Http response."/>
<int value="2" label="Missing required credential for login."/>
<int value="3" label="Empty Http response."/>
<int value="4" label="Network error."/>
<int value="5" label="Server side error or out of quota."/>
<int value="6" label="Unbale to obtain the DM Token and the Client ID."/>
</enum>
<enum name="DeprecatedArcKioskUserStatus">
<int value="0" label="Hide ARC kiosk user on the login screen"/>
<int value="1" label="Attempt to delete cryptohome for ARC kiosk user"/>

@@ -3685,6 +3685,32 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>
<histogram name="DemoMode.SignedIn.AccountCleanupResult"
enum="DemoModeSignedInAccountRequestResult" expires_after="2026-01-15">
<owner>wanghaifan@google.com</owner>
<owner>cros-demo-mode-eng@google.com</owner>
<summary>
This records the result of the account cleanup request in demo mode signed
in experience. It can be success or one of the possible failure reasons,
such as the network error. Please refer to the enum class
DemoSessionMetricsRecorder::DemoAccountRequestResultCode for more
information.
</summary>
</histogram>
<histogram name="DemoMode.SignedIn.AccountSetupResult"
enum="DemoModeSignedInAccountRequestResult" expires_after="2026-01-15">
<owner>wanghaifan@google.com</owner>
<owner>cros-demo-mode-eng@google.com</owner>
<summary>
This records the result of the account setup request in demo mode signed in
experience. It can be success or one of the possible failure reasons, such
as the network error. Please refer to the enum class
DemoSessionMetricsRecorder::DemoAccountRequestResultCode for more
information.
</summary>
</histogram>
<histogram name="DemoMode.SignedIn.Shopper.DwellTime" units="seconds"
expires_after="2026-01-21">
<owner>wanghaifan@google.com</owner>