Commit 2b61cf8a authored by Ryan Hansberry's avatar Ryan Hansberry Committed by Commit Bot

[SmartLock] Improve logs around setting devices.

In logs from Smart Lock bug reports, I have observed remote devices
being 'successfully' set in EasyUnlockServiceRegular and
ProximityAuthSystem, quickly followed by a message from
ProximityAuthSystem 'User... does not have a Smart Lock host device'.
This change improves logging around setting remote devices in order
to better diagnose the issue if it's observed again.

Bug: 953027
Change-Id: Id46c62aba4e441b3fcb4c32332b392a77e947cd6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617086
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: default avatarJeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661039}
parent ac4eb30f
...@@ -129,7 +129,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() { ...@@ -129,7 +129,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() {
multidevice_setup::mojom::FeatureState::kEnabledByUser) { multidevice_setup::mojom::FeatureState::kEnabledByUser) {
// OnFeatureStatesChanged() will call back on this method when feature state // OnFeatureStatesChanged() will call back on this method when feature state
// changes. // changes.
PA_LOG(VERBOSE) << "Smart Lock is disabled; aborting."; PA_LOG(VERBOSE) << "Smart Lock is not enabled by user; aborting.";
SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(), SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(),
base::nullopt /* local_device */); base::nullopt /* local_device */);
return; return;
...@@ -149,6 +149,8 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() { ...@@ -149,6 +149,8 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() {
pref_manager_->SetEasyUnlockEnabledStateSet(); pref_manager_->SetEasyUnlockEnabledStateSet();
LogSmartLockEnabledState(SmartLockEnabledState::ENABLED); LogSmartLockEnabledState(SmartLockEnabledState::ENABLED);
} else { } else {
PA_LOG(ERROR) << "Smart Lock is enabled by user, but no unlock key is "
"present; aborting.";
SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(), SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(),
base::nullopt /* local_device */); base::nullopt /* local_device */);
...@@ -175,7 +177,11 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() { ...@@ -175,7 +177,11 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() {
void EasyUnlockServiceRegular::UseLoadedRemoteDevices( void EasyUnlockServiceRegular::UseLoadedRemoteDevices(
const multidevice::RemoteDeviceRefList& remote_devices) { const multidevice::RemoteDeviceRefList& remote_devices) {
// When EasyUnlock is enabled, only one EasyUnlock host should exist. // When EasyUnlock is enabled, only one EasyUnlock host should exist.
DCHECK(remote_devices.size() == 1u); if (remote_devices.size() != 1u) {
PA_LOG(ERROR) << "There should only be 1 Smart Lock host, but there are: "
<< remote_devices.size();
NOTREACHED();
}
SetProximityAuthDevices(GetAccountId(), remote_devices, SetProximityAuthDevices(GetAccountId(), remote_devices,
device_sync_client_->GetLocalDeviceMetadata()); device_sync_client_->GetLocalDeviceMetadata());
...@@ -239,9 +245,21 @@ void EasyUnlockServiceRegular::UseLoadedRemoteDevices( ...@@ -239,9 +245,21 @@ void EasyUnlockServiceRegular::UseLoadedRemoteDevices(
multidevice::SoftwareFeatureState::kEnabled; multidevice::SoftwareFeatureState::kEnabled;
dict->SetBoolean(key_names::kKeyUnlockKey, unlock_key); dict->SetBoolean(key_names::kKeyUnlockKey, unlock_key);
PA_LOG(VERBOSE) << "Storing RemoteDevice: { "
<< "name: " << device.name()
<< ", unlock_key: " << unlock_key
<< ", id: " << device.GetTruncatedDeviceIdForLogs()
<< " }.";
device_list->Append(std::move(dict)); device_list->Append(std::move(dict));
} }
if (device_list->GetSize() != 2u) {
PA_LOG(ERROR) << "There should only be 2 devices persisted, the host and "
"the client, but there are: "
<< device_list->GetSize();
NOTREACHED();
}
SetStoredRemoteDevices(*device_list); SetStoredRemoteDevices(*device_list);
} }
...@@ -250,7 +268,6 @@ void EasyUnlockServiceRegular::SetStoredRemoteDevices( ...@@ -250,7 +268,6 @@ void EasyUnlockServiceRegular::SetStoredRemoteDevices(
std::string remote_devices_json; std::string remote_devices_json;
JSONStringValueSerializer serializer(&remote_devices_json); JSONStringValueSerializer serializer(&remote_devices_json);
serializer.Serialize(devices); serializer.Serialize(devices);
PA_LOG(VERBOSE) << "Setting RemoteDevices:\n " << remote_devices_json;
DictionaryPrefUpdate pairing_update(profile()->GetPrefs(), DictionaryPrefUpdate pairing_update(profile()->GetPrefs(),
prefs::kEasyUnlockPairing); prefs::kEasyUnlockPairing);
......
...@@ -59,6 +59,11 @@ void ProximityAuthSystem::SetRemoteDevicesForUser( ...@@ -59,6 +59,11 @@ void ProximityAuthSystem::SetRemoteDevicesForUser(
const AccountId& account_id, const AccountId& account_id,
const chromeos::multidevice::RemoteDeviceRefList& remote_devices, const chromeos::multidevice::RemoteDeviceRefList& remote_devices,
base::Optional<chromeos::multidevice::RemoteDeviceRef> local_device) { base::Optional<chromeos::multidevice::RemoteDeviceRef> local_device) {
PA_LOG(VERBOSE) << "Setting devices for user " << account_id.Serialize()
<< ". Remote device count: " << remote_devices.size()
<< ", Local device: ["
<< (local_device.has_value() ? "present" : "absent") << "].";
remote_devices_map_[account_id] = remote_devices; remote_devices_map_[account_id] = remote_devices;
local_device_map_.emplace(account_id, *local_device); local_device_map_.emplace(account_id, *local_device);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment