Address a race condition in displaying notifications

content/browser/notifications/platform_notification_context_impl.cc

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 8a15289d8aac51406bc6db6714058e99d5dc3317..757612288da826b48f0c381036359cd7884dc2be 100644
--- a/content/browser/notifications/platform_notification_context_impl.cc
+++ b/content/browser/notifications/platform_notification_context_impl.cc
@@ -10,6 +10,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
 #include "base/threading/sequenced_worker_pool.h"
+#include "base/time/default_clock.h"
 #include "content/browser/notifications/blink_notification_service_impl.h"
 #include "content/browser/notifications/notification_database.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
@@ -24,6 +25,10 @@ using base::DoNothing;
 namespace content {
 namespace {
 
+// Grace period, in seconds, during which we avoid deleting recently shown
+// notifications from the database.
+constexpr double kNotificationDisplayGraceSeconds = 5.0;
+
 // Name of the directory in the user's profile directory where the notification
 // database files should be stored.
 const base::FilePath::CharType kPlatformNotificationsDirectory[] =
@@ -38,7 +43,8 @@ PlatformNotificationContextImpl::PlatformNotificationContextImpl(
     : path_(path),
       browser_context_(browser_context),
       service_worker_context_(service_worker_context),
-      notification_id_generator_(browser_context) {
+      notification_id_generator_(browser_context),
+      clock_(base::MakeUnique<base::DefaultClock>()) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
@@ -272,6 +278,19 @@ void PlatformNotificationContextImpl::
   DCHECK(task_runner_->RunsTasksOnCurrentThread());
   DCHECK(displayed_notifications);
 
+  base::Time current_time = clock_->Now();
+
+  // Prune the list of recently shown notifications to those that were displayed
+  // in the past |kNotificationDisplayGraceSeconds| seconds.
+  auto iter = recent_notifications_.begin();
+  while (iter != recent_notifications_.end()) {
+    base::TimeDelta display_time = current_time - iter->second;
+    if (display_time.InSecondsF() > kNotificationDisplayGraceSeconds)
+      iter = recent_notifications_.erase(iter);
+    else
+      iter++;
+  }
+
   std::vector<NotificationDatabaseData> notification_datas;
 
   NotificationDatabase::Status status =
@@ -290,7 +309,12 @@ void PlatformNotificationContextImpl::
         // The database is only used for persistent notifications.
         DCHECK(NotificationIdGenerator::IsPersistentNotification(
             it->notification_id));
-        if (displayed_notifications->count(it->notification_id)) {
+
+        const bool notification_exists =
+            displayed_notifications->count(it->notification_id) ||
+            recent_notifications_.count(it->notification_id);
+
+        if (notification_exists) {
           ++it;
         } else {
           obsolete_notifications.push_back(it->notification_id);
@@ -375,6 +399,8 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
                             NotificationDatabase::STATUS_COUNT);
 
   if (status == NotificationDatabase::STATUS_OK) {
+    recent_notifications_[write_database_data.notification_id] = clock_->Now();
+
     BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                             base::Bind(callback, true /* success */,
                                        write_database_data.notification_id));
@@ -415,6 +441,8 @@ void PlatformNotificationContextImpl::DoDeleteNotificationData(
   UMA_HISTOGRAM_ENUMERATION("Notifications.Database.DeleteResult", status,
                             NotificationDatabase::STATUS_COUNT);
 
+  recent_notifications_.erase(notification_id);
+
   bool success = status == NotificationDatabase::STATUS_OK;
 
   // Blow away the database if deleting data failed due to corruption. Following