Commit 93c9b765 authored by Alex Ilin's avatar Alex Ilin Committed by Chromium LUCI CQ

Record ChromeColors.ColorApplied histogram

With the profile picker experiment, the user might get a color theme
even if they never interacted with the color picker.

This CL adds a new "ChromeColors.ColorApplied" histogram that shows
how often users interact with the color picker.

Bug: 1063856
Change-Id: I2973199fddf57bedf283bb70f69597ea836d24b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593126Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarMonica Basta <msalama@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840098}
parent c772f6c7
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/browser/search/chrome_colors/chrome_colors_service.h" #include "chrome/browser/search/chrome_colors/chrome_colors_service.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "chrome/browser/themes/theme_service_factory.h" #include "chrome/browser/themes/theme_service_factory.h"
#include "chrome/common/search/generated_colors_info.h" #include "chrome/common/search/generated_colors_info.h"
...@@ -11,6 +12,9 @@ ...@@ -11,6 +12,9 @@
namespace chrome_colors { namespace chrome_colors {
const int kDefaultColorId = -1;
const int kOtherColorId = 0;
ChromeColorsService::ChromeColorsService(Profile* profile) ChromeColorsService::ChromeColorsService(Profile* profile)
: theme_service_(ThemeServiceFactory::GetForProfile(profile)) {} : theme_service_(ThemeServiceFactory::GetForProfile(profile)) {}
...@@ -24,7 +28,7 @@ int ChromeColorsService::GetColorId(const SkColor color) { ...@@ -24,7 +28,7 @@ int ChromeColorsService::GetColorId(const SkColor color) {
return color_info.id; return color_info.id;
} }
return 0; return kOtherColorId;
} }
// static // static
...@@ -33,6 +37,10 @@ void ChromeColorsService::RecordColorOnLoadHistogram(SkColor color) { ...@@ -33,6 +37,10 @@ void ChromeColorsService::RecordColorOnLoadHistogram(SkColor color) {
kNumColorsInfo); kNumColorsInfo);
} }
void ChromeColorsService::RecordColorAppliedHistogram(int color_id) {
base::UmaHistogramSparse("ChromeColors.ColorApplied", color_id);
}
void ChromeColorsService::ApplyDefaultTheme(content::WebContents* tab) { void ChromeColorsService::ApplyDefaultTheme(content::WebContents* tab) {
SaveThemeRevertState(tab); SaveThemeRevertState(tab);
theme_service_->UseDefaultTheme(); theme_service_->UseDefaultTheme();
......
...@@ -15,6 +15,10 @@ class TestChromeColorsService; ...@@ -15,6 +15,10 @@ class TestChromeColorsService;
namespace chrome_colors { namespace chrome_colors {
// These constants have to match the values of ChromeColorsInfo in enums.xml.
extern const int kDefaultColorId;
extern const int kOtherColorId;
// Supports theme changes originating from the NTP customization menu. Users can // Supports theme changes originating from the NTP customization menu. Users can
// apply a Chrome color or the default theme, which will then either be reverted // apply a Chrome color or the default theme, which will then either be reverted
// or confirmed and made permanent. If third party themes are present, users // or confirmed and made permanent. If third party themes are present, users
...@@ -25,14 +29,17 @@ class ChromeColorsService : public KeyedService { ...@@ -25,14 +29,17 @@ class ChromeColorsService : public KeyedService {
explicit ChromeColorsService(Profile* profile); explicit ChromeColorsService(Profile* profile);
~ChromeColorsService() override; ~ChromeColorsService() override;
// Returns id for the given |color| if it is in the predefined set, and 0 // Returns id for the given |color| if it is in the predefined set, and
// otherwise. // |kOtherColorId| otherwise.
static int GetColorId(const SkColor color); static int GetColorId(const SkColor color);
// Record installed color id to UMA histogram. If |color| is not in the // Record installed color id to UMA histogram. If |color| is not in the
// predefined set 0 is recorded. // predefined set, |kOtherColorId| is recorded.
static void RecordColorOnLoadHistogram(SkColor color); static void RecordColorOnLoadHistogram(SkColor color);
// Records |color_id| to UMA histogram whenever a new theme is applied.
static void RecordColorAppliedHistogram(int color_id);
// Applies a theme that can be reverted by saving the previous theme state and // Applies a theme that can be reverted by saving the previous theme state and
// the |tab| that changes are made from. // the |tab| that changes are made from.
void ApplyDefaultTheme(content::WebContents* tab); void ApplyDefaultTheme(content::WebContents* tab);
......
...@@ -50,10 +50,14 @@ ChromeCustomizeThemesHandler::~ChromeCustomizeThemesHandler() { ...@@ -50,10 +50,14 @@ ChromeCustomizeThemesHandler::~ChromeCustomizeThemesHandler() {
void ChromeCustomizeThemesHandler::ApplyAutogeneratedTheme( void ChromeCustomizeThemesHandler::ApplyAutogeneratedTheme(
SkColor frame_color) { SkColor frame_color) {
chrome_colors::ChromeColorsService::RecordColorAppliedHistogram(
chrome_colors::kOtherColorId);
chrome_colors_service_->ApplyAutogeneratedTheme(frame_color, web_contents_); chrome_colors_service_->ApplyAutogeneratedTheme(frame_color, web_contents_);
} }
void ChromeCustomizeThemesHandler::ApplyDefaultTheme() { void ChromeCustomizeThemesHandler::ApplyDefaultTheme() {
chrome_colors::ChromeColorsService::RecordColorAppliedHistogram(
chrome_colors::kDefaultColorId);
chrome_colors_service_->ApplyDefaultTheme(web_contents_); chrome_colors_service_->ApplyDefaultTheme(web_contents_);
} }
...@@ -66,6 +70,7 @@ void ChromeCustomizeThemesHandler::ApplyChromeTheme(int32_t id) { ...@@ -66,6 +70,7 @@ void ChromeCustomizeThemesHandler::ApplyChromeTheme(int32_t id) {
}); });
if (result == end) if (result == end)
return; return;
chrome_colors::ChromeColorsService::RecordColorAppliedHistogram(id);
chrome_colors_service_->ApplyAutogeneratedTheme(result->color, web_contents_); chrome_colors_service_->ApplyAutogeneratedTheme(result->color, web_contents_);
} }
...@@ -116,7 +121,8 @@ void ChromeCustomizeThemesHandler::UpdateTheme() { ...@@ -116,7 +121,8 @@ void ChromeCustomizeThemesHandler::UpdateTheme() {
if (theme_service_->UsingDefaultTheme() || if (theme_service_->UsingDefaultTheme() ||
theme_service_->UsingSystemTheme()) { theme_service_->UsingSystemTheme()) {
theme->type = customize_themes::mojom::ThemeType::kDefault; theme->type = customize_themes::mojom::ThemeType::kDefault;
theme->info = customize_themes::mojom::ThemeInfo::NewChromeThemeId(-1); theme->info = customize_themes::mojom::ThemeInfo::NewChromeThemeId(
chrome_colors::kDefaultColorId);
} else if (theme_service_->UsingExtensionTheme()) { } else if (theme_service_->UsingExtensionTheme()) {
theme->type = customize_themes::mojom::ThemeType::kThirdParty; theme->type = customize_themes::mojom::ThemeType::kThirdParty;
auto info = customize_themes::mojom::ThirdPartyThemeInfo::New(); auto info = customize_themes::mojom::ThirdPartyThemeInfo::New();
...@@ -134,7 +140,7 @@ void ChromeCustomizeThemesHandler::UpdateTheme() { ...@@ -134,7 +140,7 @@ void ChromeCustomizeThemesHandler::UpdateTheme() {
DCHECK(theme_service_->UsingAutogeneratedTheme()); DCHECK(theme_service_->UsingAutogeneratedTheme());
int color_id = chrome_colors::ChromeColorsService::GetColorId( int color_id = chrome_colors::ChromeColorsService::GetColorId(
theme_service_->GetAutogeneratedThemeColor()); theme_service_->GetAutogeneratedThemeColor());
if (color_id > 0) { if (color_id != chrome_colors::kOtherColorId) {
theme->type = customize_themes::mojom::ThemeType::kChrome; theme->type = customize_themes::mojom::ThemeType::kChrome;
theme->info = theme->info =
customize_themes::mojom::ThemeInfo::NewChromeThemeId(color_id); customize_themes::mojom::ThemeInfo::NewChromeThemeId(color_id);
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/path_service.h" #include "base/path_service.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/bind.h" #include "base/test/bind.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/test_extension_environment.h" #include "chrome/browser/extensions/test_extension_environment.h"
...@@ -291,3 +292,19 @@ TEST_F(ChromeCustomizeThemesHandlerTest, ResetHandler) { ...@@ -291,3 +292,19 @@ TEST_F(ChromeCustomizeThemesHandlerTest, ResetHandler) {
EXPECT_EQ(kAutogeneratedThemeColor, EXPECT_EQ(kAutogeneratedThemeColor,
theme_service()->GetAutogeneratedThemeColor()); theme_service()->GetAutogeneratedThemeColor());
} }
TEST_F(ChromeCustomizeThemesHandlerTest, RecordColorAppliedHistogram) {
base::HistogramTester histogram_tester;
const char kHistogramName[] = "ChromeColors.ColorApplied";
constexpr SkColor kAutogeneratedThemeColor = SK_ColorBLUE;
handler()->ApplyAutogeneratedTheme(kAutogeneratedThemeColor);
histogram_tester.ExpectBucketCount(kHistogramName, 0, 1);
constexpr int kChromeThemeId = 4;
handler()->ApplyChromeTheme(kChromeThemeId);
histogram_tester.ExpectBucketCount(kHistogramName, kChromeThemeId, 1);
handler()->ApplyDefaultTheme();
histogram_tester.ExpectBucketCount(kHistogramName, -1, 1);
}
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/browser/ui/webui/signin/profile_creation_customize_themes_handler.h" #include "chrome/browser/ui/webui/signin/profile_creation_customize_themes_handler.h"
#include "chrome/browser/search/chrome_colors/chrome_colors_service.h"
#include "chrome/common/search/generated_colors_info.h" #include "chrome/common/search/generated_colors_info.h"
#include "chrome/common/themes/autogenerated_theme_util.h" #include "chrome/common/themes/autogenerated_theme_util.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -21,6 +22,8 @@ ProfileCreationCustomizeThemesHandler:: ...@@ -21,6 +22,8 @@ ProfileCreationCustomizeThemesHandler::
void ProfileCreationCustomizeThemesHandler::ApplyAutogeneratedTheme( void ProfileCreationCustomizeThemesHandler::ApplyAutogeneratedTheme(
SkColor frame_color) { SkColor frame_color) {
chrome_colors::ChromeColorsService::RecordColorAppliedHistogram(
chrome_colors::kOtherColorId);
auto theme = customize_themes::mojom::Theme::New(); auto theme = customize_themes::mojom::Theme::New();
theme->type = customize_themes::mojom::ThemeType::kAutogenerated; theme->type = customize_themes::mojom::ThemeType::kAutogenerated;
auto theme_colors = customize_themes::mojom::ThemeColors::New(); auto theme_colors = customize_themes::mojom::ThemeColors::New();
...@@ -34,13 +37,17 @@ void ProfileCreationCustomizeThemesHandler::ApplyAutogeneratedTheme( ...@@ -34,13 +37,17 @@ void ProfileCreationCustomizeThemesHandler::ApplyAutogeneratedTheme(
} }
void ProfileCreationCustomizeThemesHandler::ApplyDefaultTheme() { void ProfileCreationCustomizeThemesHandler::ApplyDefaultTheme() {
chrome_colors::ChromeColorsService::RecordColorAppliedHistogram(
chrome_colors::kDefaultColorId);
auto theme = customize_themes::mojom::Theme::New(); auto theme = customize_themes::mojom::Theme::New();
theme->type = customize_themes::mojom::ThemeType::kDefault; theme->type = customize_themes::mojom::ThemeType::kDefault;
theme->info = customize_themes::mojom::ThemeInfo::NewChromeThemeId(-1); theme->info = customize_themes::mojom::ThemeInfo::NewChromeThemeId(
chrome_colors::kDefaultColorId);
remote_client_->SetTheme(std::move(theme)); remote_client_->SetTheme(std::move(theme));
} }
void ProfileCreationCustomizeThemesHandler::ApplyChromeTheme(int32_t id) { void ProfileCreationCustomizeThemesHandler::ApplyChromeTheme(int32_t id) {
chrome_colors::ChromeColorsService::RecordColorAppliedHistogram(id);
auto theme = customize_themes::mojom::Theme::New(); auto theme = customize_themes::mojom::Theme::New();
theme->type = customize_themes::mojom::ThemeType::kChrome; theme->type = customize_themes::mojom::ThemeType::kChrome;
theme->info = customize_themes::mojom::ThemeInfo::NewChromeThemeId(id); theme->info = customize_themes::mojom::ThemeInfo::NewChromeThemeId(id);
......
...@@ -84,7 +84,7 @@ void RecordNewProfileSpec(base::Optional<SkColor> profile_color, ...@@ -84,7 +84,7 @@ void RecordNewProfileSpec(base::Optional<SkColor> profile_color,
int theme_id = int theme_id =
profile_color.has_value() profile_color.has_value()
? chrome_colors::ChromeColorsService::GetColorId(*profile_color) ? chrome_colors::ChromeColorsService::GetColorId(*profile_color)
: -1; : chrome_colors::kDefaultColorId;
base::UmaHistogramSparse("ProfilePicker.NewProfileTheme", theme_id); base::UmaHistogramSparse("ProfilePicker.NewProfileTheme", theme_id);
if (ProfileShortcutManager::IsFeatureEnabled()) { if (ProfileShortcutManager::IsFeatureEnabled()) {
......
...@@ -155,6 +155,16 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -155,6 +155,16 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary> </summary>
</histogram> </histogram>
<histogram name="ChromeColors.ColorApplied" enum="ChromeColorsInfo"
expires_after="M90">
<owner>alexilin@chromium.org</owner>
<owner>jkrcal@chromium.org</owner>
<summary>
Records the theme color every time the user clicks on a color icon in the
color picker.
</summary>
</histogram>
<histogram name="ChromeColors.ColorOnLoad" enum="ChromeColorsInfo" <histogram name="ChromeColors.ColorOnLoad" enum="ChromeColorsInfo"
expires_after="2021-05-16"> expires_after="2021-05-16">
<owner>gayane@chromium.org</owner> <owner>gayane@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