Add notifications to UKM.xml.
Added notifications and related metrics to ukm.xml and included logging
of some metrics that have been recorded to the database already,
including the number of clicks, number of action button clicks and the
ClosedReason for the notification.
UKM Collection Review:
https://docs.google.com/document/d/1oUfHs_t5gEOhMYvjoC2so2Ie_Pt51m12XIgEpbzfU1k/edit?usp=sharing
Bug: 842622
Change-Id: Iabe9e3065e5d0c5c677545873d2e1ea630aea45d
Reviewed-on: https://chromium-review.googlesource.com/1071649
Commit-Queue: Sharon Yang <yangsharon@google.com>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Robert Kaplow (OOO until 0730) <rkaplow@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578048}diff --git a/chrome/browser/notifications/platform_notification_service_impl.cc b/chrome/browser/notifications/platform_notification_service_impl.cc
index bb6e49e..010a2e25 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.cc
+++ b/chrome/browser/notifications/platform_notification_service_impl.cc
@@ -11,11 +11,13 @@
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/task_scheduler/post_task.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/engagement/site_engagement_service.h"
+#include "chrome/browser/history/history_service_factory.h"
#include "chrome/browser/notifications/metrics/notification_metrics_logger.h"
#include "chrome/browser/notifications/metrics/notification_metrics_logger_factory.h"
#include "chrome/browser/notifications/notification_common.h"
@@ -52,6 +54,7 @@
#include "content/public/common/notification_resources.h"
#include "content/public/common/platform_notification_data.h"
#include "extensions/buildflags/buildflags.h"
+#include "services/metrics/public/cpp/ukm_builders.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
@@ -74,6 +77,7 @@
using content::BrowserContext;
using content::BrowserThread;
+using content::NotificationDatabaseData;
using message_center::NotifierId;
namespace {
@@ -340,6 +344,32 @@
return next_id;
}
+void PlatformNotificationServiceImpl::RecordNotificationUkmEvent(
+ BrowserContext* browser_context,
+ const NotificationDatabaseData& data) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ // Only record the event if a user explicitly interacted with the notification
+ // to close it.
+ if (data.closed_reason != NotificationDatabaseData::ClosedReason::USER &&
+ data.num_clicks == 0 && data.num_action_button_clicks == 0) {
+ return;
+ }
+
+ // Query the HistoryService so we only record a notification if the origin is
+ // in the user's history.
+ Profile* profile = Profile::FromBrowserContext(browser_context);
+ history::HistoryService* history_service =
+ HistoryServiceFactory::GetForProfile(profile,
+ ServiceAccessType::EXPLICIT_ACCESS);
+ DCHECK(history_service);
+ history_service->QueryURL(
+ data.origin, /*want_visits=*/false,
+ base::BindOnce(
+ &PlatformNotificationServiceImpl::OnUrlHistoryQueryComplete,
+ base::Unretained(this), data),
+ &task_tracker_);
+}
+
void PlatformNotificationServiceImpl::OnClickEventDispatchComplete(
base::OnceClosure completed_closure,
content::PersistentNotificationStatus status) {
@@ -368,6 +398,38 @@
std::move(completed_closure).Run();
}
+void PlatformNotificationServiceImpl::OnUrlHistoryQueryComplete(
+ const content::NotificationDatabaseData& data,
+ bool found_url,
+ const history::URLRow& url_row,
+ const history::VisitVector& visits) {
+ // Post the |history_query_complete_closure_for_testing_| closure to the
+ // current task runner to inform tests that the history query has completed.
+ if (history_query_complete_closure_for_testing_) {
+ base::PostTask(FROM_HERE,
+ std::move(history_query_complete_closure_for_testing_));
+ }
+
+ // Only record the notification if the |data.origin| is in the history
+ // service.
+ if (!found_url)
+ return;
+
+ ukm::UkmRecorder* recorder = ukm::UkmRecorder::Get();
+ DCHECK(recorder);
+ // We are using UpdateSourceURL as notifications are not tied to a navigation.
+ // This is ok from a privacy perspective as we have explicitly verified that
+ // |data.origin| is in the HistoryService before recording to UKM.
+ ukm::SourceId source_id = ukm::UkmRecorder::GetNewSourceID();
+ recorder->UpdateSourceURL(source_id, data.origin);
+
+ ukm::builders::Notification(source_id)
+ .SetClosedReason(static_cast<int>(data.closed_reason))
+ .SetNumClicks(data.num_clicks)
+ .SetNumActionButtonClicks(data.num_action_button_clicks)
+ .Record(recorder);
+}
+
message_center::Notification
PlatformNotificationServiceImpl::CreateNotificationFromData(
Profile* profile,
diff --git a/chrome/browser/notifications/platform_notification_service_impl.h b/chrome/browser/notifications/platform_notification_service_impl.h
index 3b80694..4ec348a 100644
--- a/chrome/browser/notifications/platform_notification_service_impl.h
+++ b/chrome/browser/notifications/platform_notification_service_impl.h
@@ -19,9 +19,12 @@
#include "base/memory/singleton.h"
#include "base/optional.h"
#include "base/strings/string16.h"
+#include "base/task/cancelable_task_tracker.h"
#include "chrome/browser/notifications/notification_common.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/buildflags.h"
+#include "components/history/core/browser/history_service.h"
+#include "components/history/core/browser/history_types.h"
#include "content/public/browser/platform_notification_service.h"
#include "content/public/common/persistent_notification_status.h"
#include "third_party/blink/public/platform/modules/permissions/permission_status.mojom.h"
@@ -95,6 +98,14 @@
const DisplayedNotificationsCallback& callback) override;
int64_t ReadNextPersistentNotificationId(
content::BrowserContext* browser_context) override;
+ void RecordNotificationUkmEvent(
+ content::BrowserContext* browser_context,
+ const content::NotificationDatabaseData& data) override;
+
+ void set_history_query_complete_closure_for_testing(
+ base::OnceClosure closure) {
+ history_query_complete_closure_for_testing_ = std::move(closure);
+ }
private:
friend struct base::DefaultSingletonTraits<PlatformNotificationServiceImpl>;
@@ -105,6 +116,8 @@
CreateNotificationFromData);
FRIEND_TEST_ALL_PREFIXES(PlatformNotificationServiceTest,
DisplayNameForContextMessage);
+ FRIEND_TEST_ALL_PREFIXES(PlatformNotificationServiceTest,
+ RecordNotificationUkmEvent);
PlatformNotificationServiceImpl();
~PlatformNotificationServiceImpl() override;
@@ -115,6 +128,10 @@
void OnCloseEventDispatchComplete(
base::OnceClosure completed_closure,
content::PersistentNotificationStatus status);
+ void OnUrlHistoryQueryComplete(const content::NotificationDatabaseData& data,
+ bool found_url,
+ const history::URLRow& url_row,
+ const history::VisitVector& visits);
// Creates a new Web Notification-based Notification object. Should only be
// called when the notification is first shown.
@@ -149,6 +166,13 @@
// programmatically to avoid dispatching close events for them.
std::unordered_set<std::string> closed_notifications_;
+ // Task tracker used for querying URLs in the history service.
+ base::CancelableTaskTracker task_tracker_;
+
+ // Testing-only closure to observe when querying the history service has been
+ // completed, and the result of logging UKM can be observed.
+ base::OnceClosure history_query_complete_closure_for_testing_;
+
DISALLOW_COPY_AND_ASSIGN(PlatformNotificationServiceImpl);
};
diff --git a/chrome/browser/notifications/platform_notification_service_unittest.cc b/chrome/browser/notifications/platform_notification_service_unittest.cc
index a2c60f1..98e0af0 100644
--- a/chrome/browser/notifications/platform_notification_service_unittest.cc
+++ b/chrome/browser/notifications/platform_notification_service_unittest.cc
@@ -10,22 +10,29 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/feature_list.h"
+#include "base/files/scoped_temp_dir.h"
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "build/build_config.h"
+#include "chrome/browser/history/history_service_factory.h"
#include "chrome/browser/notifications/metrics/mock_notification_metrics_logger.h"
#include "chrome/browser/notifications/metrics/notification_metrics_logger_factory.h"
#include "chrome/browser/notifications/notification_display_service_tester.h"
#include "chrome/browser/notifications/platform_notification_service_impl.h"
#include "chrome/common/chrome_features.h"
#include "chrome/test/base/testing_profile.h"
+#include "components/history/core/browser/history_database_params.h"
+#include "components/history/core/browser/history_service.h"
+#include "components/history/core/test/test_history_database.h"
+#include "components/ukm/test_ukm_recorder.h"
#include "content/public/browser/permission_type.h"
#include "content/public/common/notification_resources.h"
#include "content/public/common/platform_notification_data.h"
#include "content/public/test/mock_permission_manager.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/buildflags/buildflags.h"
+#include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/platform/modules/permissions/permission_status.mojom.h"
@@ -43,12 +50,16 @@
using ::testing::Return;
using content::NotificationResources;
using content::PlatformNotificationData;
+using content::NotificationDatabaseData;
using message_center::Notification;
namespace {
const char kNotificationId[] = "my-notification-id";
const int kNotificationVibrationPattern[] = { 100, 200, 300 };
+const char kClosedReason[] = "ClosedReason";
+const char kNumClicks[] = "NumClicks";
+const char kNumActionButtonClicks[] = "NumActionButtonClicks";
class TestingProfileWithPermissionManager : public TestingProfile {
public:
@@ -91,6 +102,8 @@
NotificationMetricsLoggerFactory::GetInstance()
->SetTestingFactoryAndUse(
&profile_, &MockNotificationMetricsLogger::FactoryForTests));
+
+ recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
}
void TearDown() override {
@@ -125,6 +138,8 @@
// Owned by the |profile_| as a keyed service.
MockNotificationMetricsLogger* mock_logger_;
+
+ std::unique_ptr<ukm::TestAutoSetUkmRecorder> recorder_;
};
TEST_F(PlatformNotificationServiceTest, DisplayNonPersistentThenClose) {
@@ -276,6 +291,107 @@
EXPECT_TRUE(buttons[1].placeholder);
}
+TEST_F(PlatformNotificationServiceTest, RecordNotificationUkmEventHistory) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+ HistoryServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+ &profile_, [](content::BrowserContext* context) {
+ return static_cast<std::unique_ptr<KeyedService>>(
+ std::make_unique<history::HistoryService>());
+ });
+
+ history::HistoryService* history_service =
+ HistoryServiceFactory::GetForProfile(&profile_,
+ ServiceAccessType::EXPLICIT_ACCESS);
+
+ // Initialize the |history_service| based on our |temp_dir|.
+ history_service->Init(
+ history::TestHistoryDatabaseParamsForPath(temp_dir.GetPath()));
+
+ NotificationDatabaseData data;
+ data.closed_reason = NotificationDatabaseData::ClosedReason::USER;
+ data.origin = GURL("https://chrome.com/");
+
+ size_t initial_entries_count = recorder_->entries_count();
+ size_t expected_entries_count = initial_entries_count + 1;
+
+ // First attempt to record an event for |data.origin| before it has been added
+ // to the |history_service|. Nothing should be recorded.
+ service()->RecordNotificationUkmEvent(&profile_, data);
+ {
+ base::RunLoop run_loop;
+ service()->set_history_query_complete_closure_for_testing(
+ run_loop.QuitClosure());
+ run_loop.Run();
+ }
+
+ EXPECT_EQ(recorder_->entries_count(), initial_entries_count);
+
+ // Now add |data.origin| to the |history_service|. After this, notification
+ // events being logged should end up in UKM.
+ history_service->AddPage(data.origin, base::Time::Now(),
+ history::SOURCE_BROWSED);
+
+ service()->RecordNotificationUkmEvent(&profile_, data);
+ {
+ base::RunLoop run_loop;
+ service()->set_history_query_complete_closure_for_testing(
+ run_loop.QuitClosure());
+ run_loop.Run();
+ }
+
+ EXPECT_EQ(recorder_->entries_count(), expected_entries_count);
+
+ // Delete the |data.origin| from the |history_service|. Subsequent events
+ // should not be logged to UKM anymore.
+ history_service->DeleteURL(data.origin);
+
+ service()->RecordNotificationUkmEvent(&profile_, data);
+ {
+ base::RunLoop run_loop;
+ service()->set_history_query_complete_closure_for_testing(
+ run_loop.QuitClosure());
+ run_loop.Run();
+ }
+
+ EXPECT_EQ(recorder_->entries_count(), expected_entries_count);
+}
+
+TEST_F(PlatformNotificationServiceTest, RecordNotificationUkmEvent) {
+ NotificationDatabaseData data;
+ data.notification_id = "notification1";
+ data.closed_reason = NotificationDatabaseData::ClosedReason::USER;
+ data.num_clicks = 3;
+ data.num_action_button_clicks = 1;
+ history::URLRow url_row;
+ history::VisitVector visits;
+
+ // The history service does not find the given URL and nothing is recorded.
+ service()->OnUrlHistoryQueryComplete(data, false, url_row, visits);
+ std::vector<const ukm::mojom::UkmEntry*> entries =
+ recorder_->GetEntriesByName(ukm::builders::Notification::kEntryName);
+ EXPECT_EQ(0u, entries.size());
+
+ // The history service finds the given URL and the notification is logged.
+ service()->OnUrlHistoryQueryComplete(data, true, url_row, visits);
+ auto* entry =
+ recorder_->GetEntriesByName(ukm::builders::Notification::kEntryName)[0];
+ recorder_->ExpectEntryMetric(
+ entry, kClosedReason,
+ static_cast<int>(NotificationDatabaseData::ClosedReason::USER));
+ recorder_->ExpectEntryMetric(entry, kNumClicks, 3);
+ recorder_->ExpectEntryMetric(entry, kNumActionButtonClicks, 1);
+}
+
+// Expect each call to ReadNextPersistentNotificationId to return a larger
+// value.
+TEST_F(PlatformNotificationServiceTest, NextPersistentNotificationId) {
+ int64_t first_id = service()->ReadNextPersistentNotificationId(&profile_);
+ int64_t second_id = service()->ReadNextPersistentNotificationId(&profile_);
+ EXPECT_LT(first_id, second_id);
+}
+
#if BUILDFLAG(ENABLE_EXTENSIONS)
TEST_F(PlatformNotificationServiceTest, DisplayNameForContextMessage) {
@@ -341,12 +457,4 @@
base::UTF16ToUTF8(notification.context_message()));
}
-// Expect each call to ReadNextPersistentNotificationId to return a larger
-// value.
-TEST_F(PlatformNotificationServiceTest, NextPersistentNotificationId) {
- int64_t first_id = service()->ReadNextPersistentNotificationId(&profile_);
- int64_t second_id = service()->ReadNextPersistentNotificationId(&profile_);
- EXPECT_LT(first_id, second_id);
-}
-
#endif // BUILDFLAG(ENABLE_EXTENSIONS)
diff --git a/content/browser/notifications/notification_database.cc b/content/browser/notifications/notification_database.cc
index 28ff2f4..afc494d 100644
--- a/content/browser/notifications/notification_database.cc
+++ b/content/browser/notifications/notification_database.cc
@@ -107,8 +107,9 @@
} // namespace
-NotificationDatabase::NotificationDatabase(const base::FilePath& path)
- : path_(path) {}
+NotificationDatabase::NotificationDatabase(const base::FilePath& path,
+ UkmCallback callback)
+ : path_(path), record_notification_to_ukm_callback_(std::move(callback)) {}
NotificationDatabase::~NotificationDatabase() {
DCHECK(sequence_checker_.CalledOnValidSequence());
@@ -182,11 +183,13 @@
// Update the appropriate fields for UKM logging purposes.
switch (interaction) {
case PlatformNotificationContext::Interaction::CLOSED:
+ notification_database_data->closed_reason =
+ NotificationDatabaseData::ClosedReason::USER;
notification_database_data->time_until_close_millis =
base::Time::Now() - notification_database_data->creation_time_millis;
break;
case PlatformNotificationContext::Interaction::NONE:
- return status;
+ break;
case PlatformNotificationContext::Interaction::ACTION_BUTTON_CLICKED:
notification_database_data->num_action_button_clicks += 1;
UpdateNotificationClickTimestamps(notification_database_data);
@@ -262,6 +265,13 @@
DCHECK(!notification_id.empty());
DCHECK(origin.is_valid());
+ NotificationDatabaseData data;
+ Status status = ReadNotificationData(notification_id, origin, &data);
+ if (status == STATUS_OK && record_notification_to_ukm_callback_) {
+ BrowserThread::PostTask(
+ BrowserThread::UI, FROM_HERE,
+ base::BindOnce(record_notification_to_ukm_callback_, data));
+ }
std::string key = CreateDataKey(origin, notification_id);
return LevelDBStatusToNotificationDatabaseStatus(
db_->Delete(leveldb::WriteOptions(), key));
@@ -381,6 +391,13 @@
continue;
}
+ if (record_notification_to_ukm_callback_) {
+ BrowserThread::PostTask(
+ BrowserThread::UI, FROM_HERE,
+ base::BindOnce(record_notification_to_ukm_callback_,
+ notification_database_data));
+ }
+
batch.Delete(iter->key());
DCHECK(!notification_database_data.notification_id.empty());
diff --git a/content/browser/notifications/notification_database.h b/content/browser/notifications/notification_database.h
index 5237a8e3..6ed4658 100644
--- a/content/browser/notifications/notification_database.h
+++ b/content/browser/notifications/notification_database.h
@@ -14,8 +14,8 @@
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/sequence_checker.h"
-#include "content/browser/notifications/platform_notification_context_impl.h"
#include "content/common/content_export.h"
+#include "content/public/browser/platform_notification_context.h"
class GURL;
@@ -23,7 +23,7 @@
class DB;
class Env;
class FilterPolicy;
-}
+} // namespace leveldb
namespace content {
@@ -40,6 +40,9 @@
// file I/O. The same thread or task runner must be used for all method calls.
class CONTENT_EXPORT NotificationDatabase {
public:
+ using UkmCallback =
+ base::RepeatingCallback<void(const NotificationDatabaseData&)>;
+
// Result status codes for interations with the database. Will be used for
// UMA, so the assigned ids must remain stable.
enum Status {
@@ -69,7 +72,8 @@
STATUS_COUNT = 7
};
- explicit NotificationDatabase(const base::FilePath& path);
+ NotificationDatabase(const base::FilePath& path, UkmCallback callback);
+
~NotificationDatabase();
// Opens the database. If |path| is non-empty, it will be created on the given
@@ -212,6 +216,9 @@
base::SequenceChecker sequence_checker_;
+ // Callback to use for recording UKM metrics. Must be posted to the UI thread.
+ UkmCallback record_notification_to_ukm_callback_;
+
DISALLOW_COPY_AND_ASSIGN(NotificationDatabase);
};
diff --git a/content/browser/notifications/notification_database_unittest.cc b/content/browser/notifications/notification_database_unittest.cc
index f6fdef1..fb0d6cb 100644
--- a/content/browser/notifications/notification_database_unittest.cc
+++ b/content/browser/notifications/notification_database_unittest.cc
@@ -14,6 +14,7 @@
#include "base/strings/utf_string_conversions.h"
#include "content/public/browser/notification_database_data.h"
#include "content/public/common/platform_notification_data.h"
+#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/leveldatabase/src/include/leveldb/db.h"
#include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
@@ -39,15 +40,19 @@
{"https://chrome.com", "foo" /* tag */, 0}};
class NotificationDatabaseTest : public ::testing::Test {
+ public:
+ NotificationDatabaseTest()
+ : thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {}
+
protected:
// Creates a new NotificationDatabase instance in memory.
NotificationDatabase* CreateDatabaseInMemory() {
- return new NotificationDatabase(base::FilePath());
+ return new NotificationDatabase(base::FilePath(), callback());
}
// Creates a new NotificationDatabase instance in |path|.
NotificationDatabase* CreateDatabaseOnFileSystem(const base::FilePath& path) {
- return new NotificationDatabase(path);
+ return new NotificationDatabase(path, callback());
}
// Creates a new notification for |service_worker_registration_id| belonging
@@ -108,6 +113,12 @@
// Generates a random notification ID. The format of the ID is opaque.
std::string GenerateNotificationId() { return base::GenerateGUID(); }
+
+ NotificationDatabase::UkmCallback callback() { return callback_; }
+
+ TestBrowserThreadBundle thread_bundle_; // Must be first member.
+
+ NotificationDatabase::UkmCallback callback_;
};
TEST_F(NotificationDatabaseTest, OpenCloseMemory) {
diff --git a/content/browser/notifications/platform_notification_context_impl.cc b/content/browser/notifications/platform_notification_context_impl.cc
index 627e8db..a5c56da 100644
--- a/content/browser/notifications/platform_notification_context_impl.cc
+++ b/content/browser/notifications/platform_notification_context_impl.cc
@@ -66,6 +66,10 @@
browser_context_,
base::Bind(&PlatformNotificationContextImpl::DidGetNotificationsOnUI,
this));
+
+ ukm_callback_ = base::BindRepeating(
+ &PlatformNotificationService::RecordNotificationUkmEvent,
+ base::Unretained(service), browser_context_);
}
void PlatformNotificationContextImpl::DidGetNotificationsOnUI(
@@ -490,7 +494,7 @@
return;
}
- database_.reset(new NotificationDatabase(GetDatabasePath()));
+ database_.reset(new NotificationDatabase(GetDatabasePath(), ukm_callback_));
NotificationDatabase::Status status =
database_->Open(true /* create_if_missing */);
@@ -502,7 +506,7 @@
prune_database_on_open_ = false;
DestroyDatabase();
- database_.reset(new NotificationDatabase(GetDatabasePath()));
+ database_.reset(new NotificationDatabase(GetDatabasePath(), ukm_callback_));
status = database_->Open(true /* create_if_missing */);
// TODO(peter): Find the appropriate UMA to cover in regards to
@@ -513,7 +517,8 @@
// away the contents of the directory and try re-opening the database.
if (status == NotificationDatabase::STATUS_ERROR_CORRUPTED) {
if (DestroyDatabase()) {
- database_.reset(new NotificationDatabase(GetDatabasePath()));
+ database_.reset(
+ new NotificationDatabase(GetDatabasePath(), ukm_callback_));
status = database_->Open(true /* create_if_missing */);
UMA_HISTOGRAM_ENUMERATION(
diff --git a/content/browser/notifications/platform_notification_context_impl.h b/content/browser/notifications/platform_notification_context_impl.h
index 7ea92b6..4ad41a15 100644
--- a/content/browser/notifications/platform_notification_context_impl.h
+++ b/content/browser/notifications/platform_notification_context_impl.h
@@ -16,6 +16,7 @@
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
+#include "content/browser/notifications/notification_database.h"
#include "content/browser/notifications/notification_id_generator.h"
#include "content/browser/service_worker/service_worker_context_core_observer.h"
#include "content/common/content_export.h"
@@ -37,7 +38,6 @@
class BlinkNotificationServiceImpl;
class BrowserContext;
-class NotificationDatabase;
struct NotificationDatabaseData;
class ServiceWorkerContextWrapper;
@@ -212,6 +212,8 @@
// removed when either this class is destroyed or the Mojo pipe disconnects.
std::vector<std::unique_ptr<BlinkNotificationServiceImpl>> services_;
+ NotificationDatabase::UkmCallback ukm_callback_;
+
DISALLOW_COPY_AND_ASSIGN(PlatformNotificationContextImpl);
};
diff --git a/content/public/browser/platform_notification_service.h b/content/public/browser/platform_notification_service.h
index 228fc3024..0e9ce43 100644
--- a/content/public/browser/platform_notification_service.h
+++ b/content/public/browser/platform_notification_service.h
@@ -14,6 +14,7 @@
#include "base/callback_forward.h"
#include "content/common/content_export.h"
+#include "content/public/browser/notification_database_data.h"
#include "third_party/blink/public/platform/modules/permissions/permission_status.mojom.h"
class GURL;
@@ -75,6 +76,11 @@
// increments the value, as it is called once per notification write.
virtual int64_t ReadNextPersistentNotificationId(
BrowserContext* browser_context) = 0;
+
+ // Records a given notification to UKM.
+ virtual void RecordNotificationUkmEvent(
+ BrowserContext* browser_context,
+ const NotificationDatabaseData& data) = 0;
};
} // namespace content
diff --git a/content/test/mock_platform_notification_service.cc b/content/test/mock_platform_notification_service.cc
index 28dc265..b345a74 100644
--- a/content/test/mock_platform_notification_service.cc
+++ b/content/test/mock_platform_notification_service.cc
@@ -100,6 +100,10 @@
return ++next_persistent_notification_id_;
}
+void MockPlatformNotificationService::RecordNotificationUkmEvent(
+ BrowserContext* browser_context,
+ const NotificationDatabaseData& data) {}
+
void MockPlatformNotificationService::SimulateClick(
const std::string& title,
const base::Optional<int>& action_index,
diff --git a/content/test/mock_platform_notification_service.h b/content/test/mock_platform_notification_service.h
index baf24f9..8741d95 100644
--- a/content/test/mock_platform_notification_service.h
+++ b/content/test/mock_platform_notification_service.h
@@ -14,6 +14,7 @@
#include "base/macros.h"
#include "base/optional.h"
#include "base/strings/string16.h"
+#include "content/public/browser/notification_database_data.h"
#include "content/public/browser/platform_notification_service.h"
#include "url/gurl.h"
@@ -63,6 +64,9 @@
const DisplayedNotificationsCallback& callback) override;
int64_t ReadNextPersistentNotificationId(
BrowserContext* browser_context) override;
+ void RecordNotificationUkmEvent(
+ BrowserContext* browser_context,
+ const NotificationDatabaseData& data) override;
private:
// Structure to represent the information of a persistent notification.
diff --git a/services/metrics/public/cpp/ukm_recorder.h b/services/metrics/public/cpp/ukm_recorder.h
index cb467ed..7d1662a 100644
--- a/services/metrics/public/cpp/ukm_recorder.h
+++ b/services/metrics/public/cpp/ukm_recorder.h
@@ -18,6 +18,7 @@
class IOSChromePasswordManagerClient;
class MediaEngagementSession;
+class PlatformNotificationServiceImpl;
class PluginInfoHostImpl;
namespace autofill {
@@ -27,11 +28,11 @@
namespace blink {
class Document;
class NavigatorVR;
-}
+} // namespace blink
namespace cc {
class UkmManager;
-}
+} // namespace cc
namespace content {
class CrossSiteDocumentResourceHandler;
@@ -41,7 +42,7 @@
namespace download {
class DownloadUkmHelper;
-}
+} // namespace download
namespace password_manager {
class PasswordManagerMetricsRecorder;
@@ -49,11 +50,11 @@
namespace payments {
class JourneyLogger;
-}
+} // namespace payments
namespace metrics {
class UkmRecorderInterface;
-}
+} // namespace metrics
namespace media {
class MediaMetricsProvider;
@@ -63,7 +64,7 @@
namespace translate {
class TranslateRankerImpl;
-}
+} // namespace translate
namespace ukm {
@@ -73,7 +74,7 @@
namespace internal {
class SourceUrlRecorderWebContentsObserver;
class SourceUrlRecorderWebStateObserver;
-}
+} // namespace internal
// This feature controls whether UkmService should be created.
METRICS_EXPORT extern const base::Feature kUkmFeature;
@@ -104,6 +105,7 @@
friend DelegatingUkmRecorder;
friend IOSChromePasswordManagerClient;
friend MediaEngagementSession;
+ friend PlatformNotificationServiceImpl;
friend PluginInfoHostImpl;
friend TestRecordingHelper;
friend autofill::TestAutofillClient;
diff --git a/tools/metrics/ukm/ukm.xml b/tools/metrics/ukm/ukm.xml
index 530c68d8..c7d83bfd 100644
--- a/tools/metrics/ukm/ukm.xml
+++ b/tools/metrics/ukm/ukm.xml
@@ -2343,6 +2343,107 @@
</metric>
</event>
+<event name="Notification">
+ <owner>yangsharon@google.com</owner>
+ <owner>awdf@chromium.org</owner>
+ <owner>peter@chromium.org</owner>
+ <summary>
+ A notification event corresponds with the entire lifespan of a single
+ notification, and will be logged to UKM when the notification is deleted
+ from the notification database.
+ </summary>
+ <metric name="ClosedReason">
+ <summary>
+ Enum for how the notification was closed. {user, developer, unknown}
+ </summary>
+ </metric>
+ <metric name="DidReplaceAnotherNotification">
+ <summary>
+ Boolean value representing whether a tag is present and a previously shown
+ notification has been replaced. For UKM logging, we treat a replacement
+ notification as a new notification.
+ </summary>
+ </metric>
+ <metric name="DidUserOpenSettings">
+ <summary>
+ Boolean value indicating whether the user accessed the Chrome provided
+ settings from the notification.
+ </summary>
+ </metric>
+ <metric name="HasBadge">
+ <summary>
+ Boolean value representing if a badge URL was included.
+ </summary>
+ </metric>
+ <metric name="HasIcon">
+ <summary>
+ Boolean value representing if an icon URL was included.
+ </summary>
+ </metric>
+ <metric name="HasImage">
+ <summary>
+ Boolean value representing if an image URL was included.
+ </summary>
+ </metric>
+ <metric name="HasRenotify">
+ <summary>
+ Boolean value representing whether default notification indicators (sound,
+ vibration, light) should be played again if the notification is replacing
+ an older notification.
+ </summary>
+ </metric>
+ <metric name="HasTag">
+ <summary>
+ Boolean value representing if a tag is present.
+ </summary>
+ </metric>
+ <metric name="IsSilent">
+ <summary>
+ Boolean value representing whether default notification indicators
+ (sound, vibration, light) should be suppressed.
+ </summary>
+ </metric>
+ <metric name="NumActionButtonClicks">
+ <summary>
+ Number of clicks on developer provided action buttons.
+ </summary>
+ </metric>
+ <metric name="NumActions">
+ <summary>
+ Number of developer specified actions that can be taken.
+ </summary>
+ </metric>
+ <metric name="NumClicks">
+ <summary>
+ Number of clicks, excluding action buttons. I.e. clicks on the
+ notification itself.
+ </summary>
+ </metric>
+ <metric name="RequireInteraction">
+ <summary>
+ Boolean value representing whether the notification should remain onscreen
+ indefinitely, rather than being auto-minimized to the notification center
+ (if allowed by platform).
+ </summary>
+ </metric>
+ <metric name="TimeUntilClose">
+ <summary>
+ The length of time, in ms, between when the notification was triggered and
+ when it was closed.
+ </summary>
+ </metric>
+ <metric name="TimeUntilFirstClick">
+ <summary>
+ Time, in ms, between when the notification is shown and the first click.
+ </summary>
+ </metric>
+ <metric name="TimeUntilLastClick">
+ <summary>
+ Time, in ms, between when the notification is shown and the last click.
+ </summary>
+ </metric>
+</event>
+
<event name="OfflinePages.SavePageRequested">
<owner>petewil@chromium.org</owner>
<metric name="RequestedFromForeground">
