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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 2300093002: Make //content responsible for generating notification Ids (Closed)
Patch Set: comments Created 4 years, 3 months 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
« no previous file with comments | « content/browser/notifications/notification_database_data_unittest.cc ('k') | content/browser/notifications/notification_event_dispatcher_impl.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/notifications/notification_database_unittest.cc
diff --git a/content/browser/notifications/notification_database_unittest.cc b/content/browser/notifications/notification_database_unittest.cc
index f02e0679c9e62c4695d7581b64bef6f1c1125ede..52d94c85700f8611238fef4bdfe8d05947fd1cb0 100644
--- a/content/browser/notifications/notification_database_unittest.cc
+++ b/content/browser/notifications/notification_database_unittest.cc
@@ -8,6 +8,7 @@
#include <stdint.h>
#include "base/files/scoped_temp_dir.h"
+#include "base/guid.h"
#include "base/macros.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
@@ -56,22 +57,26 @@ class NotificationDatabaseTest : public ::testing::Test {
const GURL& origin,
const std::string& tag,
int64_t service_worker_registration_id,
- int64_t* notification_id) {
+ std::string* notification_id) {
+ DCHECK(notification_id);
+
NotificationDatabaseData database_data;
+ database_data.notification_id = GenerateNotificationId();
database_data.origin = origin;
database_data.service_worker_registration_id =
service_worker_registration_id;
database_data.notification_data.tag = tag;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
- database->WriteNotificationData(origin, database_data,
- notification_id));
+ database->WriteNotificationData(origin, database_data));
+
+ *notification_id = database_data.notification_id;
}
// Populates |database| with a series of example notifications that differ in
// their origin and Service Worker registration id.
void PopulateDatabaseWithExampleData(NotificationDatabase* database) {
- int64_t notification_id;
+ std::string notification_id;
for (size_t i = 0; i < arraysize(kExampleNotificationData); ++i) {
ASSERT_NO_FATAL_FAILURE(CreateAndWriteNotification(
database, GURL(kExampleNotificationData[i].origin),
@@ -100,6 +105,9 @@ class NotificationDatabaseTest : public ::testing::Test {
database->GetDBForTesting()->Put(leveldb::WriteOptions(), key, value);
ASSERT_TRUE(status.ok());
}
+
+ // Generates a random notification ID. The format of the ID is opaque.
+ std::string GenerateNotificationId() { return base::GenerateGUID(); }
};
TEST_F(NotificationDatabaseTest, OpenCloseMemory) {
@@ -180,32 +188,24 @@ TEST_F(NotificationDatabaseTest, NotificationIdIncrements) {
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->Open(true /* create_if_missing */));
- GURL origin("https://example.com");
+ EXPECT_EQ(database->GetNextPersistentNotificationId(), 1);
+ EXPECT_EQ(database->GetNextPersistentNotificationId(), 2);
+ EXPECT_EQ(database->GetNextPersistentNotificationId(), 3);
- int64_t notification_id = 0;
-
- // Verify that getting two ids on the same database instance results in
- // incrementing values. Notification ids will start at 1.
- ASSERT_NO_FATAL_FAILURE(
- CreateAndWriteNotification(database.get(), origin, "" /* tag */,
- 0 /* sw_registration_id */, &notification_id));
- EXPECT_EQ(notification_id, 1);
+ GURL origin("https://example.com");
+ std::string notification_id;
ASSERT_NO_FATAL_FAILURE(
CreateAndWriteNotification(database.get(), origin, "" /* tag */,
0 /* sw_registration_id */, &notification_id));
- EXPECT_EQ(notification_id, 2);
database.reset(CreateDatabaseOnFileSystem(database_dir.path()));
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->Open(false /* create_if_missing */));
- // Verify that the next notification id was stored in the database, and
- // continues where we expect it to be, even after closing and opening it.
- ASSERT_NO_FATAL_FAILURE(
- CreateAndWriteNotification(database.get(), origin, "" /* tag */,
- 0 /* sw_registration_id */, &notification_id));
- EXPECT_EQ(notification_id, 3);
+ // Verify that the next persistent notification id was stored in the database,
+ // and continues where we expect it to be, even after closing and opening it.
+ EXPECT_EQ(database->GetNextPersistentNotificationId(), 4);
}
TEST_F(NotificationDatabaseTest, NotificationIdIncrementsStorage) {
@@ -215,19 +215,17 @@ TEST_F(NotificationDatabaseTest, NotificationIdIncrementsStorage) {
GURL origin("https://example.com");
- NotificationDatabaseData database_data;
- database_data.notification_id = -1;
+ NotificationDatabaseData database_data, read_database_data;
+ database_data.notification_id = GenerateNotificationId();
- int64_t notification_id = 0;
- ASSERT_EQ(
- NotificationDatabase::STATUS_OK,
- database->WriteNotificationData(origin, database_data, &notification_id));
+ ASSERT_EQ(NotificationDatabase::STATUS_OK,
+ database->WriteNotificationData(origin, database_data));
- ASSERT_EQ(
- NotificationDatabase::STATUS_OK,
- database->ReadNotificationData(notification_id, origin, &database_data));
+ ASSERT_EQ(NotificationDatabase::STATUS_OK,
+ database->ReadNotificationData(database_data.notification_id,
+ origin, &read_database_data));
- EXPECT_EQ(notification_id, database_data.notification_id);
+ EXPECT_EQ(database_data.notification_id, read_database_data.notification_id);
}
TEST_F(NotificationDatabaseTest, NotificationIdCorruption) {
@@ -243,12 +241,10 @@ TEST_F(NotificationDatabaseTest, NotificationIdCorruption) {
GURL origin("https://example.com");
NotificationDatabaseData database_data;
- int64_t notification_id = 0;
+ database_data.notification_id = GenerateNotificationId();
- ASSERT_EQ(
- NotificationDatabase::STATUS_OK,
- database->WriteNotificationData(origin, database_data, &notification_id));
- EXPECT_EQ(notification_id, 1);
+ ASSERT_EQ(NotificationDatabase::STATUS_OK,
+ database->WriteNotificationData(origin, database_data));
// Deliberately write an invalid value as the next notification id. When
// re-opening the database, the Open() method should realize that an invalid
@@ -271,7 +267,7 @@ TEST_F(NotificationDatabaseTest, ReadInvalidNotificationData) {
// Reading the notification data for a notification that does not exist should
// return the ERROR_NOT_FOUND status code.
EXPECT_EQ(NotificationDatabase::STATUS_ERROR_NOT_FOUND,
- database->ReadNotificationData(9001, GURL("https://chrome.com"),
+ database->ReadNotificationData("bad-id", GURL("https://chrome.com"),
&database_data));
}
@@ -280,28 +276,27 @@ TEST_F(NotificationDatabaseTest, ReadNotificationDataDifferentOrigin) {
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->Open(true /* create_if_missing */));
- int64_t notification_id = 0;
GURL origin("https://example.com");
NotificationDatabaseData database_data, read_database_data;
+ database_data.notification_id = GenerateNotificationId();
database_data.notification_data.title = base::UTF8ToUTF16("My Notification");
- ASSERT_EQ(
- NotificationDatabase::STATUS_OK,
- database->WriteNotificationData(origin, database_data, &notification_id));
+ ASSERT_EQ(NotificationDatabase::STATUS_OK,
+ database->WriteNotificationData(origin, database_data));
// Reading the notification from the database when given a different origin
// should return the ERROR_NOT_FOUND status code.
- EXPECT_EQ(
- NotificationDatabase::STATUS_ERROR_NOT_FOUND,
- database->ReadNotificationData(
- notification_id, GURL("https://chrome.com"), &read_database_data));
+ EXPECT_EQ(NotificationDatabase::STATUS_ERROR_NOT_FOUND,
+ database->ReadNotificationData(database_data.notification_id,
+ GURL("https://chrome.com"),
+ &read_database_data));
// However, reading the notification from the database with the same origin
// should return STATUS_OK and the associated notification data.
ASSERT_EQ(NotificationDatabase::STATUS_OK,
- database->ReadNotificationData(notification_id, origin,
- &read_database_data));
+ database->ReadNotificationData(database_data.notification_id,
+ origin, &read_database_data));
EXPECT_EQ(database_data.notification_data.title,
read_database_data.notification_data.title);
@@ -312,8 +307,6 @@ TEST_F(NotificationDatabaseTest, ReadNotificationDataReflection) {
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->Open(true /* create_if_missing */));
- int64_t notification_id = 0;
-
GURL origin("https://example.com");
PlatformNotificationData notification_data;
@@ -327,26 +320,25 @@ TEST_F(NotificationDatabaseTest, ReadNotificationDataReflection) {
notification_data.silent = true;
NotificationDatabaseData database_data;
- database_data.notification_id = notification_id;
+ database_data.notification_id = GenerateNotificationId();
database_data.origin = origin;
database_data.service_worker_registration_id = 42;
database_data.notification_data = notification_data;
// Write the constructed notification to the database, and then immediately
// read it back from the database again as well.
- ASSERT_EQ(
- NotificationDatabase::STATUS_OK,
- database->WriteNotificationData(origin, database_data, &notification_id));
+ ASSERT_EQ(NotificationDatabase::STATUS_OK,
+ database->WriteNotificationData(origin, database_data));
NotificationDatabaseData read_database_data;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
- database->ReadNotificationData(notification_id, origin,
- &read_database_data));
+ database->ReadNotificationData(database_data.notification_id,
+ origin, &read_database_data));
// Verify that all members retrieved from the database are exactly the same
// as the ones that were written to it. This tests the serialization behavior.
- EXPECT_EQ(notification_id, read_database_data.notification_id);
+ EXPECT_EQ(database_data.notification_id, read_database_data.notification_id);
EXPECT_EQ(database_data.origin, read_database_data.origin);
EXPECT_EQ(database_data.service_worker_registration_id,
@@ -370,7 +362,9 @@ TEST_F(NotificationDatabaseTest, ReadWriteMultipleNotificationData) {
database->Open(true /* create_if_missing */));
GURL origin("https://example.com");
- int64_t notification_id = 0;
+
+ std::vector<std::string> notification_ids;
+ std::string notification_id;
// Write ten notifications to the database, each with a unique title and
// notification id (it is the responsibility of the user to increment this).
@@ -378,19 +372,25 @@ TEST_F(NotificationDatabaseTest, ReadWriteMultipleNotificationData) {
ASSERT_NO_FATAL_FAILURE(CreateAndWriteNotification(
database.get(), origin, "" /* tag */, i /* sw_registration_id */,
&notification_id));
- EXPECT_EQ(notification_id, i);
+
+ EXPECT_FALSE(notification_id.empty());
+
+ notification_ids.push_back(notification_id);
}
NotificationDatabaseData database_data;
+ int64_t service_worker_registration_id = 1;
+
// Read the ten notifications from the database, and verify that the titles
// of each of them matches with how they were created.
- for (int i = 1; i <= 10; ++i) {
+ for (const std::string& notification_id : notification_ids) {
ASSERT_EQ(NotificationDatabase::STATUS_OK,
- database->ReadNotificationData(i /* notification_id */, origin,
+ database->ReadNotificationData(notification_id, origin,
&database_data));
- EXPECT_EQ(i, database_data.service_worker_registration_id);
+ EXPECT_EQ(service_worker_registration_id++,
+ database_data.service_worker_registration_id);
}
}
@@ -400,8 +400,9 @@ TEST_F(NotificationDatabaseTest, DeleteInvalidNotificationData) {
database->Open(true /* create_if_missing */));
// Deleting non-existing notifications is not considered to be a failure.
- ASSERT_EQ(NotificationDatabase::STATUS_OK,
- database->DeleteNotificationData(9001, GURL("https://chrome.com")));
+ ASSERT_EQ(
+ NotificationDatabase::STATUS_OK,
+ database->DeleteNotificationData("bad-id", GURL("https://chrome.com")));
}
TEST_F(NotificationDatabaseTest, DeleteNotificationDataSameOrigin) {
@@ -409,14 +410,15 @@ TEST_F(NotificationDatabaseTest, DeleteNotificationDataSameOrigin) {
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->Open(true /* create_if_missing */));
- int64_t notification_id = 0;
+ const std::string notification_id = GenerateNotificationId();
NotificationDatabaseData database_data;
+ database_data.notification_id = notification_id;
+
GURL origin("https://example.com");
- ASSERT_EQ(
- NotificationDatabase::STATUS_OK,
- database->WriteNotificationData(origin, database_data, &notification_id));
+ ASSERT_EQ(NotificationDatabase::STATUS_OK,
+ database->WriteNotificationData(origin, database_data));
// Reading a notification after writing one should succeed.
EXPECT_EQ(
@@ -437,14 +439,15 @@ TEST_F(NotificationDatabaseTest, DeleteNotificationDataDifferentOrigin) {
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->Open(true /* create_if_missing */));
- int64_t notification_id = 0;
+ const std::string notification_id = GenerateNotificationId();
NotificationDatabaseData database_data;
+ database_data.notification_id = notification_id;
+
GURL origin("https://example.com");
- ASSERT_EQ(
- NotificationDatabase::STATUS_OK,
- database->WriteNotificationData(origin, database_data, &notification_id));
+ ASSERT_EQ(NotificationDatabase::STATUS_OK,
+ database->WriteNotificationData(origin, database_data));
// Attempting to delete the notification with a different origin, but with the
// same |notification_id|, should not return an error (the notification could
@@ -528,12 +531,12 @@ TEST_F(NotificationDatabaseTest, DeleteAllNotificationDataForOrigin) {
GURL origin("https://example.com:443");
- std::set<int64_t> deleted_notification_set;
+ std::set<std::string> deleted_notification_ids;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->DeleteAllNotificationDataForOrigin(
- origin, "" /* tag */, &deleted_notification_set));
+ origin, "" /* tag */, &deleted_notification_ids));
- EXPECT_EQ(4u, deleted_notification_set.size());
+ EXPECT_EQ(4u, deleted_notification_ids.size());
std::vector<NotificationDatabaseData> notifications;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
@@ -570,12 +573,12 @@ TEST_F(NotificationDatabaseTest, DeleteAllNotificationDataForOriginWithTag) {
ASSERT_GT(notifications_with_tag, 0u);
ASSERT_GT(notifications_without_tag, 0u);
- std::set<int64_t> deleted_notification_set;
+ std::set<std::string> deleted_notification_ids;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->DeleteAllNotificationDataForOrigin(
- origin, "foo" /* tag */, &deleted_notification_set));
+ origin, "foo" /* tag */, &deleted_notification_ids));
- EXPECT_EQ(notifications_with_tag, deleted_notification_set.size());
+ EXPECT_EQ(notifications_with_tag, deleted_notification_ids.size());
std::vector<NotificationDatabaseData> updated_notifications;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
@@ -605,12 +608,12 @@ TEST_F(NotificationDatabaseTest, DeleteAllNotificationDataForOriginEmpty) {
GURL origin("https://example.com");
- std::set<int64_t> deleted_notification_set;
+ std::set<std::string> deleted_notification_ids;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->DeleteAllNotificationDataForOrigin(
- origin, "" /* tag */, &deleted_notification_set));
+ origin, "" /* tag */, &deleted_notification_ids));
- EXPECT_EQ(0u, deleted_notification_set.size());
+ EXPECT_EQ(0u, deleted_notification_ids.size());
}
TEST_F(NotificationDatabaseTest,
@@ -622,13 +625,13 @@ TEST_F(NotificationDatabaseTest,
ASSERT_NO_FATAL_FAILURE(PopulateDatabaseWithExampleData(database.get()));
GURL origin("https://example.com:443");
- std::set<int64_t> deleted_notification_set;
+ std::set<std::string> deleted_notification_ids;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
database->DeleteAllNotificationDataForServiceWorkerRegistration(
origin, kExampleServiceWorkerRegistrationId,
- &deleted_notification_set));
+ &deleted_notification_ids));
- EXPECT_EQ(2u, deleted_notification_set.size());
+ EXPECT_EQ(2u, deleted_notification_ids.size());
std::vector<NotificationDatabaseData> notifications;
ASSERT_EQ(NotificationDatabase::STATUS_OK,
« no previous file with comments | « content/browser/notifications/notification_database_data_unittest.cc ('k') | content/browser/notifications/notification_event_dispatcher_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698