Skip to content

Commit

Permalink
[Phone Hub] Add logging for Phone Hub flow
Browse files Browse the repository at this point in the history
Adds new logs to help debug feature status, host status, and ui status
of the tray icon.

Bug: b/238917647
Test: Verified on DUT chrome://multidevice-internals
Change-Id: I51ea057de1fab22b3a7c5bebbec29122cae6e800
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3766875
Reviewed-by: Jon Mann <jonmann@chromium.org>
Commit-Queue: Juliet Lévesque <julietlevesque@google.com>
Cr-Commit-Position: refs/heads/main@{#1028775}
  • Loading branch information
julietlevesque authored and Chromium LUCI CQ committed Jul 27, 2022
1 parent 30101ca commit 0665040
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 1 deletion.
1 change: 1 addition & 0 deletions ash/components/phonehub/feature_status_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ FeatureStatusProviderImpl::~FeatureStatusProviderImpl() {
}

FeatureStatus FeatureStatusProviderImpl::GetStatus() const {
PA_LOG(VERBOSE) << __func__ << ": status = " << *status_;
return *status_;
}

Expand Down
11 changes: 11 additions & 0 deletions ash/components/phonehub/multidevice_feature_access_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ MultideviceFeatureAccessManagerImpl::MultideviceFeatureAccessManagerImpl(
DCHECK(multidevice_setup_client_);

current_feature_status_ = feature_status_provider_->GetStatus();
PA_LOG(VERBOSE) << __func__
<< ": current feature status = " << current_feature_status_;

feature_status_provider_->AddObserver(this);

pref_change_registrar_.Init(pref_service_);
Expand Down Expand Up @@ -307,6 +310,10 @@ void MultideviceFeatureAccessManagerImpl::
const FeatureStatus previous_feature_status = current_feature_status_;
current_feature_status_ = feature_status_provider_->GetStatus();

PA_LOG(VERBOSE) << __func__
<< ": previous feature status = " << previous_feature_status
<< ", current feature status = " << current_feature_status_;

if (previous_feature_status == current_feature_status_)
return;

Expand Down Expand Up @@ -339,6 +346,10 @@ void MultideviceFeatureAccessManagerImpl::
const FeatureStatus previous_feature_status = current_feature_status_;
current_feature_status_ = feature_status_provider_->GetStatus();

PA_LOG(VERBOSE) << __func__
<< ": previous feature status = " << previous_feature_status
<< ", current feature status = " << current_feature_status_;

if (previous_feature_status == current_feature_status_)
return;

Expand Down
1 change: 1 addition & 0 deletions ash/components/phonehub/tether_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ TetherControllerImpl::~TetherControllerImpl() {
}

TetherController::Status TetherControllerImpl::GetStatus() const {
PA_LOG(VERBOSE) << __func__ << ": status = " << status_;
return status_;
}

Expand Down
33 changes: 33 additions & 0 deletions ash/services/multidevice_setup/host_status_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,37 @@ namespace ash {

namespace multidevice_setup {

namespace {

std::string HostStatusToString(mojom::HostStatus status) {
switch (status) {
case mojom::HostStatus::kNoEligibleHosts:
return "[kNoEligibleHosts]";
case mojom::HostStatus::kEligibleHostExistsButNoHostSet:
return "[kEligibleHostExistsButNoHostSet]";
case mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation:
return "[kHostSetLocallyButWaitingForBackendConfirmation]";
case mojom::HostStatus::kHostSetButNotYetVerified:
return "[kHostSetButNotYetVerified]";
case mojom::HostStatus::kHostVerified:
return "[kHostVerified]";
}
}

std::string HostStatusWithDeviceToString(
mojom::HostStatus host_status,
const absl::optional<multidevice::RemoteDeviceRef>& host_device) {
std::ostringstream stream;
stream << "{" << std::endl;
stream << " " << HostStatusToString(host_status) << ": "
<< (host_device ? host_device->pii_free_name() : " no device")
<< std::endl;
stream << "}";
return stream.str();
}

} // namespace

HostStatusProvider::HostStatusWithDevice::HostStatusWithDevice(
mojom::HostStatus host_status,
const absl::optional<multidevice::RemoteDeviceRef>& host_device)
Expand Down Expand Up @@ -66,6 +97,8 @@ void HostStatusProvider::NotifyHostStatusChange(
mojom::HostStatus host_status,
const absl::optional<multidevice::RemoteDeviceRef>& host_device) {
HostStatusWithDevice host_status_with_device(host_status, host_device);
PA_LOG(INFO) << __func__ << ": "
<< HostStatusWithDeviceToString(host_status, host_device);
for (auto& observer : observer_list_)
observer.OnHostStatusChange(host_status_with_device);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,25 @@ namespace ash {

namespace multidevice_setup {

namespace {

std::string HostStatusToString(mojom::HostStatus status) {
switch (status) {
case mojom::HostStatus::kNoEligibleHosts:
return "[kNoEligibleHosts]";
case mojom::HostStatus::kEligibleHostExistsButNoHostSet:
return "[kEligibleHostExistsButNoHostSet]";
case mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation:
return "[kHostSetLocallyButWaitingForBackendConfirmation]";
case mojom::HostStatus::kHostSetButNotYetVerified:
return "[kHostSetButNotYetVerified]";
case mojom::HostStatus::kHostVerified:
return "[kHostVerified]";
}
}

} // namespace

// static
MultiDeviceSetupClient::HostStatusWithDevice
MultiDeviceSetupClient::GenerateDefaultHostStatusWithDevice() {
Expand Down Expand Up @@ -89,6 +108,20 @@ std::string FeatureStatesMapToString(
return stream.str();
}

std::string HostStatusWithDeviceToString(
const MultiDeviceSetupClient::HostStatusWithDevice&
host_status_with_device) {
std::ostringstream stream;
stream << "{" << std::endl;
stream << " " << HostStatusToString(host_status_with_device.first) << ": "
<< (host_status_with_device.second
? host_status_with_device.second->pii_free_name()
: " no device")
<< std::endl;
stream << "}";
return stream.str();
}

} // namespace multidevice_setup

} // namespace ash
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ class MultiDeviceSetupClient {

std::string FeatureStatesMapToString(
const MultiDeviceSetupClient::FeatureStatesMap& map);
std::string HostStatusWithDeviceToString(
const MultiDeviceSetupClient::HostStatusWithDevice&
host_status_with_device);

} // namespace multidevice_setup

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ void MultiDeviceSetupClientImpl::RemoveHostDevice() {

const MultiDeviceSetupClient::HostStatusWithDevice&
MultiDeviceSetupClientImpl::GetHostStatus() const {
PA_LOG(VERBOSE) << "Responding to GetHostStatus() with the following host = "
<< HostStatusWithDeviceToString(host_status_with_device_);
return host_status_with_device_;
}

Expand Down Expand Up @@ -182,6 +184,8 @@ void MultiDeviceSetupClientImpl::OnHostStatusChanged(
std::make_pair(host_status, absl::nullopt /* host_device */);
}

PA_LOG(INFO) << "Host status with device has changed. New status: "
<< HostStatusWithDeviceToString(host_status_with_device_);
NotifyHostStatusChanged(host_status_with_device_);
}

Expand Down
65 changes: 64 additions & 1 deletion ash/system/phonehub/phone_hub_ui_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,57 @@ phone_hub_metrics::Screen GetMetricsScreen(
}
}

std::string PhoneHubUIStateToString(PhoneHubUiController::UiState ui_state) {
switch (ui_state) {
case PhoneHubUiController::UiState::kOnboardingWithoutPhone:
return "[kOnboardingWithoutPhone]";

case PhoneHubUiController::UiState::kOnboardingWithPhone:
return "[kOnboardingWithPhone]";

case PhoneHubUiController::UiState::kPhoneConnected:
return "[kPhoneConnected]";

case PhoneHubUiController::UiState::kPhoneDisconnected:
return "[kPhoneDisconnected]";

case PhoneHubUiController::UiState::kPhoneConnecting:
return "[kPhoneConnecting]";

case PhoneHubUiController::UiState::kBluetoothDisabled:
return "[kBluetoothDisabled]";

case PhoneHubUiController::UiState::kTetherConnectionPending:
return "[kTetherConnectionPending]";

case PhoneHubUiController::UiState::kHidden:
return "[kHidden]";
}
}

std::string FeatureStatusToString(FeatureStatus feature_status) {
switch (feature_status) {
case FeatureStatus::kNotEligibleForFeature:
return "[kNotEligibleForFeature]";
case FeatureStatus::kEligiblePhoneButNotSetUp:
return "[kEligiblePhoneButNotSetUp]";
case FeatureStatus::kPhoneSelectedAndPendingSetup:
return "[kPhoneSelectedAndPendingSetup]";
case FeatureStatus::kDisabled:
return "[kDisabled]";
case FeatureStatus::kUnavailableBluetoothOff:
return "[kUnavailableBluetoothOff]";
case FeatureStatus::kEnabledButDisconnected:
return "[kEnabledButDisconnected]";
case FeatureStatus::kEnabledAndConnecting:
return "[kEnabledAndConnecting]";
case FeatureStatus::kEnabledAndConnected:
return "[kEnabledAndConnected]";
case FeatureStatus::kLockOrSuspended:
return "[kLockOrSuspended]";
}
}

} // namespace

PhoneHubUiController::PhoneHubUiController() {
Expand Down Expand Up @@ -109,6 +160,9 @@ std::unique_ptr<views::View> PhoneHubUiController::CreateStatusHeaderView(

std::unique_ptr<PhoneHubContentView> PhoneHubUiController::CreateContentView(
OnboardingView::Delegate* delegate) {
PA_LOG(VERBOSE) << __func__
<< ": ui state = " << PhoneHubUIStateToString(ui_state_);

switch (ui_state_) {
case UiState::kHidden:
return nullptr;
Expand Down Expand Up @@ -162,8 +216,14 @@ void PhoneHubUiController::HandleBubbleOpened() {
feature_status == FeatureStatus::kEnabledButDisconnected ||
feature_status == FeatureStatus::kEnabledAndConnected;

if (!is_feature_enabled)
if (!is_feature_enabled) {
PA_LOG(VERBOSE) << __func__ << ": feature is not enabled. Feature status = "
<< FeatureStatusToString(feature_status);
return;
}

PA_LOG(VERBOSE) << __func__ << ": feature is enabled. Feature status = "
<< FeatureStatusToString(feature_status);

if (!has_requested_tether_scan_during_session_ &&
phone_hub_manager_->GetTetherController()->GetStatus() ==
Expand Down Expand Up @@ -244,6 +304,9 @@ void PhoneHubUiController::UpdateUiState(
if (new_state == ui_state_)
return;

PA_LOG(VERBOSE) << __func__
<< ": old ui = " << PhoneHubUIStateToString(ui_state_)
<< ", new ui = " << PhoneHubUIStateToString(new_state);
ui_state_ = new_state;
for (auto& observer : observer_list_)
observer.OnPhoneHubUiStateChanged();
Expand Down

0 comments on commit 0665040

Please sign in to comment.