Commit 1fc4f4ff authored by phillis's avatar phillis Committed by Commit Bot

Badging: Add UKM metric for |BadgeManager:SetBadge| call

Add UKM metric that records SetBadge call per web app
to track the Badging API usage on SetAppBadge.
Review doc is at https://docs.google.com/document/d/1XNamCi2kjjGLQQ3uJtpYcZpxDTY1NlDnMBFBFjlybGk/edit

Bug: 1051682
Change-Id: Ia6a1829cd275f873bc5a72d087b873f3f301959e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2108537Reviewed-by: default avatarBrian White <bcwhite@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarChris Mumford <cmumford@google.com>
Commit-Queue: Phillis Tang <phillis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760215}
parent b541cc6a
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/browser/badging/badge_manager.h" #include "chrome/browser/badging/badge_manager.h"
#include <tuple>
#include <utility> #include <utility>
#include "base/i18n/number_formatting.h" #include "base/i18n/number_formatting.h"
...@@ -17,10 +18,13 @@ ...@@ -17,10 +18,13 @@
#include "chrome/browser/ui/web_applications/app_browser_controller.h" #include "chrome/browser/ui/web_applications/app_browser_controller.h"
#include "chrome/browser/web_applications/components/app_registrar.h" #include "chrome/browser/web_applications/components/app_registrar.h"
#include "chrome/browser/web_applications/components/web_app_provider_base.h" #include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "components/ukm/app_source_url_recorder.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "services/metrics/public/cpp/delegating_ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/strings/grit/ui_strings.h" #include "ui/strings/grit/ui_strings.h"
...@@ -96,11 +100,27 @@ base::Optional<BadgeManager::BadgeValue> BadgeManager::GetBadgeValue( ...@@ -96,11 +100,27 @@ base::Optional<BadgeManager::BadgeValue> BadgeManager::GetBadgeValue(
} }
void BadgeManager::SetBadgeForTesting(const web_app::AppId& app_id, void BadgeManager::SetBadgeForTesting(const web_app::AppId& app_id,
BadgeValue value) { BadgeValue value,
ukm::UkmRecorder* test_recorder) {
ukm::SourceId source_id = ukm::UkmRecorder::GetNewSourceID();
if (value == base::nullopt) {
ukm::builders::Badging(source_id)
.SetUpdateAppBadge(kSetFlagBadge)
.Record(test_recorder);
} else {
ukm::builders::Badging(source_id)
.SetUpdateAppBadge(kSetNumericBadge)
.Record(test_recorder);
}
UpdateBadge(app_id, value); UpdateBadge(app_id, value);
} }
void BadgeManager::ClearBadgeForTesting(const web_app::AppId& app_id) { void BadgeManager::ClearBadgeForTesting(const web_app::AppId& app_id,
ukm::UkmRecorder* test_recorder) {
ukm::SourceId source_id = ukm::UkmRecorder::GetNewSourceID();
ukm::builders::Badging(source_id)
.SetUpdateAppBadge(kClearBadge)
.Record(test_recorder);
UpdateBadge(app_id, base::nullopt); UpdateBadge(app_id, base::nullopt);
} }
...@@ -125,39 +145,65 @@ void BadgeManager::SetBadge(blink::mojom::BadgeValuePtr mojo_value) { ...@@ -125,39 +145,65 @@ void BadgeManager::SetBadge(blink::mojom::BadgeValuePtr mojo_value) {
return; return;
} }
const std::vector<web_app::AppId> app_ids = const std::vector<std::tuple<web_app::AppId, GURL>> app_ids_and_urls =
receivers_.current_context()->GetAppIdsForBadging(); receivers_.current_context()->GetAppIdsAndUrlsForBadging();
// Convert the mojo badge representation into a BadgeManager::BadgeValue. // Convert the mojo badge representation into a BadgeManager::BadgeValue.
BadgeValue value = mojo_value->is_flag() BadgeValue value = mojo_value->is_flag()
? base::nullopt ? base::nullopt
: base::make_optional(mojo_value->get_number()); : base::make_optional(mojo_value->get_number());
for (const auto& app_id : app_ids) // ukm::SourceId source_id = ukm::UkmRecorder::GetNewSourceID();
UpdateBadge(app_id, base::make_optional(value)); ukm::UkmRecorder* recorder = ukm::UkmRecorder::Get();
for (const auto& app : app_ids_and_urls) {
GURL url = std::get<1>(app);
// The app's start_url is used to identify the app
// for recording badging usage per app.
ukm::SourceId source_id = ukm::AppSourceUrlRecorder::GetSourceIdForPWA(url);
if (value == base::nullopt) {
ukm::builders::Badging(source_id)
.SetUpdateAppBadge(kSetFlagBadge)
.Record(recorder);
} else {
ukm::builders::Badging(source_id)
.SetUpdateAppBadge(kSetNumericBadge)
.Record(recorder);
}
UpdateBadge(/*app_id=*/std::get<0>(app), base::make_optional(value));
}
} }
void BadgeManager::ClearBadge() { void BadgeManager::ClearBadge() {
const std::vector<web_app::AppId> app_ids = const std::vector<std::tuple<web_app::AppId, GURL>> app_ids =
receivers_.current_context()->GetAppIdsForBadging(); receivers_.current_context()->GetAppIdsAndUrlsForBadging();
for (const auto& app_id : app_ids) ukm::UkmRecorder* recorder = ukm::UkmRecorder::Get();
UpdateBadge(app_id, base::nullopt); for (const auto& app : app_ids) {
// The app's start_url is used to identify the app
// for recording badging usage per app.
GURL url = std::get<1>(app);
ukm::SourceId source_id = ukm::AppSourceUrlRecorder::GetSourceIdForPWA(url);
ukm::builders::Badging(source_id)
.SetUpdateAppBadge(kClearBadge)
.Record(recorder);
UpdateBadge(/*app_id=*/std::get<0>(app), base::nullopt);
}
} }
std::vector<web_app::AppId> std::vector<std::tuple<web_app::AppId, GURL>>
BadgeManager::FrameBindingContext::GetAppIdsForBadging() const { BadgeManager::FrameBindingContext::GetAppIdsAndUrlsForBadging() const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::RenderFrameHost* frame = content::RenderFrameHost* frame =
content::RenderFrameHost::FromID(process_id_, frame_id_); content::RenderFrameHost::FromID(process_id_, frame_id_);
if (!frame) if (!frame)
return std::vector<web_app::AppId>{}; return std::vector<std::tuple<web_app::AppId, GURL>>{};
content::WebContents* contents = content::WebContents* contents =
content::WebContents::FromRenderFrameHost(frame); content::WebContents::FromRenderFrameHost(frame);
if (!contents) if (!contents)
return std::vector<web_app::AppId>{}; return std::vector<std::tuple<web_app::AppId, GURL>>{};
const web_app::AppRegistrar& registrar = const web_app::AppRegistrar& registrar =
web_app::WebAppProviderBase::GetProviderBase( web_app::WebAppProviderBase::GetProviderBase(
...@@ -167,25 +213,30 @@ BadgeManager::FrameBindingContext::GetAppIdsForBadging() const { ...@@ -167,25 +213,30 @@ BadgeManager::FrameBindingContext::GetAppIdsForBadging() const {
const base::Optional<web_app::AppId> app_id = const base::Optional<web_app::AppId> app_id =
registrar.FindAppWithUrlInScope(frame->GetLastCommittedURL()); registrar.FindAppWithUrlInScope(frame->GetLastCommittedURL());
if (!app_id) if (!app_id)
return std::vector<web_app::AppId>{}; return std::vector<std::tuple<web_app::AppId, GURL>>{};
return std::vector<web_app::AppId>{app_id.value()}; return std::vector<std::tuple<web_app::AppId, GURL>>{std::make_tuple(
app_id.value(), registrar.GetAppLaunchURL(app_id.value()))};
} }
std::vector<web_app::AppId> std::vector<std::tuple<web_app::AppId, GURL>>
BadgeManager::ServiceWorkerBindingContext::GetAppIdsForBadging() const { BadgeManager::ServiceWorkerBindingContext::GetAppIdsAndUrlsForBadging() const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::RenderProcessHost* render_process_host = content::RenderProcessHost* render_process_host =
content::RenderProcessHost::FromID(process_id_); content::RenderProcessHost::FromID(process_id_);
if (!render_process_host) if (!render_process_host)
return std::vector<web_app::AppId>{}; return std::vector<std::tuple<web_app::AppId, GURL>>{};
const web_app::AppRegistrar& registrar = const web_app::AppRegistrar& registrar =
web_app::WebAppProviderBase::GetProviderBase( web_app::WebAppProviderBase::GetProviderBase(
Profile::FromBrowserContext(render_process_host->GetBrowserContext())) Profile::FromBrowserContext(render_process_host->GetBrowserContext()))
->registrar(); ->registrar();
std::vector<std::tuple<web_app::AppId, GURL>> app_ids_urls{};
return registrar.FindAppsInScope(scope_); for (const auto& app_id : registrar.FindAppsInScope(scope_)) {
app_ids_urls.push_back(
std::make_tuple(app_id, registrar.GetAppLaunchURL(app_id)));
}
return app_ids_urls;
} }
std::string GetBadgeString(base::Optional<uint64_t> badge_content) { std::string GetBadgeString(base::Optional<uint64_t> badge_content) {
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/optional.h" #include "base/optional.h"
#include "chrome/browser/web_applications/components/web_app_id.h" #include "chrome/browser/web_applications/components/web_app_id.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "components/ukm/test_ukm_recorder.h"
#include "mojo/public/cpp/bindings/receiver_set.h" #include "mojo/public/cpp/bindings/receiver_set.h"
#include "third_party/blink/public/mojom/badging/badging.mojom.h" #include "third_party/blink/public/mojom/badging/badging.mojom.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -28,6 +29,19 @@ class RenderProcessHost; ...@@ -28,6 +29,19 @@ class RenderProcessHost;
namespace badging { namespace badging {
class BadgeManagerDelegate; class BadgeManagerDelegate;
// Records different types of update badge event.
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum UpdateBadgeType {
// Set badge to a positive integer value.
kSetNumericBadge = 0,
// Set badge without value, display a plain dot.
kSetFlagBadge = 1,
// Clear badge with either navigator.setAppBadge(0)
// or navigator.clearAppBadge().
kClearBadge = 2,
};
// The maximum value of badge contents before saturation occurs. // The maximum value of badge contents before saturation occurs.
constexpr uint64_t kMaxBadgeContent = 99u; constexpr uint64_t kMaxBadgeContent = 99u;
...@@ -58,8 +72,13 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService { ...@@ -58,8 +72,13 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
// badged. // badged.
base::Optional<BadgeValue> GetBadgeValue(const web_app::AppId& app_id); base::Optional<BadgeValue> GetBadgeValue(const web_app::AppId& app_id);
void SetBadgeForTesting(const web_app::AppId& app_id, BadgeValue value); void SetBadgeForTesting(
void ClearBadgeForTesting(const web_app::AppId& app_id); const web_app::AppId& app_id,
BadgeValue value,
ukm::UkmRecorder* test_recorder = ukm::TestUkmRecorder::Get());
void ClearBadgeForTesting(
const web_app::AppId& app_id,
ukm::UkmRecorder* test_recorder = ukm::TestUkmRecorder::Get());
private: private:
// The BindingContext of a mojo request. Allows mojo calls to be tied back // The BindingContext of a mojo request. Allows mojo calls to be tied back
...@@ -73,7 +92,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService { ...@@ -73,7 +92,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
// Gets the list of app IDs to badge, based on the state of this // Gets the list of app IDs to badge, based on the state of this
// BindingContext. Returns an empty list when no apps exist for this // BindingContext. Returns an empty list when no apps exist for this
// BindingContext. // BindingContext.
virtual std::vector<web_app::AppId> GetAppIdsForBadging() const = 0; virtual std::vector<std::tuple<web_app::AppId, GURL>>
GetAppIdsAndUrlsForBadging() const = 0;
}; };
// The BindingContext for Window execution contexts. // The BindingContext for Window execution contexts.
...@@ -85,7 +105,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService { ...@@ -85,7 +105,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
// Returns the AppId that matches the frame's URL. Returns either 0 or 1 // Returns the AppId that matches the frame's URL. Returns either 0 or 1
// AppIds. // AppIds.
std::vector<web_app::AppId> GetAppIdsForBadging() const override; std::vector<std::tuple<web_app::AppId, GURL>> GetAppIdsAndUrlsForBadging()
const override;
private: private:
int process_id_; int process_id_;
...@@ -101,7 +122,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService { ...@@ -101,7 +122,8 @@ class BadgeManager : public KeyedService, public blink::mojom::BadgeService {
// Returns the list of AppIds within the service worker's scope. Returns // Returns the list of AppIds within the service worker's scope. Returns
// either 0, 1 or more AppIds. // either 0, 1 or more AppIds.
std::vector<web_app::AppId> GetAppIdsForBadging() const override; std::vector<std::tuple<web_app::AppId, GURL>> GetAppIdsAndUrlsForBadging()
const override;
private: private:
int process_id_; int process_id_;
......
...@@ -15,10 +15,12 @@ ...@@ -15,10 +15,12 @@
#include "chrome/browser/web_applications/components/web_app_provider_base.h" #include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "chrome/browser/web_applications/test/test_app_registrar.h" #include "chrome/browser/web_applications/test/test_app_registrar.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/ukm/test_ukm_recorder.h"
#include "content/public/test/browser_task_environment.h" #include "content/public/test/browser_task_environment.h"
#include "extensions/common/extension.h" #include "extensions/common/extension.h"
#include "extensions/common/extension_builder.h" #include "extensions/common/extension_builder.h"
#include "extensions/common/extension_id.h" #include "extensions/common/extension_id.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -71,7 +73,14 @@ class BadgeManagerUnittest : public ::testing::Test { ...@@ -71,7 +73,14 @@ class BadgeManagerUnittest : public ::testing::Test {
}; };
TEST_F(BadgeManagerUnittest, SetFlagBadgeForApp) { TEST_F(BadgeManagerUnittest, SetFlagBadgeForApp) {
badge_manager()->SetBadgeForTesting(kAppId, base::nullopt); ukm::TestUkmRecorder test_recorder;
badge_manager()->SetBadgeForTesting(kAppId, base::nullopt, &test_recorder);
auto entries =
test_recorder.GetEntriesByName(ukm::builders::Badging::kEntryName);
ASSERT_EQ(entries.size(), 1u);
test_recorder.ExpectEntryMetric(
entries[0], ukm::builders::Badging::kUpdateAppBadgeName, kSetFlagBadge);
EXPECT_EQ(1UL, delegate()->set_badges().size()); EXPECT_EQ(1UL, delegate()->set_badges().size());
EXPECT_EQ(kAppId, delegate()->set_badges().front().first); EXPECT_EQ(kAppId, delegate()->set_badges().front().first);
...@@ -79,9 +88,15 @@ TEST_F(BadgeManagerUnittest, SetFlagBadgeForApp) { ...@@ -79,9 +88,15 @@ TEST_F(BadgeManagerUnittest, SetFlagBadgeForApp) {
} }
TEST_F(BadgeManagerUnittest, SetBadgeForApp) { TEST_F(BadgeManagerUnittest, SetBadgeForApp) {
badge_manager()->SetBadgeForTesting(kAppId, ukm::TestUkmRecorder test_recorder;
base::make_optional(kBadgeContents)); badge_manager()->SetBadgeForTesting(
kAppId, base::make_optional(kBadgeContents), &test_recorder);
auto entries =
test_recorder.GetEntriesByName(ukm::builders::Badging::kEntryName);
ASSERT_EQ(entries.size(), 1u);
test_recorder.ExpectEntryMetric(entries[0],
ukm::builders::Badging::kUpdateAppBadgeName,
kSetNumericBadge);
EXPECT_EQ(1UL, delegate()->set_badges().size()); EXPECT_EQ(1UL, delegate()->set_badges().size());
EXPECT_EQ(kAppId, delegate()->set_badges().front().first); EXPECT_EQ(kAppId, delegate()->set_badges().front().first);
EXPECT_EQ(kBadgeContents, delegate()->set_badges().front().second); EXPECT_EQ(kBadgeContents, delegate()->set_badges().front().second);
...@@ -122,10 +137,16 @@ TEST_F(BadgeManagerUnittest, SetBadgeForAppAfterClear) { ...@@ -122,10 +137,16 @@ TEST_F(BadgeManagerUnittest, SetBadgeForAppAfterClear) {
} }
TEST_F(BadgeManagerUnittest, ClearBadgeForBadgedApp) { TEST_F(BadgeManagerUnittest, ClearBadgeForBadgedApp) {
ukm::TestUkmRecorder test_recorder;
badge_manager()->SetBadgeForTesting(kAppId, badge_manager()->SetBadgeForTesting(kAppId,
base::make_optional(kBadgeContents)); base::make_optional(kBadgeContents));
badge_manager()->ClearBadgeForTesting(kAppId); badge_manager()->ClearBadgeForTesting(kAppId, &test_recorder);
auto entries =
test_recorder.GetEntriesByName(ukm::builders::Badging::kEntryName);
ASSERT_EQ(entries.size(), 1u);
test_recorder.ExpectEntryMetric(
entries[0], ukm::builders::Badging::kUpdateAppBadgeName, kClearBadge);
EXPECT_EQ(1UL, delegate()->cleared_badges().size()); EXPECT_EQ(1UL, delegate()->cleared_badges().size());
EXPECT_EQ(kAppId, delegate()->cleared_badges().front()); EXPECT_EQ(kAppId, delegate()->cleared_badges().front());
} }
......
...@@ -19,6 +19,9 @@ namespace app_list { ...@@ -19,6 +19,9 @@ namespace app_list {
class AppLaunchEventLogger; class AppLaunchEventLogger;
} // namespace app_list } // namespace app_list
namespace badging {
class BadgeManager;
} // namespace badging
namespace ukm { namespace ukm {
const base::Feature kUkmAppLogging{"UkmAppLogging", const base::Feature kUkmAppLogging{"UkmAppLogging",
...@@ -30,6 +33,8 @@ class AppSourceUrlRecorder { ...@@ -30,6 +33,8 @@ class AppSourceUrlRecorder {
friend class app_list::AppLaunchEventLogger; friend class app_list::AppLaunchEventLogger;
friend class badging::BadgeManager;
// TODO(lukasza): https://crbug.com/920638: Remove the friendship declaration // TODO(lukasza): https://crbug.com/920638: Remove the friendship declaration
// below, after gathering sufficient data for the // below, after gathering sufficient data for the
// Extensions.CrossOriginFetchFromContentScript3 metric (possibly as early as // Extensions.CrossOriginFetchFromContentScript3 metric (possibly as early as
......
...@@ -66556,6 +66556,12 @@ Full version information for the fingerprint enum values: ...@@ -66556,6 +66556,12 @@ Full version information for the fingerprint enum values:
<int value="3" label="No app shim"/> <int value="3" label="No app shim"/>
</enum> </enum>
<enum name="UpdateAppBadgeTypes">
<int value="0" label="Set numeric badge"/>
<int value="1" label="Set flag badge"/>
<int value="2" label="Clear badge"/>
</enum>
<enum name="UpdateCheckStatus"> <enum name="UpdateCheckStatus">
<int value="0" label="Unknown"/> <int value="0" label="Unknown"/>
<int value="1" label="Ok"/> <int value="1" label="Ok"/>
...@@ -1593,6 +1593,27 @@ be describing additional metrics about the same event. ...@@ -1593,6 +1593,27 @@ be describing additional metrics about the same event.
</metric> </metric>
</event> </event>
<event name="Badging">
<owner>phillis@chromium.org</owner>
<owner>cmumford@chromium.org</owner>
<summary>
Metrics that measure the use of Badging API
</summary>
<metric name="UpdateAppBadge" enum="UpdateAppBadgeTypes">
<summary>
Logged when navigator.setAppBadge or navigator.clearAppBadge is called.
</summary>
<aggregation>
<history>
<index fields="profile.country"/>
<statistics>
<quantiles type="enumeration"/>
</statistics>
</history>
</aggregation>
</metric>
</event>
<event name="Blink.FindInPage" singular="True"> <event name="Blink.FindInPage" singular="True">
<owner>vmpstr@chromium.org</owner> <owner>vmpstr@chromium.org</owner>
<owner>chrishtr@chromium.org</owner> <owner>chrishtr@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