coral: add missing frontend metrics
This cl adds following missing metrics for coral:
- Ash.Birch.Coral.Action to record the actions performed on the coral
group, e.g. restore, launch to a new desk, save as a desk template.
- Ash.Birch.Coral.ClusterItemRemoved to record the number of items
removed from a group via chip chevron expanded menu.
We also fixed the type of Ash.Birch.Coral.ClusterCount with extract
linear type.
Test: manual test + unit tests
Bug: b:383567646
Change-Id: Ie7a5ba489b06bd7813897f4fcdb0b39d7b01736e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6108497
Reviewed-by: Howard Yang <hcyang@google.com>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Tony Yeoman <tby@chromium.org>
Commit-Queue: Xiaodan Zhu <zxdan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1398952}
diff --git a/ash/birch/birch_coral_item.cc b/ash/birch/birch_coral_item.cc
index ac69169..8ca5de6 100644
--- a/ash/birch/birch_coral_item.cc
+++ b/ash/birch/birch_coral_item.cc
@@ -22,6 +22,7 @@
#include "ash/wm/overview/overview_grid.h"
#include "ash/wm/overview/overview_session.h"
#include "base/barrier_callback.h"
+#include "base/metrics/histogram_functions.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/image/image_skia.h"
#include "ui/gfx/image/image_skia_operations.h"
@@ -113,6 +114,9 @@
BirchCoralItem::~BirchCoralItem() = default;
void BirchCoralItem::LaunchGroup(BirchChipButtonBase* birch_chip_button) {
+ // Record basic metrics.
+ RecordActionMetrics();
+
auto* birch_coral_provider = BirchCoralProvider::Get();
switch (source_) {
@@ -121,6 +125,8 @@
birch_coral_provider->ExtractGroupById(group_id_);
Shell::Get()->coral_delegate()->LaunchPostLoginGroup(std::move(group));
BirchCoralProvider::Get()->OnPostLoginClusterRestored();
+ base::UmaHistogramEnumeration("Ash.Birch.Coral.Action",
+ ActionType::kRestore);
// End the Overview after restore.
// TODO(zxdan|sammie): Consider the restoring failed cases.
OverviewController::Get()->EndOverview(OverviewEndAction::kCoral,
@@ -153,6 +159,9 @@
Shell::Get()->coral_controller()->OpenNewDeskWithGroup(std::move(group),
source_desk);
+ base::UmaHistogramEnumeration("Ash.Birch.Coral.Action",
+ ActionType::kLaunchToNewDesk);
+
// Nudge the desk name on the same display with the `birch_chip_button`.
auto* overview_controller = Shell::Get()->overview_controller();
CHECK(overview_controller->InOverviewSession());
@@ -199,8 +208,7 @@
base::Value::Dict().Set("title", title()).Set("items", std::move(items)));
}
-void BirchCoralItem::PerformAction() {
-}
+void BirchCoralItem::PerformAction() {}
// TODO(b/362530155): Consider refactoring icon loading logic into
// `CoralGroupedIconImage`.
diff --git a/ash/birch/birch_coral_item.h b/ash/birch/birch_coral_item.h
index e2eee63..e475c4e 100644
--- a/ash/birch/birch_coral_item.h
+++ b/ash/birch/birch_coral_item.h
@@ -21,6 +21,16 @@
class ASH_EXPORT BirchCoralItem : public BirchItem {
public:
+ // The actions that perform to a coral group. These values are persisted to
+ // logs. Entries should not be renumbered and numeric values should never be
+ // reused.
+ enum class ActionType {
+ kRestore,
+ kLaunchToNewDesk,
+ kSaveAsDeskTemplate,
+ kMaxValue = kSaveAsDeskTemplate,
+ };
+
BirchCoralItem(const std::u16string& coral_title,
const std::u16string& coral_text,
CoralSource source,
diff --git a/ash/birch/birch_coral_provider.cc b/ash/birch/birch_coral_provider.cc
index c6dd45c..2ec2d461 100644
--- a/ash/birch/birch_coral_provider.cc
+++ b/ash/birch/birch_coral_provider.cc
@@ -430,9 +430,20 @@
}
groups.push_back(std::move(new_group));
}
- fake_response_copy->set_source(HasValidPostLoginData()
- ? CoralSource::kPostLogin
- : CoralSource::kInSession);
+
+ switch (fake_response_->source()) {
+ case CoralSource::kUnknown:
+ fake_response_copy->set_source(HasValidPostLoginData()
+ ? CoralSource::kPostLogin
+ : CoralSource::kInSession);
+ break;
+ case CoralSource::kInSession:
+ fake_response_copy->set_source(CoralSource::kInSession);
+ break;
+ case CoralSource::kPostLogin:
+ fake_response_copy->set_source(CoralSource::kPostLogin);
+ break;
+ }
fake_response_copy->set_groups(std::move(groups));
HandleCoralResponse(std::move(fake_response_copy));
return;
diff --git a/ash/wm/coral/coral_test_util.cc b/ash/wm/coral/coral_test_util.cc
index d52f303..c74f012 100644
--- a/ash/wm/coral/coral_test_util.cc
+++ b/ash/wm/coral/coral_test_util.cc
@@ -59,8 +59,10 @@
"Coral Group");
}
-void OverrideTestResponse(std::vector<coral::mojom::GroupPtr> test_groups) {
+void OverrideTestResponse(std::vector<coral::mojom::GroupPtr> test_groups,
+ CoralSource source) {
auto test_response = std::make_unique<CoralResponse>();
+ test_response->set_source(source);
test_response->set_groups(std::move(test_groups));
BirchCoralProvider::Get()->OverrideCoralResponseForTest(
std::move(test_response));
diff --git a/ash/wm/coral/coral_test_util.h b/ash/wm/coral/coral_test_util.h
index f72833b..74ba3a27 100644
--- a/ash/wm/coral/coral_test_util.h
+++ b/ash/wm/coral/coral_test_util.h
@@ -9,6 +9,7 @@
#include <variant>
#include <vector>
+#include "ash/birch/coral_constants.h"
#include "chromeos/ash/services/coral/public/mojom/coral_service.mojom.h"
namespace ui::test {
@@ -41,7 +42,8 @@
// Creates a group with some default urls and apps.
coral::mojom::GroupPtr CreateDefaultTestGroup();
-void OverrideTestResponse(std::vector<coral::mojom::GroupPtr> test_groups);
+void OverrideTestResponse(std::vector<coral::mojom::GroupPtr> test_groups,
+ CoralSource source = CoralSource::kUnknown);
// Brings up the selector menu host object by entering overview and clicking
// the birch coral chip.
diff --git a/ash/wm/overview/birch/birch_bar_controller.cc b/ash/wm/overview/birch/birch_bar_controller.cc
index 39aab610..4a3f3269 100644
--- a/ash/wm/overview/birch/birch_bar_controller.cc
+++ b/ash/wm/overview/birch/birch_bar_controller.cc
@@ -436,10 +436,8 @@
[](const std::unique_ptr<ash::BirchItem>& item) {
return item->GetType() == BirchItemType::kCoral;
});
- base::UmaHistogramCustomCounts("Ash.Birch.Coral.ClusterCount",
- num_coral_items,
- /*min=*/0, /*exclusive_max=*/3,
- /*buckets=*/3);
+ base::UmaHistogramExactLinear("Ash.Birch.Coral.ClusterCount", num_coral_items,
+ /*exclusive_max=*/3);
RecordTimeOfDayRankingHistogram(items);
diff --git a/ash/wm/overview/birch/birch_bar_unittest.cc b/ash/wm/overview/birch/birch_bar_unittest.cc
index 4746779..5886fd6 100644
--- a/ash/wm/overview/birch/birch_bar_unittest.cc
+++ b/ash/wm/overview/birch/birch_bar_unittest.cc
@@ -5,6 +5,7 @@
#include "ash/birch/birch_item.h"
#include "ash/birch/birch_item_remover.h"
#include "ash/birch/birch_model.h"
+#include "ash/birch/coral_constants.h"
#include "ash/birch/test_birch_client.h"
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_pref_names.h"
@@ -298,7 +299,7 @@
}
// Adds `num` coral items to data source.
- void SetCoralItems(size_t num) {
+ void SetCoralItems(size_t num, CoralSource source = CoralSource::kUnknown) {
// The number of coral items cannot exceed 2.
ASSERT_GE(2u, num);
@@ -323,7 +324,7 @@
test_groups.push_back(std::move(test_group));
}
- OverrideTestResponse(std::move(test_groups));
+ OverrideTestResponse(std::move(test_groups), source);
}
BirchBarMenuModelAdapter* GetBirchBarChipMenuModelAdaper() {
@@ -408,7 +409,6 @@
base::Time::Now() + base::Minutes(30), GURL(), GURL(),
std::string(), /*all_day_event=*/false);
birch_client_->SetCalendarItems(items);
- SetCoralItems(/*num=*/2);
// Entering overview shows the birch bar.
EnterOverview();
@@ -418,16 +418,13 @@
histograms.ExpectBucketCount("Ash.Birch.Bar.Impression", true, 1);
// Four chips were shown.
- histograms.ExpectBucketCount("Ash.Birch.ChipCount", 4, 1);
+ histograms.ExpectBucketCount("Ash.Birch.ChipCount", 2, 1);
// One impression was recorded for each chip type.
histograms.ExpectBucketCount("Ash.Birch.Chip.Impression",
BirchItemType::kFile, 1);
histograms.ExpectBucketCount("Ash.Birch.Chip.Impression",
BirchItemType::kCalendar, 1);
- histograms.ExpectBucketCount("Ash.Birch.Chip.Impression",
- BirchItemType::kCoral, 2);
- histograms.ExpectBucketCount("Ash.Birch.Coral.ClusterCount", 2, 1);
// Two rankings were recorded for the current time slot histogram.
histograms.ExpectBucketCount("Ash.Birch.Ranking.1200to1700", 1, 1);
@@ -438,6 +435,72 @@
histograms.ExpectBucketCount("Ash.Birch.Ranking.Total", 12, 1);
}
+// Tests that we get expected records when showing/hiding/activating the coral
+// chips.
+TEST_F(BirchBarTest, RecordsHistogramForCoralChips) {
+ birch_client_->Reset();
+
+ base::HistogramTester histograms;
+
+ // Add one restore chip.
+ SetCoralItems(1, CoralSource::kPostLogin);
+
+ // Entering overview shows the birch bar.
+ EnterOverview();
+
+ const auto& restore_chips =
+ OverviewGridTestApi(Shell::GetPrimaryRootWindow()).GetBirchChips();
+ ASSERT_EQ(restore_chips.size(), 1u);
+
+ // One cluster count was recorded for the coral chip.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.ClusterCount", 1, 1);
+
+ // Clicking on the restore chip to restore the group and exit Overview.
+ LeftClickOn(restore_chips[0]);
+
+ // One restore action was recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.Action",
+ BirchCoralItem::ActionType::kRestore, 1);
+
+ // One chip activation was recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Chip.Activate", BirchItemType::kCoral,
+ 1);
+
+ ASSERT_FALSE(IsInOverviewSession());
+
+ // Add two in-session chips.
+ SetCoralItems(2, CoralSource::kInSession);
+
+ // Entering overview shows the birch bar.
+ EnterOverview();
+
+ const auto& in_session_chips =
+ OverviewGridTestApi(Shell::GetPrimaryRootWindow()).GetBirchChips();
+ ASSERT_EQ(in_session_chips.size(), 2u);
+
+ // Two cluster count was recorded for the coral chip.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.ClusterCount", 2, 1);
+
+ // Hide the one chip.
+ BirchBarController::Get()->OnItemHiddenByUser(in_session_chips[0]->GetItem());
+ ASSERT_EQ(in_session_chips.size(), 1u);
+
+ // One cluster hidden was recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Chip.Hidden", BirchItemType::kCoral,
+ 1);
+
+ // Clicking on the in-session chip to launch the group.
+ LeftClickOn(in_session_chips[0]);
+
+ // One restore action was recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.Action",
+ BirchCoralItem::ActionType::kLaunchToNewDesk, 1);
+
+ // Another chip activation was recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Chip.Activate", BirchItemType::kCoral,
+ 2);
+}
+
// Tests that the birch bar will be hidden in the partial Overview with a split
// screen.
TEST_F(BirchBarTest, HideBirchBarInPartialSplitScreen) {
diff --git a/ash/wm/overview/birch/tab_app_selection_host.cc b/ash/wm/overview/birch/tab_app_selection_host.cc
index 50ce31e1..4a8ba6b 100644
--- a/ash/wm/overview/birch/tab_app_selection_host.cc
+++ b/ash/wm/overview/birch/tab_app_selection_host.cc
@@ -105,7 +105,10 @@
coral_chip->GetWidget()->GetNativeWindow());
}
-TabAppSelectionHost::~TabAppSelectionHost() = default;
+TabAppSelectionHost::~TabAppSelectionHost() {
+ base::UmaHistogramExactLinear("Ash.Birch.Coral.ClusterItemRemoved",
+ number_of_removed_items_, /*exclusive_max=*/9);
+}
void TabAppSelectionHost::ProcessKeyEvent(ui::KeyEvent* event) {
if (event->type() != ui::EventType::kKeyPressed) {
@@ -125,6 +128,7 @@
}
void TabAppSelectionHost::OnItemRemoved() {
+ number_of_removed_items_++;
owner_->ReloadIcon();
}
diff --git a/ash/wm/overview/birch/tab_app_selection_host.h b/ash/wm/overview/birch/tab_app_selection_host.h
index 2bd95621..335050f 100644
--- a/ash/wm/overview/birch/tab_app_selection_host.h
+++ b/ash/wm/overview/birch/tab_app_selection_host.h
@@ -46,6 +46,9 @@
std::unique_ptr<SelectionHostHider> hider_;
const raw_ptr<CoralChipButton> owner_;
+ // Used for metrics.
+ int number_of_removed_items_ = 0;
+
// This widget isn't activatable so this is a way to force accessibility
// features to focus on the underlying window.
std::unique_ptr<ScopedA11yOverrideWindowSetter> scoped_a11y_overrider_;
diff --git a/ash/wm/overview/birch/tab_app_selection_view.cc b/ash/wm/overview/birch/tab_app_selection_view.cc
index f45dfb3f..c0f2317 100644
--- a/ash/wm/overview/birch/tab_app_selection_view.cc
+++ b/ash/wm/overview/birch/tab_app_selection_view.cc
@@ -32,6 +32,7 @@
#include "ui/views/controls/separator.h"
#include "ui/views/highlight_border.h"
#include "ui/views/view_class_properties.h"
+#include "ui/views/view_utils.h"
namespace ash {
@@ -289,7 +290,7 @@
END_METADATA
// -----------------------------------------------------------------------------
-// UserFeedbackView:
+// TabAppSelectionView::UserFeedbackView:
// A view that allows users to give feedback via the thumb up and thumb down
// buttons.
//
@@ -305,7 +306,7 @@
// `UserFeedbackView`
// TODO(crbug.com/374117101): Add hover state for thumb up/down buttons.
// TODO(crbug.com/374116829): Localization and proper accessibility names.
-class UserFeedbackView : public views::BoxLayoutView {
+class TabAppSelectionView::UserFeedbackView : public views::BoxLayoutView {
METADATA_HEADER(UserFeedbackView, views::BoxLayoutView)
public:
@@ -348,6 +349,7 @@
StyleUtil::SetUpInkDropForButton(thumb_up_button_, gfx::Insets(),
/*highlight_on_hover=*/true,
/*highlight_on_focus=*/false);
+ thumb_up_button_->SetID(TabAppSelectionView::ViewID::kThumbsUpID);
thumb_down_button_ =
thumb_buttons_container->AddChildView(std::make_unique<IconButton>(
@@ -362,6 +364,7 @@
StyleUtil::SetUpInkDropForButton(thumb_down_button_, gfx::Insets(),
/*highlight_on_hover=*/true,
/*highlight_on_focus=*/false);
+ thumb_down_button_->SetID(TabAppSelectionView::ViewID::kThumbsDownID);
}
UserFeedbackView(const UserFeedbackView&) = delete;
@@ -416,7 +419,7 @@
raw_ptr<IconButton> thumb_down_button_;
};
-BEGIN_METADATA(UserFeedbackView)
+BEGIN_METADATA(TabAppSelectionView, UserFeedbackView)
END_METADATA
// -----------------------------------------------------------------------------
diff --git a/ash/wm/overview/birch/tab_app_selection_view.h b/ash/wm/overview/birch/tab_app_selection_view.h
index 8b983684..6f82cfc7 100644
--- a/ash/wm/overview/birch/tab_app_selection_view.h
+++ b/ash/wm/overview/birch/tab_app_selection_view.h
@@ -38,14 +38,19 @@
private:
class TabAppSelectionItemView;
+ class UserFeedbackView;
+
FRIEND_TEST_ALL_PREFIXES(CoralPixelDiffTest, CoralSelectorView);
FRIEND_TEST_ALL_PREFIXES(TabAppSelectionViewTest, CloseSelectorItems);
+ FRIEND_TEST_ALL_PREFIXES(TabAppSelectionViewTest, RecordsHistogram);
// We don't use an enum class to avoid too many explicit casts at callsites.
enum ViewID : int {
kTabSubtitleID = 1,
kAppSubtitleID,
kCloseButtonID,
+ kThumbsUpID,
+ kThumbsDownID,
};
void AdvanceSelection(bool reverse);
diff --git a/ash/wm/overview/birch/tab_app_selection_view_unittest.cc b/ash/wm/overview/birch/tab_app_selection_view_unittest.cc
index 4e52be13..99957a6 100644
--- a/ash/wm/overview/birch/tab_app_selection_view_unittest.cc
+++ b/ash/wm/overview/birch/tab_app_selection_view_unittest.cc
@@ -16,6 +16,7 @@
#include "ash/wm/overview/birch/tab_app_selection_host.h"
#include "ash/wm/overview/birch/tab_app_selection_view.h"
#include "ash/wm/overview/overview_utils.h"
+#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "ui/views/view_utils.h"
@@ -127,4 +128,58 @@
EXPECT_TRUE(!menu->IsVisible());
}
+// Tests that corresponding metrics are recorded when showing the menu, removing
+// the item, and clicking on the thumbs up/down buttons.
+TEST_F(TabAppSelectionViewTest, RecordsHistogram) {
+ base::HistogramTester histograms;
+
+ TabAppSelectionHost* menu = ShowAndGetSelectorMenu(GetEventGenerator());
+ ASSERT_TRUE(menu);
+ EXPECT_TRUE(menu->IsVisible());
+
+ // One menu expanded is recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.ClusterExpanded", true, 1);
+
+ auto* selection_view =
+ views::AsViewClass<TabAppSelectionView>(menu->GetContentsView());
+ ASSERT_TRUE(selection_view);
+ // There are 5 items.
+ ASSERT_EQ(selection_view->item_views_.size(), 5u);
+
+ // Close two items.
+ selection_view->OnCloseButtonPressed(selection_view->item_views_.front());
+ selection_view->OnCloseButtonPressed(selection_view->item_views_.front());
+
+ // Click on thumbs up button.
+ LeftClickOn(
+ selection_view->GetViewByID(TabAppSelectionView::ViewID::kThumbsUpID));
+ // One thumbs up is recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.UserFeedback", true, 1);
+
+ LeftClickOn(menu->owner_for_testing()->addon_view());
+ EXPECT_FALSE(menu->IsVisible());
+
+ LeftClickOn(menu->owner_for_testing()->addon_view());
+ EXPECT_TRUE(menu->IsVisible());
+
+ // Another menu expanded is recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.ClusterExpanded", true, 2);
+
+ // There are 3 items.
+ ASSERT_EQ(selection_view->item_views_.size(), 3u);
+
+ // Close one item.
+ selection_view->OnCloseButtonPressed(selection_view->item_views_.front());
+
+ // Click on the thumbs down button.
+ LeftClickOn(
+ selection_view->GetViewByID(TabAppSelectionView::ViewID::kThumbsDownID));
+ // One thumbs down is recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.UserFeedback", false, 1);
+
+ ExitOverview();
+ // After exiting Overview, the total number of removed items is recorded.
+ histograms.ExpectBucketCount("Ash.Birch.Coral.ClusterItemRemoved", 3, 1);
+}
+
} // namespace ash
diff --git a/tools/metrics/histograms/metadata/ash/enums.xml b/tools/metrics/histograms/metadata/ash/enums.xml
index c6178fd..c92e176 100644
--- a/tools/metrics/histograms/metadata/ash/enums.xml
+++ b/tools/metrics/histograms/metadata/ash/enums.xml
@@ -546,6 +546,12 @@
label="Disabled because the related URLs are blocked by policy"/>
</enum>
+<enum name="CoralActionType">
+ <int value="0" label="Restore"/>
+ <int value="1" label="LaunchToNewDesk"/>
+ <int value="2" label="SaveAsDeskTemplate"/>
+</enum>
+
<enum name="CriticalNotificationOutcome">
<int value="0" label="NotificationShown"/>
<int value="1" label="Crashed"/>
diff --git a/tools/metrics/histograms/metadata/ash/histograms.xml b/tools/metrics/histograms/metadata/ash/histograms.xml
index 8ec41eb8..b923092 100644
--- a/tools/metrics/histograms/metadata/ash/histograms.xml
+++ b/tools/metrics/histograms/metadata/ash/histograms.xml
@@ -1054,9 +1054,19 @@
</summary>
</histogram>
+<histogram name="Ash.Birch.Coral.Action" enum="CoralActionType"
+ expires_after="2025-03-30">
+ <owner>zxdan@chromium.org</owner>
+ <owner>cros-coral@google.com</owner>
+ <summary>
+ Recorded when a coral cluster is restored, launched to a new desk, and saved
+ as a desk templated.
+ </summary>
+</histogram>
+
<histogram name="Ash.Birch.Coral.ClusterCount" units="count"
expires_after="2025-10-10">
- <owner>yulunwu@chromium.org</owner>
+ <owner>zxdan@chromium.org</owner>
<owner>cros-coral@google.com</owner>
<summary>
The number of coral items shown in the birch bar when the user enters
@@ -1066,7 +1076,7 @@
<histogram name="Ash.Birch.Coral.ClusterExpanded" enum="Boolean"
expires_after="2025-03-30">
- <owner>yulunwu@chromium.org</owner>
+ <owner>zxdan@chromium.org</owner>
<owner>cros-coral@google.com</owner>
<summary>
Recorded whenever a user expands a coral BirchChipButton. Can be recorded
@@ -1074,6 +1084,15 @@
</summary>
</histogram>
+<histogram name="Ash.Birch.Coral.ClusterItemRemoved" units="count"
+ expires_after="2025-03-30">
+ <owner>zxdan@chromium.org</owner>
+ <owner>cros-coral@google.com</owner>
+ <summary>
+ The number of coral items removed by user from the chevron expanded menu.
+ </summary>
+</histogram>
+
<histogram name="Ash.Birch.Coral.UserFeedback" enum="Boolean"
expires_after="2025-10-10">
<owner>conniekxu@chromium.org</owner>