Commit 01155337 authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

[ContentIndex] Collect UKM events.

UKM collection review doc (Google-only):
https://docs.google.com/document/d/17roFDpjKXeglB9_dzfX7uGykoWeRCVTa_4SDDJyv04Y/

Bug: 973844
Change-Id: I2a8d36286ff7b0472c64313b222f7404f88e241e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724077Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarRichard Knoll <knollr@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684408}
parent 7c0d5728
...@@ -20,8 +20,10 @@ ...@@ -20,8 +20,10 @@
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "components/offline_items_collection/core/offline_content_provider.h" #include "components/offline_items_collection/core/offline_content_provider.h"
#include "components/offline_items_collection/core/offline_item.h" #include "components/offline_items_collection/core/offline_item.h"
#include "components/ukm/test_ukm_recorder.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "third_party/blink/public/common/service_worker/service_worker_status_code.h" #include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
#include "third_party/re2/src/re2/re2.h" #include "third_party/re2/src/re2/re2.h"
...@@ -267,7 +269,7 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UserDeletedEntryDispatchesEvent) { ...@@ -267,7 +269,7 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UserDeletedEntryDispatchesEvent) {
EXPECT_TRUE(GetAllItems().empty()); EXPECT_TRUE(GetAllItems().empty());
} }
IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) { IN_PROC_BROWSER_TEST_F(ContentIndexTest, MetricsCollected) {
// Inititally there is no content. // Inititally there is no content.
{ {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
...@@ -279,12 +281,20 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) { ...@@ -279,12 +281,20 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) {
// Record that two articles were added. // Record that two articles were added.
{ {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
ukm::TestAutoSetUkmRecorder ukm_recorder;
RunScript("AddContent('my-id-1')"); RunScript("AddContent('my-id-1')");
RunScript("AddContent('my-id-2')"); RunScript("AddContent('my-id-2')");
base::RunLoop() base::RunLoop()
.RunUntilIdle(); // Wait for the provider to get the content. .RunUntilIdle(); // Wait for the provider to get the content.
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"ContentIndex.ContentAdded", blink::mojom::ContentCategory::ARTICLE, 2); "ContentIndex.ContentAdded", blink::mojom::ContentCategory::ARTICLE, 2);
EXPECT_EQ(
ukm_recorder
.GetEntriesByName(ukm::builders::ContentIndex_Added::kEntryName)
.size(),
2u);
} }
// Querying the items should record that there are 2 entries available. // Querying the items should record that there are 2 entries available.
...@@ -298,6 +308,8 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) { ...@@ -298,6 +308,8 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) {
// User deletion will dispatch an event. // User deletion will dispatch an event.
{ {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
ukm::TestAutoSetUkmRecorder ukm_recorder;
provider()->RemoveItem(offline_items().at("my-id-1").id); provider()->RemoveItem(offline_items().at("my-id-1").id);
EXPECT_EQ(RunScript("waitForMessageFromServiceWorker()"), "my-id-1"); EXPECT_EQ(RunScript("waitForMessageFromServiceWorker()"), "my-id-1");
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
...@@ -309,11 +321,18 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) { ...@@ -309,11 +321,18 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) {
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
"ContentIndex.ContentDeleteEvent.Dispatch", "ContentIndex.ContentDeleteEvent.Dispatch",
blink::ServiceWorkerStatusCode::kOk, 1); blink::ServiceWorkerStatusCode::kOk, 1);
EXPECT_EQ(ukm_recorder
.GetEntriesByName(
ukm::builders::ContentIndex_DeletedByUser::kEntryName)
.size(),
1u);
} }
// Opening an article is recorded. // Opening an article is recorded.
{ {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
ukm::TestAutoSetUkmRecorder ukm_recorder;
provider()->OpenItem( provider()->OpenItem(
offline_items_collection::LaunchLocation::DOWNLOAD_HOME, offline_items_collection::LaunchLocation::DOWNLOAD_HOME,
offline_items().at("my-id-2").id); offline_items().at("my-id-2").id);
...@@ -326,6 +345,12 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) { ...@@ -326,6 +345,12 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, UmaCollected) {
histogram_tester.ExpectBucketCount("ContentIndex.ContentOpened", histogram_tester.ExpectBucketCount("ContentIndex.ContentOpened",
blink::mojom::ContentCategory::ARTICLE, blink::mojom::ContentCategory::ARTICLE,
1); 1);
EXPECT_EQ(
ukm_recorder
.GetEntriesByName(ukm::builders::ContentIndex_Opened::kEntryName)
.size(),
1u);
} }
} }
......
...@@ -5,19 +5,70 @@ ...@@ -5,19 +5,70 @@
#include "chrome/browser/content_index/content_index_metrics.h" #include "chrome/browser/content_index/content_index_metrics.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
#include "chrome/browser/metrics/ukm_background_recorder_service.h"
#include "content/public/browser/web_contents.h"
#include "net/base/network_change_notifier.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
namespace content_index { namespace {
void RecordContentAdded(blink::mojom::ContentCategory category) { void MaybeRecordUkmContentAdded(blink::mojom::ContentCategory category,
base::Optional<ukm::SourceId> source_id) {
if (!source_id)
return;
ukm::builders::ContentIndex_Added(*source_id)
.SetCategory(static_cast<int>(category))
.Record(ukm::UkmRecorder::Get());
}
void MaybeRecordUkmContentDeletedByUser(
base::Optional<ukm::SourceId> source_id) {
if (!source_id)
return;
ukm::builders::ContentIndex_DeletedByUser(*source_id)
.SetDeleted(true)
.Record(ukm::UkmRecorder::Get());
}
} // namespace
ContentIndexMetrics::ContentIndexMetrics(
ukm::UkmBackgroundRecorderService* ukm_background_service)
: ukm_background_service_(ukm_background_service) {
DCHECK(ukm_background_service_);
}
ContentIndexMetrics::~ContentIndexMetrics() = default;
void ContentIndexMetrics::RecordContentAdded(
const url::Origin& origin,
blink::mojom::ContentCategory category) {
base::UmaHistogramEnumeration("ContentIndex.ContentAdded", category); base::UmaHistogramEnumeration("ContentIndex.ContentAdded", category);
ukm_background_service_->GetBackgroundSourceIdIfAllowed(
origin, base::BindOnce(&MaybeRecordUkmContentAdded, category));
} }
void RecordContentOpened(blink::mojom::ContentCategory category) { void ContentIndexMetrics::RecordContentOpened(
content::WebContents* web_contents,
blink::mojom::ContentCategory category) {
base::UmaHistogramEnumeration("ContentIndex.ContentOpened", category); base::UmaHistogramEnumeration("ContentIndex.ContentOpened", category);
ukm::builders::ContentIndex_Opened(web_contents->GetLastCommittedSourceId())
.SetIsOffline(net::NetworkChangeNotifier::IsOffline())
.Record(ukm::UkmRecorder::Get());
} }
void RecordContentIndexEntries(size_t num_entries) { void ContentIndexMetrics::RecordContentDeletedByUser(
base::UmaHistogramCounts1000("ContentIndex.NumEntriesAvailable", num_entries); const url::Origin& origin) {
ukm_background_service_->GetBackgroundSourceIdIfAllowed(
origin, base::BindOnce(&MaybeRecordUkmContentDeletedByUser));
} }
} // namespace content_index // static
void ContentIndexMetrics::RecordContentIndexEntries(size_t num_entries) {
base::UmaHistogramCounts1000("ContentIndex.NumEntriesAvailable", num_entries);
}
...@@ -5,19 +5,45 @@ ...@@ -5,19 +5,45 @@
#ifndef CHROME_BROWSER_CONTENT_INDEX_CONTENT_INDEX_METRICS_H_ #ifndef CHROME_BROWSER_CONTENT_INDEX_CONTENT_INDEX_METRICS_H_
#define CHROME_BROWSER_CONTENT_INDEX_CONTENT_INDEX_METRICS_H_ #define CHROME_BROWSER_CONTENT_INDEX_CONTENT_INDEX_METRICS_H_
#include "base/macros.h"
#include "third_party/blink/public/mojom/content_index/content_index.mojom.h" #include "third_party/blink/public/mojom/content_index/content_index.mojom.h"
namespace content_index { namespace content {
class WebContents;
} // namespace content
// Records the category of the Content Index entry after registration. namespace ukm {
void RecordContentAdded(blink::mojom::ContentCategory category); class UkmBackgroundRecorderService;
} // namespace ukm
// Records the category of the Content Index entry when a user opens it. namespace url {
void RecordContentOpened(blink::mojom::ContentCategory category); class Origin;
} // namespace url
// Records the number of Content Index entries available when requested. class ContentIndexMetrics {
void RecordContentIndexEntries(size_t num_entries); public:
explicit ContentIndexMetrics(
ukm::UkmBackgroundRecorderService* ukm_background_service);
~ContentIndexMetrics();
} // namespace content_index // Records the category of the Content Index entry after registration.
void RecordContentAdded(const url::Origin& origin,
blink::mojom::ContentCategory category);
// Records the category of the Content Index entry when a user opens it.
void RecordContentOpened(content::WebContents* web_contents,
blink::mojom::ContentCategory category);
// Records when a Content UIndex entry is deleted by a user.
void RecordContentDeletedByUser(const url::Origin& origin);
// Records the number of Content Index entries available when requested.
static void RecordContentIndexEntries(size_t num_entries);
private:
ukm::UkmBackgroundRecorderService* ukm_background_service_;
DISALLOW_COPY_AND_ASSIGN(ContentIndexMetrics);
};
#endif // CHROME_BROWSER_CONTENT_INDEX_CONTENT_INDEX_METRICS_H_ #endif // CHROME_BROWSER_CONTENT_INDEX_CONTENT_INDEX_METRICS_H_
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "chrome/browser/content_index/content_index_provider_factory.h" #include "chrome/browser/content_index/content_index_provider_factory.h"
#include "chrome/browser/content_index/content_index_provider_impl.h" #include "chrome/browser/content_index/content_index_provider_impl.h"
#include "chrome/browser/metrics/ukm_background_recorder_service.h"
#include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h" #include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
#include "chrome/browser/profiles/incognito_helpers.h" #include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -27,6 +28,7 @@ ContentIndexProviderFactory::ContentIndexProviderFactory() ...@@ -27,6 +28,7 @@ ContentIndexProviderFactory::ContentIndexProviderFactory()
"ContentIndexProvider", "ContentIndexProvider",
BrowserContextDependencyManager::GetInstance()) { BrowserContextDependencyManager::GetInstance()) {
DependsOn(OfflineContentAggregatorFactory::GetInstance()); DependsOn(OfflineContentAggregatorFactory::GetInstance());
DependsOn(ukm::UkmBackgroundRecorderFactory::GetInstance());
} }
ContentIndexProviderFactory::~ContentIndexProviderFactory() = default; ContentIndexProviderFactory::~ContentIndexProviderFactory() = default;
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/content_index/content_index_metrics.h" #include "chrome/browser/metrics/ukm_background_recorder_service.h"
#include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h" #include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "components/offline_items_collection/core/offline_content_aggregator.h" #include "components/offline_items_collection/core/offline_content_aggregator.h"
...@@ -129,7 +129,7 @@ void DidGetAllEntries(base::OnceClosure done_closure, ...@@ -129,7 +129,7 @@ void DidGetAllEntries(base::OnceClosure done_closure,
void DidGetAllEntriesAcrossStorageParitions( void DidGetAllEntriesAcrossStorageParitions(
std::unique_ptr<ContentIndexProviderImpl::OfflineItemList> item_list, std::unique_ptr<ContentIndexProviderImpl::OfflineItemList> item_list,
ContentIndexProviderImpl::MultipleItemCallback callback) { ContentIndexProviderImpl::MultipleItemCallback callback) {
content_index::RecordContentIndexEntries(item_list->size()); ContentIndexMetrics::RecordContentIndexEntries(item_list->size());
std::move(callback).Run(*item_list); std::move(callback).Run(*item_list);
} }
...@@ -137,8 +137,9 @@ void DidGetAllEntriesAcrossStorageParitions( ...@@ -137,8 +137,9 @@ void DidGetAllEntriesAcrossStorageParitions(
ContentIndexProviderImpl::ContentIndexProviderImpl(Profile* profile) ContentIndexProviderImpl::ContentIndexProviderImpl(Profile* profile)
: profile_(profile), : profile_(profile),
aggregator_(OfflineContentAggregatorFactory::GetForKey( metrics_(ukm::UkmBackgroundRecorderFactory::GetForProfile(profile)),
profile_->GetProfileKey())), aggregator_(
OfflineContentAggregatorFactory::GetForKey(profile->GetProfileKey())),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
aggregator_->RegisterProvider(kProviderNamespace, this); aggregator_->RegisterProvider(kProviderNamespace, this);
} }
...@@ -166,7 +167,8 @@ void ContentIndexProviderImpl::OnContentAdded( ...@@ -166,7 +167,8 @@ void ContentIndexProviderImpl::OnContentAdded(
for (auto& observer : observers_) for (auto& observer : observers_)
observer.OnItemsAdded(items); observer.OnItemsAdded(items);
content_index::RecordContentAdded(entry.description->category); metrics_.RecordContentAdded(url::Origin::Create(entry.launch_url.GetOrigin()),
entry.description->category);
} }
void ContentIndexProviderImpl::OnContentDeleted( void ContentIndexProviderImpl::OnContentDeleted(
...@@ -208,15 +210,21 @@ void ContentIndexProviderImpl::DidGetEntryToOpen( ...@@ -208,15 +210,21 @@ void ContentIndexProviderImpl::DidGetEntryToOpen(
WindowOpenDisposition::NEW_FOREGROUND_TAB, WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui::PAGE_TRANSITION_LINK, ui::PAGE_TRANSITION_LINK,
/* is_renderer_initiated= */ false); /* is_renderer_initiated= */ false);
ServiceTabLauncher::GetInstance()->LaunchTab(profile_, params, ServiceTabLauncher::GetInstance()->LaunchTab(
base::DoNothing()); profile_, params,
base::BindOnce(&ContentIndexProviderImpl::DidOpenTab,
weak_ptr_factory_.GetWeakPtr(), std::move(*entry)));
#else #else
NavigateParams nav_params(profile_, entry->launch_url, NavigateParams nav_params(profile_, entry->launch_url,
ui::PAGE_TRANSITION_LINK); ui::PAGE_TRANSITION_LINK);
Navigate(&nav_params); Navigate(&nav_params);
DidOpenTab(std::move(*entry), nav_params.navigated_or_inserted_contents);
#endif #endif
}
content_index::RecordContentOpened(entry->description->category); void ContentIndexProviderImpl::DidOpenTab(content::ContentIndexEntry entry,
content::WebContents* web_contents) {
metrics_.RecordContentOpened(web_contents, entry.description->category);
} }
void ContentIndexProviderImpl::RemoveItem(const ContentId& id) { void ContentIndexProviderImpl::RemoveItem(const ContentId& id) {
...@@ -228,6 +236,8 @@ void ContentIndexProviderImpl::RemoveItem(const ContentId& id) { ...@@ -228,6 +236,8 @@ void ContentIndexProviderImpl::RemoveItem(const ContentId& id) {
if (!storage_partition || !storage_partition->GetContentIndexContext()) if (!storage_partition || !storage_partition->GetContentIndexContext())
return; return;
metrics_.RecordContentDeletedByUser(components.origin);
storage_partition->GetContentIndexContext()->OnUserDeletedItem( storage_partition->GetContentIndexContext()->OnUserDeletedItem(
components.service_worker_registration_id, components.origin, components.service_worker_registration_id, components.origin,
components.description_id); components.description_id);
......
...@@ -11,11 +11,16 @@ ...@@ -11,11 +11,16 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "base/optional.h" #include "base/optional.h"
#include "chrome/browser/content_index/content_index_metrics.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "components/offline_items_collection/core/offline_content_provider.h" #include "components/offline_items_collection/core/offline_content_provider.h"
#include "components/offline_items_collection/core/offline_item.h" #include "components/offline_items_collection/core/offline_item.h"
#include "content/public/browser/content_index_provider.h" #include "content/public/browser/content_index_provider.h"
namespace content {
class WebContents;
} // namespace content
namespace offline_items_collection { namespace offline_items_collection {
class OfflineContentAggregator; class OfflineContentAggregator;
} // namespace offline_items_collection } // namespace offline_items_collection
...@@ -68,8 +73,11 @@ class ContentIndexProviderImpl ...@@ -68,8 +73,11 @@ class ContentIndexProviderImpl
VisualsCallback callback, VisualsCallback callback,
std::vector<SkBitmap> icons); std::vector<SkBitmap> icons);
void DidGetEntryToOpen(base::Optional<content::ContentIndexEntry> entry); void DidGetEntryToOpen(base::Optional<content::ContentIndexEntry> entry);
void DidOpenTab(content::ContentIndexEntry entry,
content::WebContents* web_contents);
Profile* profile_; Profile* profile_;
ContentIndexMetrics metrics_;
offline_items_collection::OfflineContentAggregator* aggregator_; offline_items_collection::OfflineContentAggregator* aggregator_;
base::ObserverList<Observer>::Unchecked observers_; base::ObserverList<Observer>::Unchecked observers_;
base::WeakPtrFactory<ContentIndexProviderImpl> weak_ptr_factory_; base::WeakPtrFactory<ContentIndexProviderImpl> weak_ptr_factory_;
......
...@@ -37,6 +37,8 @@ class ContentIndexProviderImplTest : public testing::Test, ...@@ -37,6 +37,8 @@ class ContentIndexProviderImplTest : public testing::Test,
public OfflineContentProvider::Observer { public OfflineContentProvider::Observer {
public: public:
void SetUp() override { void SetUp() override {
ASSERT_TRUE(profile_.CreateHistoryService(/* delete_file= */ true,
/* no_db= */ false));
provider_ = std::make_unique<ContentIndexProviderImpl>(&profile_); provider_ = std::make_unique<ContentIndexProviderImpl>(&profile_);
provider_->AddObserver(this); provider_->AddObserver(this);
} }
......
...@@ -2412,6 +2412,44 @@ be describing additional metrics about the same event. ...@@ -2412,6 +2412,44 @@ be describing additional metrics about the same event.
</metric> </metric>
</event> </event>
<event name="ContentIndex.Added">
<owner>rayankans@chromium.org</owner>
<summary>
Collected when a new Content Index entry is registered.
</summary>
<metric name="Category" enum="ContentIndexCategory">
<summary>
The type of the content registered. Corresponds to
blink.mojom.ContentCategory.
</summary>
</metric>
</event>
<event name="ContentIndex.DeletedByUser">
<owner>rayankans@chromium.org</owner>
<summary>
Collected when a Content Index entry is deleted by the user from the
Downloads page.
</summary>
<metric name="Deleted" enum="Boolean">
<summary>
Always true.
</summary>
</metric>
</event>
<event name="ContentIndex.Opened">
<owner>rayankans@chromium.org</owner>
<summary>
Collected when a Content Index entry is opened from the Downloads page.
</summary>
<metric name="IsOffline" enum="Boolean">
<summary>
Whether the browser was offline when the content was opened.
</summary>
</metric>
</event>
<event name="ContextualSearch"> <event name="ContextualSearch">
<owner>donnd@chromium.org</owner> <owner>donnd@chromium.org</owner>
<summary> <summary>
......
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