Commit 97824893 authored by Tim Song's avatar Tim Song Committed by Commit Bot

[CrOS PhoneHub] Add metrics for connected phone UI.

This CL implements 3 histograms for the phone connected screen:
  * Ash.PhoneHub.NotificationOptInEvents
  * Ash.PhoneHub.QuickActionResult.*
  * Ash.PhoneHub.TabContinuationChipClicked

BUG=1138137,1106937

Change-Id: Ic6cb37d5dd5208a09dee1249540c8f3c877be3a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2491243Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Tim Song <tengs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821408}
parent ce621fd9
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "ash/root_window_controller.h" #include "ash/root_window_controller.h"
#include "ash/shell.h" #include "ash/shell.h"
#include "ash/style/ash_color_provider.h" #include "ash/style/ash_color_provider.h"
#include "ash/system/phonehub/phone_hub_metrics.h"
#include "ash/system/phonehub/phone_hub_tray.h" #include "ash/system/phonehub/phone_hub_tray.h"
#include "ash/system/status_area_widget.h" #include "ash/system/status_area_widget.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
...@@ -32,8 +33,9 @@ constexpr int kTitleMaxLines = 2; ...@@ -32,8 +33,9 @@ constexpr int kTitleMaxLines = 2;
} // namespace } // namespace
ContinueBrowsingChip::ContinueBrowsingChip( ContinueBrowsingChip::ContinueBrowsingChip(
const chromeos::phonehub::BrowserTabsModel::BrowserTabMetadata& metadata) const chromeos::phonehub::BrowserTabsModel::BrowserTabMetadata& metadata,
: views::Button(this), url_(metadata.url) { int index)
: views::Button(this), url_(metadata.url), index_(index) {
auto* color_provider = AshColorProvider::Get(); auto* color_provider = AshColorProvider::Get();
SetFocusBehavior(FocusBehavior::ALWAYS); SetFocusBehavior(FocusBehavior::ALWAYS);
focus_ring()->SetColor(color_provider->GetControlsLayerColor( focus_ring()->SetColor(color_provider->GetControlsLayerColor(
...@@ -101,8 +103,11 @@ void ContinueBrowsingChip::OnPaintBackground(gfx::Canvas* canvas) { ...@@ -101,8 +103,11 @@ void ContinueBrowsingChip::OnPaintBackground(gfx::Canvas* canvas) {
void ContinueBrowsingChip::ButtonPressed(views::Button* sender, void ContinueBrowsingChip::ButtonPressed(views::Button* sender,
const ui::Event& event) { const ui::Event& event) {
PA_LOG(INFO) << "Opening browser tab: " << url_; PA_LOG(INFO) << "Opening browser tab: " << url_;
phone_hub_metrics::LogTabContinuationChipClicked(index_);
NewWindowDelegate::GetInstance()->NewTabWithUrl( NewWindowDelegate::GetInstance()->NewTabWithUrl(
url_, /*from_user_interaction=*/true); url_, /*from_user_interaction=*/true);
Shell::GetPrimaryRootWindowController() Shell::GetPrimaryRootWindowController()
->GetStatusAreaWidget() ->GetStatusAreaWidget()
->phone_hub_tray() ->phone_hub_tray()
......
...@@ -17,8 +17,9 @@ namespace ash { ...@@ -17,8 +17,9 @@ namespace ash {
class ASH_EXPORT ContinueBrowsingChip : public views::Button, class ASH_EXPORT ContinueBrowsingChip : public views::Button,
public views::ButtonListener { public views::ButtonListener {
public: public:
explicit ContinueBrowsingChip( ContinueBrowsingChip(
const chromeos::phonehub::BrowserTabsModel::BrowserTabMetadata& metadata); const chromeos::phonehub::BrowserTabsModel::BrowserTabMetadata& metadata,
int index);
~ContinueBrowsingChip() override; ~ContinueBrowsingChip() override;
ContinueBrowsingChip(ContinueBrowsingChip&) = delete; ContinueBrowsingChip(ContinueBrowsingChip&) = delete;
...@@ -30,7 +31,11 @@ class ASH_EXPORT ContinueBrowsingChip : public views::Button, ...@@ -30,7 +31,11 @@ class ASH_EXPORT ContinueBrowsingChip : public views::Button,
const char* GetClassName() const override; const char* GetClassName() const override;
private: private:
// The URL of the tab to open.
GURL url_; GURL url_;
// The index of the chip as it is ordered in the parent view.
int index_;
}; };
} // namespace ash } // namespace ash
......
...@@ -6,12 +6,15 @@ ...@@ -6,12 +6,15 @@
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/resources/vector_icons/vector_icons.h"
#include "ash/strings/grit/ash_strings.h" #include "ash/strings/grit/ash_strings.h"
#include "ash/system/phonehub/phone_hub_metrics.h"
#include "ash/system/phonehub/quick_action_item.h" #include "ash/system/phonehub/quick_action_item.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
namespace ash { namespace ash {
using Status = chromeos::phonehub::TetherController::Status; using Status = chromeos::phonehub::TetherController::Status;
using phone_hub_metrics::LogQuickActionClick;
using phone_hub_metrics::QuickAction;
EnableHotspotQuickActionController::EnableHotspotQuickActionController( EnableHotspotQuickActionController::EnableHotspotQuickActionController(
chromeos::phonehub::TetherController* tether_controller) chromeos::phonehub::TetherController* tether_controller)
...@@ -38,6 +41,9 @@ QuickActionItem* EnableHotspotQuickActionController::CreateItem() { ...@@ -38,6 +41,9 @@ QuickActionItem* EnableHotspotQuickActionController::CreateItem() {
} }
void EnableHotspotQuickActionController::OnButtonPressed(bool is_now_enabled) { void EnableHotspotQuickActionController::OnButtonPressed(bool is_now_enabled) {
LogQuickActionClick(is_now_enabled ? QuickAction::kToggleHotspotOff
: QuickAction::kToggleHotspotOn);
is_now_enabled ? tether_controller_->Disconnect() is_now_enabled ? tether_controller_->Disconnect()
: tether_controller_->AttemptConnection(); : tether_controller_->AttemptConnection();
} }
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/resources/vector_icons/vector_icons.h"
#include "ash/strings/grit/ash_strings.h" #include "ash/strings/grit/ash_strings.h"
#include "ash/system/phonehub/phone_hub_metrics.h"
#include "ash/system/phonehub/quick_action_item.h" #include "ash/system/phonehub/quick_action_item.h"
#include "ash/system/phonehub/silence_phone_quick_action_controller.h" #include "ash/system/phonehub/silence_phone_quick_action_controller.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
...@@ -13,6 +14,9 @@ ...@@ -13,6 +14,9 @@
namespace ash { namespace ash {
using phone_hub_metrics::LogQuickActionClick;
using phone_hub_metrics::QuickAction;
namespace { namespace {
// Time to wait until we check the state of the phone to prevent showing wrong // Time to wait until we check the state of the phone to prevent showing wrong
...@@ -50,6 +54,9 @@ QuickActionItem* LocatePhoneQuickActionController::CreateItem() { ...@@ -50,6 +54,9 @@ QuickActionItem* LocatePhoneQuickActionController::CreateItem() {
} }
void LocatePhoneQuickActionController::OnButtonPressed(bool is_now_enabled) { void LocatePhoneQuickActionController::OnButtonPressed(bool is_now_enabled) {
LogQuickActionClick(is_now_enabled ? QuickAction::kToggleLocatePhoneOff
: QuickAction::kToggleLocatePhoneOn);
requested_state_ = is_now_enabled ? ActionState::kOff : ActionState::kOn; requested_state_ = is_now_enabled ? ActionState::kOff : ActionState::kOn;
SetItemState(requested_state_.value()); SetItemState(requested_state_.value());
......
...@@ -30,7 +30,7 @@ namespace ash { ...@@ -30,7 +30,7 @@ namespace ash {
using phone_hub_metrics::InterstitialScreen; using phone_hub_metrics::InterstitialScreen;
using phone_hub_metrics::InterstitialScreenEvent; using phone_hub_metrics::InterstitialScreenEvent;
using phone_hub_metrics::LogInterstitialScreenEvent; using phone_hub_metrics::LogNotificationOptInEvent;
namespace { namespace {
...@@ -64,8 +64,7 @@ NotificationOptInView::NotificationOptInView( ...@@ -64,8 +64,7 @@ NotificationOptInView::NotificationOptInView(
notification_access_manager_(notification_access_manager) { notification_access_manager_(notification_access_manager) {
SetID(PhoneHubViewID::kNotificationOptInView); SetID(PhoneHubViewID::kNotificationOptInView);
InitLayout(); InitLayout();
LogInterstitialScreenEvent(InterstitialScreen::kNotificationOptIn, LogNotificationOptInEvent(InterstitialScreenEvent::kShown);
InterstitialScreenEvent::kShown);
} }
NotificationOptInView::~NotificationOptInView() = default; NotificationOptInView::~NotificationOptInView() = default;
...@@ -75,8 +74,7 @@ void NotificationOptInView::ButtonPressed(views::Button* sender, ...@@ -75,8 +74,7 @@ void NotificationOptInView::ButtonPressed(views::Button* sender,
switch (sender->tag()) { switch (sender->tag()) {
case kDismissButtonTag: case kDismissButtonTag:
// Dismiss this view if user chose to opt out and update the bubble size. // Dismiss this view if user chose to opt out and update the bubble size.
LogInterstitialScreenEvent(InterstitialScreen::kNotificationOptIn, LogNotificationOptInEvent(InterstitialScreenEvent::kDismiss);
InterstitialScreenEvent::kDismiss);
SetVisible(false); SetVisible(false);
bubble_view_->UpdateBubble(); bubble_view_->UpdateBubble();
notification_access_manager_->DismissSetupRequiredUi(); notification_access_manager_->DismissSetupRequiredUi();
...@@ -84,8 +82,7 @@ void NotificationOptInView::ButtonPressed(views::Button* sender, ...@@ -84,8 +82,7 @@ void NotificationOptInView::ButtonPressed(views::Button* sender,
case kSetUpButtonTag: case kSetUpButtonTag:
// Opens the notification set up dialog in settings to start the opt in // Opens the notification set up dialog in settings to start the opt in
// flow. // flow.
LogInterstitialScreenEvent(InterstitialScreen::kNotificationOptIn, LogNotificationOptInEvent(InterstitialScreenEvent::kConfirm);
InterstitialScreenEvent::kConfirm);
NewWindowDelegate::GetInstance()->NewTabWithUrl( NewWindowDelegate::GetInstance()->NewTabWithUrl(
GURL(kMultideviceSettingsUrl), /*from_user_interaction=*/true); GURL(kMultideviceSettingsUrl), /*from_user_interaction=*/true);
break; break;
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#include "ash/system/phonehub/phone_hub_metrics.h" #include "ash/system/phonehub/phone_hub_metrics.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
namespace ash { namespace ash {
...@@ -13,14 +12,12 @@ namespace phone_hub_metrics { ...@@ -13,14 +12,12 @@ namespace phone_hub_metrics {
namespace { namespace {
std::string GetHistogramName(InterstitialScreen screen) { std::string GetInterstitialScreenHistogramName(InterstitialScreen screen) {
switch (screen) { switch (screen) {
case InterstitialScreen::kConnectionError: case InterstitialScreen::kConnectionError:
return "Ash.PhoneHub.InterstitialScreenEvent.ConnectionError"; return "Ash.PhoneHub.InterstitialScreenEvent.ConnectionError";
case InterstitialScreen::kBluetoothOrWifiDisabled: case InterstitialScreen::kBluetoothOrWifiDisabled:
return "Ash.PhoneHub.InterstitialScreenEvent.BluetoothOrWifiDisabled"; return "Ash.PhoneHub.InterstitialScreenEvent.BluetoothOrWifiDisabled";
case InterstitialScreen::kNotificationOptIn:
return "Ash.PhoneHub.InterstitialScreenEvent.NotificationOptIn";
case InterstitialScreen::kReconnecting: case InterstitialScreen::kReconnecting:
return "Ash.PhoneHub.InterstitialScreenEvent.Reconnecting"; return "Ash.PhoneHub.InterstitialScreenEvent.Reconnecting";
case InterstitialScreen::kInitialConnecting: case InterstitialScreen::kInitialConnecting:
...@@ -38,7 +35,21 @@ std::string GetHistogramName(InterstitialScreen screen) { ...@@ -38,7 +35,21 @@ std::string GetHistogramName(InterstitialScreen screen) {
void LogInterstitialScreenEvent(InterstitialScreen screen, void LogInterstitialScreenEvent(InterstitialScreen screen,
InterstitialScreenEvent event) { InterstitialScreenEvent event) {
base::UmaHistogramEnumeration(GetHistogramName(screen), event); base::UmaHistogramEnumeration(GetInterstitialScreenHistogramName(screen),
event);
}
void LogNotificationOptInEvent(InterstitialScreenEvent event) {
base::UmaHistogramEnumeration("Ash.PhoneHub.NotificationOptIn", event);
}
void LogTabContinuationChipClicked(int tab_index) {
base::UmaHistogramCounts100("Ash.PhoneHub.TabContinuationChipClicked",
tab_index);
}
void LogQuickActionClick(QuickAction action) {
base::UmaHistogramEnumeration("Ash.PhoneHub.QuickActionClicked", action);
} }
} // namespace phone_hub_metrics } // namespace phone_hub_metrics
......
...@@ -17,22 +17,41 @@ enum class InterstitialScreenEvent { ...@@ -17,22 +17,41 @@ enum class InterstitialScreenEvent {
kMaxValue = kConfirm kMaxValue = kConfirm
}; };
// Keep in sync with corresponding suffix in // Keep in sync with corresponding template variants in the
// tools/metrics/histograms_xml/histogram_suffixes_list.xml. // Ash.PhoneHub.InterstitialEvent.* histogram.
enum class InterstitialScreen { enum class InterstitialScreen {
kConnectionError = 0, kConnectionError = 0,
kBluetoothOrWifiDisabled, kBluetoothOrWifiDisabled,
kNotificationOptIn,
kReconnecting, kReconnecting,
kInitialConnecting, kInitialConnecting,
kOnboardingExistingMultideviceUser, kOnboardingExistingMultideviceUser,
kOnboardingNewMultideviceUser kOnboardingNewMultideviceUser
}; };
// Keep in sync with corresponding enum in tools/metrics/histograms/enums.xml.
enum class QuickAction {
kToggleHotspotOn = 0,
kToggleHotspotOff,
kToggleQuietModeOn,
kToggleQuietModeOff,
kToggleLocatePhoneOn,
kToggleLocatePhoneOff,
kMaxValue = kToggleLocatePhoneOff
};
// Logs an |event| occurring for the given |interstitial_screen|. // Logs an |event| occurring for the given |interstitial_screen|.
void LogInterstitialScreenEvent(InterstitialScreen screen, void LogInterstitialScreenEvent(InterstitialScreen screen,
InterstitialScreenEvent event); InterstitialScreenEvent event);
// Logs an |event| for the notification opt-in prompt.
void LogNotificationOptInEvent(InterstitialScreenEvent event);
// Logs the |tab_index| of the tab continuation chip that was clicked.
void LogTabContinuationChipClicked(int tab_index);
// Logs a given |quick_action| click.
void LogQuickActionClick(QuickAction quick_action);
} // namespace phone_hub_metrics } // namespace phone_hub_metrics
} // namespace ash } // namespace ash
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/resources/vector_icons/vector_icons.h"
#include "ash/strings/grit/ash_strings.h" #include "ash/strings/grit/ash_strings.h"
#include "ash/system/phonehub/phone_hub_metrics.h"
#include "ash/system/phonehub/quick_action_item.h" #include "ash/system/phonehub/quick_action_item.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -14,6 +15,9 @@ namespace ash { ...@@ -14,6 +15,9 @@ namespace ash {
namespace { namespace {
using phone_hub_metrics::LogQuickActionClick;
using phone_hub_metrics::QuickAction;
// Time to wait until we check the state of the phone to prevent showing wrong // Time to wait until we check the state of the phone to prevent showing wrong
// state // state
constexpr base::TimeDelta kWaitForRequestTimeout = constexpr base::TimeDelta kWaitForRequestTimeout =
...@@ -54,6 +58,9 @@ QuickActionItem* SilencePhoneQuickActionController::CreateItem() { ...@@ -54,6 +58,9 @@ QuickActionItem* SilencePhoneQuickActionController::CreateItem() {
} }
void SilencePhoneQuickActionController::OnButtonPressed(bool is_now_enabled) { void SilencePhoneQuickActionController::OnButtonPressed(bool is_now_enabled) {
LogQuickActionClick(is_now_enabled ? QuickAction::kToggleQuietModeOff
: QuickAction::kToggleQuietModeOn);
requested_state_ = is_now_enabled ? ActionState::kOff : ActionState::kOn; requested_state_ = is_now_enabled ? ActionState::kOff : ActionState::kOn;
SetItemState(requested_state_.value()); SetItemState(requested_state_.value());
......
...@@ -170,9 +170,11 @@ void TaskContinuationView::Update() { ...@@ -170,9 +170,11 @@ void TaskContinuationView::Update() {
return; return;
} }
int index = 0;
for (const BrowserTabsModel::BrowserTabMetadata& metadata : for (const BrowserTabsModel::BrowserTabMetadata& metadata :
browser_tabs.most_recent_tabs()) { browser_tabs.most_recent_tabs()) {
chips_view_->AddTaskChip(new ContinueBrowsingChip(metadata)); chips_view_->AddTaskChip(new ContinueBrowsingChip(metadata, index));
index++;
} }
PreferredSizeChanged(); PreferredSizeChanged();
......
...@@ -57790,6 +57790,15 @@ Called by update_net_trust_anchors.py.--> ...@@ -57790,6 +57790,15 @@ Called by update_net_trust_anchors.py.-->
<int value="14" label="Show notification access setup response"/> <int value="14" label="Show notification access setup response"/>
</enum> </enum>
<enum name="PhoneHubQuickAction">
<int value="0" label="Toggle Hotspot On"/>
<int value="1" label="Toggle Hotspot Off"/>
<int value="2" label="Toggle Quiet Mode On"/>
<int value="3" label="Toggle Quiet Mode Off"/>
<int value="4" label="Toggle Locate Phone On"/>
<int value="5" label="Toggle Locate Phone Off"/>
</enum>
<enum name="PhoneNumberRegexVariantResult"> <enum name="PhoneNumberRegexVariantResult">
<obsolete> <obsolete>
Removed in M82 as the experiment has been stopped. Removed in M82 as the experiment has been stopped.
...@@ -1032,8 +1032,6 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -1032,8 +1032,6 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
<histogram name="Ash.PhoneHub.InterstitialScreenEvent.{ScreenName}" <histogram name="Ash.PhoneHub.InterstitialScreenEvent.{ScreenName}"
enum="PhoneHubInterstitialScreenEvent" expires_after="2021-10-31"> enum="PhoneHubInterstitialScreenEvent" expires_after="2021-10-31">
<!-- Name completed by histogram_suffixes name="PhoneHubInterstitialScreen" -->
<owner>tengs@chromium.org</owner> <owner>tengs@chromium.org</owner>
<owner>khorimoto@chromium.org</owner> <owner>khorimoto@chromium.org</owner>
<summary>Events for the given PhoneHub interstitial screen.</summary> <summary>Events for the given PhoneHub interstitial screen.</summary>
...@@ -1041,13 +1039,37 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -1041,13 +1039,37 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
<variant name="BluetoothOrWifiDisabled"/> <variant name="BluetoothOrWifiDisabled"/>
<variant name="ConnectionError"/> <variant name="ConnectionError"/>
<variant name="InitialConnecting"/> <variant name="InitialConnecting"/>
<variant name="NotificationOptIn"/>
<variant name="Onboarding.ExistingMultideviceUser"/> <variant name="Onboarding.ExistingMultideviceUser"/>
<variant name="Onboarding.NewMultideviceUser"/> <variant name="Onboarding.NewMultideviceUser"/>
<variant name="Reconnecting"/> <variant name="Reconnecting"/>
</token> </token>
</histogram> </histogram>
<histogram name="Ash.PhoneHub.NotificationOptInEvents"
enum="PhoneHubInterstitialScreenEvent" expires_after="2021-10-31">
<owner>tengs@chromium.org</owner>
<owner>khorimoto@chromium.org</owner>
<summary>Events for the given notification opt-in prompt.</summary>
</histogram>
<histogram name="Ash.PhoneHub.QuickActionClicked" enum="PhoneHubQuickAction"
expires_after="2021-10-31">
<owner>tengs@chromium.org</owner>
<owner>khorimoto@chromium.org</owner>
<summary>Event logged after the user clicks on a quick action.</summary>
</histogram>
<histogram name="Ash.PhoneHub.TabContinuationChipClicked" units="tab index"
expires_after="2021-10-31">
<owner>tengs@chromium.org</owner>
<owner>khorimoto@chromium.org</owner>
<summary>
After a tab continuation chip is clicked, the index of the tab is logged.
Tab indices are ordered left-to-right, top-to-bottom in a standard LTR
locale.
</summary>
</histogram>
<histogram name="Ash.Pip.AndroidPipUseTime" units="ms" <histogram name="Ash.Pip.AndroidPipUseTime" units="ms"
expires_after="2021-06-16"> expires_after="2021-06-16">
<owner>takise@chromium.org</owner> <owner>takise@chromium.org</owner>
......
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