cros: Add 'session-state' crash key
This permantent crash key will allow us to tell whether the crash happened before or after the login, or if it was on the lock screen. Bug: b:278694020, b:259728844 Change-Id: I61c4cc3f0e869c8b4c7afcdd0d878d2a69b8715f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4501222 Reviewed-by: Alexander Alekseev <alemate@chromium.org> Reviewed-by: Maksim Ivanov <emaxx@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Denis Kuznetsov <antrim@chromium.org> Commit-Queue: Anastasiia N <anastasiian@chromium.org> Cr-Commit-Position: refs/heads/main@{#1144568}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
75d2c0028b
commit
e98bb37390
chromeos/ash/components/login/auth
@ -30,10 +30,12 @@ component("auth") {
|
||||
"//chromeos/ash/components/metrics",
|
||||
"//chromeos/dbus/constants",
|
||||
"//components/account_id",
|
||||
"//components/crash/core/common:crash_key",
|
||||
"//components/device_event_log",
|
||||
"//components/metrics",
|
||||
"//components/password_manager/core/browser:password_hash_data",
|
||||
"//components/prefs",
|
||||
"//components/session_manager/core:core",
|
||||
"//components/user_manager",
|
||||
"//components/version_info:channel",
|
||||
"//crypto",
|
||||
@ -141,6 +143,7 @@ source_set("unit_tests") {
|
||||
"//chromeos/ash/components/login/auth",
|
||||
"//chromeos/ash/components/osauth/impl",
|
||||
"//components/account_id",
|
||||
"//components/crash/core/common:crash_key",
|
||||
"//components/password_manager/core/browser:password_hash_data",
|
||||
"//components/prefs",
|
||||
"//components/prefs:test_support",
|
||||
|
@ -10,9 +10,11 @@ include_rules = [
|
||||
"+chromeos/ash/components/metrics",
|
||||
"+chromeos/dbus",
|
||||
"+components/account_id",
|
||||
"+components/crash/core/common",
|
||||
"+components/device_event_log",
|
||||
"+components/password_manager",
|
||||
"+components/prefs",
|
||||
"+components/session_manager/core",
|
||||
"+components/user_manager",
|
||||
"+crypto",
|
||||
"+google_apis",
|
||||
|
@ -6,6 +6,7 @@
|
||||
|
||||
#include <vector>
|
||||
|
||||
#include "base/check_is_test.h"
|
||||
#include "base/containers/contains.h"
|
||||
#include "base/memory/ptr_util.h"
|
||||
#include "base/metrics/histogram_functions.h"
|
||||
@ -17,10 +18,14 @@
|
||||
#include "chromeos/ash/components/cryptohome/auth_factor.h"
|
||||
#include "chromeos/ash/components/login/auth/public/auth_failure.h"
|
||||
#include "chromeos/ash/components/login/auth/public/user_context.h"
|
||||
#include "components/crash/core/common/crash_key.h"
|
||||
|
||||
namespace ash {
|
||||
namespace {
|
||||
|
||||
constexpr int kMaxSessionStateCrashKeyLength = 32;
|
||||
constexpr char kSessionStateCrashKey[] = "session-state";
|
||||
|
||||
using AuthenticationSurface = AuthEventsRecorder::AuthenticationSurface;
|
||||
using AuthenticationOutcome = AuthEventsRecorder::AuthenticationOutcome;
|
||||
using CryptohomeRecoveryResult = AuthEventsRecorder::CryptohomeRecoveryResult;
|
||||
@ -181,6 +186,29 @@ std::string GetRecoveryDurationHistogramName(CryptohomeRecoveryResult result) {
|
||||
{kRecoveryDurationHistogramPrefix, GetRecoveryOutcomeSuffix(result)});
|
||||
}
|
||||
|
||||
// Values of the `kSessionStateCrashKey`. The length should not exceed
|
||||
// `kMaxSessionStateCrashKeyLength`.
|
||||
std::string GetSessionStateCrashKeyValue(session_manager::SessionState state) {
|
||||
switch (state) {
|
||||
case session_manager::SessionState::ACTIVE:
|
||||
return "active";
|
||||
case session_manager::SessionState::LOGGED_IN_NOT_ACTIVE:
|
||||
return "logged_in_not_active";
|
||||
case session_manager::SessionState::LOCKED:
|
||||
return "locked";
|
||||
case session_manager::SessionState::LOGIN_PRIMARY:
|
||||
return "login_primary";
|
||||
case session_manager::SessionState::LOGIN_SECONDARY:
|
||||
return "login_secondary";
|
||||
case session_manager::SessionState::OOBE:
|
||||
return "oobe";
|
||||
case session_manager::SessionState::RMA:
|
||||
return "rma";
|
||||
case session_manager::SessionState::UNKNOWN:
|
||||
return "unknown";
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
// static
|
||||
@ -189,6 +217,15 @@ AuthEventsRecorder* AuthEventsRecorder::instance_ = nullptr;
|
||||
AuthEventsRecorder::AuthEventsRecorder() {
|
||||
DCHECK(!instance_);
|
||||
instance_ = this;
|
||||
// Note: SessionManager may be nullptr in tests.
|
||||
if (!session_manager::SessionManager::Get()) {
|
||||
LOG(WARNING) << "Failed to observe SessionManager";
|
||||
CHECK_IS_TEST();
|
||||
return;
|
||||
}
|
||||
session_observation_.Observe(session_manager::SessionManager::Get());
|
||||
// Set the initial value.
|
||||
OnSessionStateChanged();
|
||||
}
|
||||
|
||||
AuthEventsRecorder::~AuthEventsRecorder() {
|
||||
@ -287,6 +324,14 @@ void AuthEventsRecorder::OnRecoveryDone(CryptohomeRecoveryResult result,
|
||||
base::UmaHistogramEnumeration(kRecoveryResultHistogramName, result);
|
||||
}
|
||||
|
||||
void AuthEventsRecorder::OnSessionStateChanged() {
|
||||
session_manager::SessionState session_state =
|
||||
session_manager::SessionManager::Get()->session_state();
|
||||
static crash_reporter::CrashKeyString<kMaxSessionStateCrashKeyLength> key(
|
||||
kSessionStateCrashKey);
|
||||
key.Set(GetSessionStateCrashKeyValue(session_state));
|
||||
}
|
||||
|
||||
void AuthEventsRecorder::MaybeUpdateUserLoginType(bool is_new_user,
|
||||
bool is_login_offline,
|
||||
bool is_ephemeral) {
|
||||
|
@ -7,10 +7,13 @@
|
||||
|
||||
#include <vector>
|
||||
|
||||
#include "base/scoped_observation.h"
|
||||
#include "base/time/time.h"
|
||||
#include "chromeos/ash/components/cryptohome/auth_factor.h"
|
||||
#include "chromeos/ash/components/login/auth/public/auth_failure.h"
|
||||
#include "chromeos/ash/components/login/auth/public/user_context.h"
|
||||
#include "components/session_manager/core/session_manager.h"
|
||||
#include "components/session_manager/core/session_manager_observer.h"
|
||||
|
||||
namespace ash {
|
||||
|
||||
@ -18,7 +21,8 @@ namespace ash {
|
||||
// User actions and behaviors are recorded by AuthEventsRecorder in multiple
|
||||
// stages of the login flow. It will set the appropriate crash keys, and send
|
||||
// UMA metrics.
|
||||
class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_LOGIN_AUTH) AuthEventsRecorder {
|
||||
class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_LOGIN_AUTH) AuthEventsRecorder
|
||||
: public session_manager::SessionManagerObserver {
|
||||
public:
|
||||
// Enum used for UMA. Do NOT reorder or remove entry. Don't forget to
|
||||
// update LoginFlowUserLoginType enum in enums.xml when adding new entries.
|
||||
@ -68,7 +72,7 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_LOGIN_AUTH) AuthEventsRecorder {
|
||||
AuthEventsRecorder& operator=(const AuthEventsRecorder&) = delete;
|
||||
AuthEventsRecorder(AuthEventsRecorder&&) = delete;
|
||||
AuthEventsRecorder& operator=(AuthEventsRecorder&&) = delete;
|
||||
~AuthEventsRecorder();
|
||||
~AuthEventsRecorder() override;
|
||||
|
||||
static AuthEventsRecorder* Get();
|
||||
|
||||
@ -127,6 +131,9 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_LOGIN_AUTH) AuthEventsRecorder {
|
||||
return knowledge_factor_auth_failure_count_;
|
||||
}
|
||||
|
||||
// session_manager::SessionManagerObserver:
|
||||
void OnSessionStateChanged() override;
|
||||
|
||||
private:
|
||||
friend class ChromeBrowserMainPartsAsh;
|
||||
|
||||
@ -149,9 +156,12 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_LOGIN_AUTH) AuthEventsRecorder {
|
||||
|
||||
void Reset();
|
||||
|
||||
int knowledge_factor_auth_failure_count_ = 0;
|
||||
base::ScopedObservation<session_manager::SessionManager,
|
||||
session_manager::SessionManagerObserver>
|
||||
session_observation_{this};
|
||||
|
||||
// All values should be reset to nullopt in `Reset()`;
|
||||
// All values should be reset in `Reset()`;
|
||||
int knowledge_factor_auth_failure_count_ = 0;
|
||||
absl::optional<int> user_count_;
|
||||
absl::optional<bool> show_users_on_signin_;
|
||||
absl::optional<UserLoginType> user_login_type_;
|
||||
|
@ -9,19 +9,31 @@
|
||||
#include "base/test/metrics/histogram_tester.h"
|
||||
#include "base/time/time.h"
|
||||
#include "chromeos/ash/components/cryptohome/auth_factor.h"
|
||||
#include "components/crash/core/common/crash_key.h"
|
||||
#include "testing/gtest/include/gtest/gtest.h"
|
||||
|
||||
namespace ash {
|
||||
|
||||
namespace {
|
||||
|
||||
std::string GetSessionStateCrashKeyValue() {
|
||||
return crash_reporter::GetCrashKeyValue("session-state");
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
class AuthEventsRecorderTest : public ::testing::Test {
|
||||
public:
|
||||
AuthEventsRecorderTest() {
|
||||
crash_reporter::InitializeCrashKeysForTesting();
|
||||
session_manager_ = std::make_unique<session_manager::SessionManager>();
|
||||
recorder_ = AuthEventsRecorder::CreateForTesting();
|
||||
}
|
||||
|
||||
~AuthEventsRecorderTest() override { recorder_.reset(); }
|
||||
|
||||
protected:
|
||||
std::unique_ptr<session_manager::SessionManager> session_manager_;
|
||||
std::unique_ptr<ash::AuthEventsRecorder> recorder_;
|
||||
};
|
||||
|
||||
@ -247,4 +259,19 @@ TEST_F(AuthEventsRecorderTest, OnRecoveryDone) {
|
||||
"Login.CryptohomeRecoveryDuration.Failure", two_seconds, 1);
|
||||
}
|
||||
|
||||
TEST_F(AuthEventsRecorderTest, SessionStateCrashKey) {
|
||||
session_manager_->SetSessionState(
|
||||
session_manager::SessionState::LOGIN_PRIMARY);
|
||||
EXPECT_EQ(GetSessionStateCrashKeyValue(), "login_primary");
|
||||
|
||||
session_manager_->SetSessionState(session_manager::SessionState::LOCKED);
|
||||
EXPECT_EQ(GetSessionStateCrashKeyValue(), "locked");
|
||||
|
||||
session_manager_->SetSessionState(session_manager::SessionState::ACTIVE);
|
||||
EXPECT_EQ(GetSessionStateCrashKeyValue(), "active");
|
||||
|
||||
session_manager_->SetSessionState(session_manager::SessionState::UNKNOWN);
|
||||
EXPECT_EQ(GetSessionStateCrashKeyValue(), "unknown");
|
||||
}
|
||||
|
||||
} // namespace ash
|
||||
|
Reference in New Issue
Block a user