Commit f1181050 authored by Richard Knoll's avatar Richard Knoll Committed by Commit Bot

Check quota for scheduled notifications.

This makes sure that we only allow up to 30 scheduled notifications and
handles the case when we update an existing notification with a
scheduled one.

Bug: 891339
Change-Id: Ifb884367d71f9156abf303d8330364f671e3afb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1505503
Commit-Queue: Richard Knoll <knollr@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638686}
parent b77db01a
...@@ -1274,6 +1274,7 @@ jumbo_source_set("browser") { ...@@ -1274,6 +1274,7 @@ jumbo_source_set("browser") {
"notifications/notification_id_generator.h", "notifications/notification_id_generator.h",
"notifications/notification_storage.cc", "notifications/notification_storage.cc",
"notifications/notification_storage.h", "notifications/notification_storage.h",
"notifications/notification_trigger_constants.h",
"notifications/platform_notification_context_impl.cc", "notifications/platform_notification_context_impl.cc",
"notifications/platform_notification_context_impl.h", "notifications/platform_notification_context_impl.h",
"notifications/platform_notification_service_proxy.cc", "notifications/platform_notification_service_proxy.cc",
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CONTENT_BROWSER_NOTIFICATIONS_NOTIFICATION_TRIGGER_CONSTANTS_H_
#define CONTENT_BROWSER_NOTIFICATIONS_NOTIFICATION_TRIGGER_CONSTANTS_H_
namespace content {
// Maximum number of currently scheduled notifications per origin.
constexpr int kMaximumScheduledNotificationsPerOrigin = 30;
} // namespace content
#endif // CONTENT_BROWSER_NOTIFICATIONS_NOTIFICATION_TRIGGER_CONSTANTS_H_
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "content/browser/notifications/blink_notification_service_impl.h" #include "content/browser/notifications/blink_notification_service_impl.h"
#include "content/browser/notifications/notification_database.h" #include "content/browser/notifications/notification_database.h"
#include "content/browser/notifications/notification_trigger_constants.h"
#include "content/browser/notifications/platform_notification_service_proxy.h" #include "content/browser/notifications/platform_notification_service_proxy.h"
#include "content/browser/service_worker/service_worker_context_wrapper.h" #include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
...@@ -472,6 +473,28 @@ void PlatformNotificationContextImpl::WriteNotificationData( ...@@ -472,6 +473,28 @@ void PlatformNotificationContextImpl::WriteNotificationData(
database_data, std::move(callback))); database_data, std::move(callback)));
} }
bool PlatformNotificationContextImpl::DoCheckNotificationTriggerQuota(
const GURL& origin) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
int notification_count = 0;
// Iterate over all notifications and count all scheduled notifications for
// |origin|.
NotificationDatabase::Status status =
database_->ForEachNotificationData(base::BindRepeating(
[](const GURL& expected_origin, int* count,
const NotificationDatabaseData& data) {
if (CanTrigger(data) && data.origin == expected_origin)
*count = *count + 1;
},
origin, &notification_count));
// Blow away the database if reading data failed due to corruption.
if (status == NotificationDatabase::STATUS_ERROR_CORRUPTED)
DestroyDatabase();
return notification_count < kMaximumScheduledNotificationsPerOrigin;
}
void PlatformNotificationContextImpl::DoWriteNotificationData( void PlatformNotificationContextImpl::DoWriteNotificationData(
int64_t service_worker_registration_id, int64_t service_worker_registration_id,
int64_t persistent_notification_id, int64_t persistent_notification_id,
...@@ -489,6 +512,7 @@ void PlatformNotificationContextImpl::DoWriteNotificationData( ...@@ -489,6 +512,7 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
return; return;
} }
bool replaces_existing = false;
std::string notification_id = std::string notification_id =
notification_id_generator_.GenerateForPersistentNotification( notification_id_generator_.GenerateForPersistentNotification(
origin, database_data.notification_data.tag, origin, database_data.notification_data.tag,
...@@ -502,6 +526,8 @@ void PlatformNotificationContextImpl::DoWriteNotificationData( ...@@ -502,6 +526,8 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
origin, database_data.notification_data.tag, origin, database_data.notification_data.tag,
&deleted_notification_ids); &deleted_notification_ids);
replaces_existing = deleted_notification_ids.count(notification_id) != 0;
UMA_HISTOGRAM_ENUMERATION("Notifications.Database.DeleteBeforeWriteResult", UMA_HISTOGRAM_ENUMERATION("Notifications.Database.DeleteBeforeWriteResult",
delete_status, delete_status,
NotificationDatabase::STATUS_COUNT); NotificationDatabase::STATUS_COUNT);
...@@ -525,6 +551,16 @@ void PlatformNotificationContextImpl::DoWriteNotificationData( ...@@ -525,6 +551,16 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
write_database_data.notification_id = notification_id; write_database_data.notification_id = notification_id;
write_database_data.origin = origin; write_database_data.origin = origin;
if (CanTrigger(write_database_data) &&
!DoCheckNotificationTriggerQuota(origin)) {
// TODO(knollr): Reply with a custom error so developers can handle this.
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::UI, base::TaskPriority::USER_VISIBLE},
base::BindOnce(std::move(callback), /* success= */ false,
/* notification_id= */ ""));
return;
}
NotificationDatabase::Status status = NotificationDatabase::Status status =
database_->WriteNotificationData(origin, write_database_data); database_->WriteNotificationData(origin, write_database_data);
...@@ -533,6 +569,8 @@ void PlatformNotificationContextImpl::DoWriteNotificationData( ...@@ -533,6 +569,8 @@ void PlatformNotificationContextImpl::DoWriteNotificationData(
if (status == NotificationDatabase::STATUS_OK) { if (status == NotificationDatabase::STATUS_OK) {
if (CanTrigger(write_database_data)) { if (CanTrigger(write_database_data)) {
if (replaces_existing)
service_proxy_->CloseNotification(notification_id);
// Schedule notification to be shown. // Schedule notification to be shown.
base::PostTaskWithTraits( base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::UI, base::TaskPriority::USER_VISIBLE}, FROM_HERE, {BrowserThread::UI, base::TaskPriority::USER_VISIBLE},
......
...@@ -186,6 +186,10 @@ class CONTENT_EXPORT PlatformNotificationContextImpl ...@@ -186,6 +186,10 @@ class CONTENT_EXPORT PlatformNotificationContextImpl
bool supports_synchronization, bool supports_synchronization,
bool initialized); bool initialized);
// Checks if the number of notifications scheduled for |origin| does not
// exceed the quota.
bool DoCheckNotificationTriggerQuota(const GURL& origin);
// Actually writes the notification database to the database. Must only be // Actually writes the notification database to the database. Must only be
// called on the |task_runner_| thread. |callback| will be invoked on the // called on the |task_runner_| thread. |callback| will be invoked on the
// UI thread when the operation has completed. // UI thread when the operation has completed.
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "content/browser/notifications/notification_trigger_constants.h"
#include "content/browser/notifications/platform_notification_context_impl.h" #include "content/browser/notifications/platform_notification_context_impl.h"
#include "content/browser/service_worker/service_worker_context_wrapper.h" #include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/public/browser/notification_database_data.h" #include "content/public/browser/notification_database_data.h"
...@@ -82,14 +83,15 @@ class PlatformNotificationContextTriggerTest : public ::testing::Test { ...@@ -82,14 +83,15 @@ class PlatformNotificationContextTriggerTest : public ::testing::Test {
} }
protected: protected:
void WriteNotificationData(const std::string& tag, Time timestamp) { void WriteNotificationData(const std::string& tag,
base::Optional<base::Time> timestamp) {
ASSERT_TRUE( ASSERT_TRUE(
TryWriteNotificationData("https://example.com", tag, timestamp)); TryWriteNotificationData("https://example.com", tag, timestamp));
} }
bool TryWriteNotificationData(const std::string& url, bool TryWriteNotificationData(const std::string& url,
const std::string& tag, const std::string& tag,
Time timestamp) { base::Optional<base::Time> timestamp) {
GURL origin(url); GURL origin(url);
NotificationDatabaseData notification_database_data; NotificationDatabaseData notification_database_data;
notification_database_data.origin = origin; notification_database_data.origin = origin;
...@@ -194,4 +196,57 @@ TEST_F(PlatformNotificationContextTriggerTest, ...@@ -194,4 +196,57 @@ TEST_F(PlatformNotificationContextTriggerTest,
ASSERT_EQ(1u, GetDisplayedNotifications().size()); ASSERT_EQ(1u, GetDisplayedNotifications().size());
} }
TEST_F(PlatformNotificationContextTriggerTest,
OverwriteDisplayedNotificationToFuture) {
WriteNotificationData("1", Time::Now() + TimeDelta::FromSeconds(10));
thread_bundle_.FastForwardBy(TimeDelta::FromSeconds(10));
// Overwrites a displayed notification which hides it until the trigger
// timestamp is reached.
WriteNotificationData("1", Time::Now() + TimeDelta::FromSeconds(10));
ASSERT_EQ(0u, GetDisplayedNotifications().size());
thread_bundle_.FastForwardBy(TimeDelta::FromSeconds(10));
ASSERT_EQ(1u, GetDisplayedNotifications().size());
}
TEST_F(PlatformNotificationContextTriggerTest,
LimitsNumberOfScheduledNotificationsPerOrigin) {
for (int i = 1; i <= kMaximumScheduledNotificationsPerOrigin; ++i) {
WriteNotificationData(std::to_string(i),
Time::Now() + TimeDelta::FromSeconds(i));
}
ASSERT_FALSE(TryWriteNotificationData(
"https://example.com",
std::to_string(kMaximumScheduledNotificationsPerOrigin + 1),
Time::Now() +
TimeDelta::FromSeconds(kMaximumScheduledNotificationsPerOrigin + 1)));
ASSERT_TRUE(TryWriteNotificationData(
"https://example2.com",
std::to_string(kMaximumScheduledNotificationsPerOrigin + 1),
Time::Now() +
TimeDelta::FromSeconds(kMaximumScheduledNotificationsPerOrigin + 1)));
}
TEST_F(PlatformNotificationContextTriggerTest, EnforcesLimitOnUpdate) {
for (int i = 1; i <= kMaximumScheduledNotificationsPerOrigin; ++i) {
WriteNotificationData(std::to_string(i),
Time::Now() + TimeDelta::FromSeconds(i));
}
ASSERT_TRUE(TryWriteNotificationData(
"https://example.com",
std::to_string(kMaximumScheduledNotificationsPerOrigin + 1),
base::nullopt));
ASSERT_FALSE(TryWriteNotificationData(
"https://example.com",
std::to_string(kMaximumScheduledNotificationsPerOrigin + 1),
Time::Now() +
TimeDelta::FromSeconds(kMaximumScheduledNotificationsPerOrigin + 1)));
}
} // namespace content } // namespace content
...@@ -85,4 +85,16 @@ void PlatformNotificationServiceProxy::DisplayNotification( ...@@ -85,4 +85,16 @@ void PlatformNotificationServiceProxy::DisplayNotification(
weak_ptr_factory_.GetWeakPtr(), data, std::move(callback)))); weak_ptr_factory_.GetWeakPtr(), data, std::move(callback))));
} }
void PlatformNotificationServiceProxy::CloseNotification(
const std::string& notification_id) {
if (!notification_service_)
return;
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::UI, base::TaskPriority::USER_VISIBLE},
base::BindOnce(&PlatformNotificationService::ClosePersistentNotification,
base::Unretained(notification_service_), browser_context_,
notification_id));
}
} // namespace content } // namespace content
...@@ -45,6 +45,9 @@ class CONTENT_EXPORT PlatformNotificationServiceProxy { ...@@ -45,6 +45,9 @@ class CONTENT_EXPORT PlatformNotificationServiceProxy {
void DisplayNotification(const NotificationDatabaseData& data, void DisplayNotification(const NotificationDatabaseData& data,
DisplayResultCallback callback); DisplayResultCallback callback);
// Closes the notification with |notification_id|.
void CloseNotification(const std::string& notification_id);
private: private:
// Actually calls |notification_service_| to display the notification after // Actually calls |notification_service_| to display the notification after
// verifying the |service_worker_scope|. // verifying the |service_worker_scope|.
......
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