Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(487)

Unified Diff: content/browser/notifications/platform_notification_context_impl.cc

Issue 2534443002: Use notification display service to collect persistent notifications. (Closed)
Patch Set: threading Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/notifications/platform_notification_context_impl.cc
diff --git a/content/browser/notifications/platform_notification_context_impl.cc b/content/browser/notifications/platform_notification_context_impl.cc
index fa5c1387e1ccacdb81ebfc43ce707dca5e069afe..dbc39b5277f00b6cdeca2c1d096e9c312068a3b3 100644
--- a/content/browser/notifications/platform_notification_context_impl.cc
+++ b/content/browser/notifications/platform_notification_context_impl.cc
@@ -60,8 +60,8 @@ void PlatformNotificationContextImpl::Initialize() {
std::set<std::string> displayed_notifications;
bool notification_synchronization_supported =
- service->GetDisplayedPersistentNotifications(browser_context_,
- &displayed_notifications);
+ service->GetDisplayedNotifications(browser_context_,
+ &displayed_notifications);
// Synchronize the notifications stored in the database with the set of
// displaying notifications in |displayed_notifications|. This is necessary
@@ -182,25 +182,65 @@ void PlatformNotificationContextImpl::DoReadNotificationData(
}
void PlatformNotificationContextImpl::
- ReadAllNotificationDataForServiceWorkerRegistration(
+ SynchronizeDisplayedNotificationsForServiceWorkerRegistration(
const GURL& origin,
int64_t service_worker_registration_id,
- const ReadAllResultCallback& callback) {
+ const ReadAllResultCallback& callback,
+ std::set<std::string>* notification_ids,
+ bool sync_supported) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
LazyInitialize(
base::Bind(&PlatformNotificationContextImpl::
DoReadAllNotificationDataForServiceWorkerRegistration,
- this, origin, service_worker_registration_id, callback),
+ this, origin, service_worker_registration_id, callback,
+ base::Unretained(notification_ids), sync_supported),
base::Bind(callback, false /* success */,
std::vector<NotificationDatabaseData>()));
}
void PlatformNotificationContextImpl::
- DoReadAllNotificationDataForServiceWorkerRegistration(
+ ReadAllNotificationDataForServiceWorkerRegistration(
const GURL& origin,
int64_t service_worker_registration_id,
const ReadAllResultCallback& callback) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+ // notification_ids becomes owned by
+ // DoReadAllNotificationDataForServiceWorkerRegistration
+ std::set<std::string>* notification_ids = new std::set<std::string>();
+ PlatformNotificationService* service =
+ GetContentClient()->browser()->GetPlatformNotificationService();
+
+ if (!service) {
+ // Rely on the database only
+ SynchronizeDisplayedNotificationsForServiceWorkerRegistration(
+ origin, service_worker_registration_id, callback, notification_ids,
+ false /* sync_supported */);
+ return;
+ }
+
+ BrowserThread::PostTaskAndReplyWithResult(
+ BrowserThread::UI, FROM_HERE,
+ base::Bind(&PlatformNotificationService::GetDisplayedNotifications,
+ base::Unretained(service), browser_context_,
+ base::Unretained(notification_ids)),
+ base::Bind(
+ &PlatformNotificationContextImpl::
+ SynchronizeDisplayedNotificationsForServiceWorkerRegistration,
+ this, origin, service_worker_registration_id, callback,
+ base::Unretained(notification_ids)));
+}
+
+void PlatformNotificationContextImpl::
+ DoReadAllNotificationDataForServiceWorkerRegistration(
+ const GURL& origin,
+ int64_t service_worker_registration_id,
+ const ReadAllResultCallback& callback,
+ std::set<std::string>* displayed_notifications,
+ bool synchronization_supported) {
DCHECK(task_runner_->RunsTasksOnCurrentThread());
+ std::unique_ptr<std::set<std::string>> displayed_ids =
+ base::WrapUnique(displayed_notifications);
std::vector<NotificationDatabaseData> notification_datas;
@@ -212,6 +252,22 @@ void PlatformNotificationContextImpl::
status, NotificationDatabase::STATUS_COUNT);
if (status == NotificationDatabase::STATUS_OK) {
+ if (synchronization_supported) {
+ DCHECK(displayed_ids);
+ // Filter out notifications that are not actually on display anymore.
+ // TODO(miguelg) synchronize the database if there are inconsistencies.
+ for (auto it = notification_datas.begin();
+ it != notification_datas.end();) {
+ if (NotificationIdGenerator::IsPersistentNotification(
+ it->notification_id) &&
+ displayed_ids->count(it->notification_id)) {
Peter Beverloo 2016/12/13 11:41:12 Mm. std::set<> makes this O(n log n). Could be O(n
Miguel Garcia 2016/12/13 16:43:10 Yeah, and it makes the test more predictable (sinc
+ ++it;
+ } else {
+ it = notification_datas.erase(it);
Peter Beverloo 2016/12/13 11:41:12 nit: drop this. Maybe DCHECK on NotificationIdGene
Miguel Garcia 2016/12/13 16:43:10 Done.
+ }
+ }
+ }
+
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::Bind(callback, true /* success */, notification_datas));

Powered by Google App Engine
This is Rietveld 408576698