content/browser/notifications/notification_database_unittest.cc - Issue 2300093002: Make //content responsible for generating notification Ids

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

Issue 2300093002: Make //content responsible for generating notification Ids (Closed)

Patch Set: Make //content responsible for generating notification Ids 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

« content/browser/notifications/notification_database_data.proto ('K') | « content/browser/notifications/notification_database_data_unittest.cc ('k') | content/browser/notifications/notification_event_dispatcher_impl.h » ('j') | content/browser/notifications/platform_notification_context_impl.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | 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..421efdaa25801850e09d992deda2d9a360f9eaf1 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 = base::GenerateGUID();

johnme 2016/09/08 15:21:50 Why is there a GUID here? If this is just to gener

Peter Beverloo 2016/09/08 18:56:02 Done.

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),

@@ -180,21 +185,16 @@ TEST_F(NotificationDatabaseTest, NotificationIdIncrements) {

ASSERT_EQ(NotificationDatabase::STATUS_OK,

database->Open(true /* create_if_missing */));

- GURL origin("https://example.com");

- int64_t notification_id = 0;

+ EXPECT_EQ(database->GetNextNotificationId(), 1);

+ EXPECT_EQ(database->GetNextNotificationId(), 2);

+ EXPECT_EQ(database->GetNextNotificationId(), 3);

- // 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,

@@ -202,10 +202,7 @@ TEST_F(NotificationDatabaseTest, NotificationIdIncrements) {

// 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);

+ EXPECT_EQ(database->GetNextNotificationId(), 4);

}

TEST_F(NotificationDatabaseTest, NotificationIdIncrementsStorage) {

@@ -215,19 +212,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 = base::GenerateGUID();

- 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 +238,10 @@ TEST_F(NotificationDatabaseTest, NotificationIdCorruption) {

GURL origin("https://example.com");

NotificationDatabaseData database_data;

- int64_t notification_id = 0;

+ database_data.notification_id = base::GenerateGUID();

- 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 +264,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("my-id", GURL("https://chrome.com"),

johnme 2016/09/08 15:21:50 Nit: s/my-id/bad-id/ ?

Peter Beverloo 2016/09/08 18:56:02 Done.

&database_data));

}

@@ -280,28 +273,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 = base::GenerateGUID();

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 +304,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 +317,25 @@ TEST_F(NotificationDatabaseTest, ReadNotificationDataReflection) {

notification_data.silent = true;

NotificationDatabaseData database_data;

- database_data.notification_id = notification_id;

+ database_data.notification_id = base::GenerateGUID();

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 +359,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 +369,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 +397,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("9001", GURL("https://chrome.com")));

johnme 2016/09/08 15:21:50 Nit: s/9001/bad-id/

Peter Beverloo 2016/09/08 18:56:02 Done.

}

TEST_F(NotificationDatabaseTest, DeleteNotificationDataSameOrigin) {

@@ -409,14 +407,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 = base::GenerateGUID();

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 +436,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 = base::GenerateGUID();

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 +528,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 +570,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 +605,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 +622,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,