[Notifications] Write data to database when displaying via mojo

- Display() Part 2: Write notification data to database; use a real
notification ID.

- Note this is still behind the NotificationsWithMojo flag while
development continues.

Bug: 796991
Change-Id: I61d146b8f675c5e82fc7b3ea5e7782a3229f9a7c
Reviewed-on: https://chromium-review.googlesource.com/925461
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539171}

content/browser/notifications/blink_notification_service_impl.cc

@@ -4,12 +4,15 @@
 
 #include "content/browser/notifications/blink_notification_service_impl.h"
 
+#include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "base/strings/string16.h"
 #include "content/browser/notifications/notification_event_dispatcher_impl.h"
 #include "content/browser/notifications/platform_notification_context_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/notification_database_data.h"
 #include "content/public/browser/platform_notification_service.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/notification_resources.h"
@@ -162,9 +165,34 @@ void BlinkNotificationServiceImpl::DisplayPersistentNotification(
     return;
   }
 
-  // TODO(https://crbug.com/796991): Write notification data to the database,
-  // and get back a real notification ID to use here.
-  std::string notification_id = "FIXME";
+  // TODO(awdf): Necessary to validate resources here?
+
+  NotificationDatabaseData database_data;
+  database_data.origin = origin_.GetURL();
+  database_data.service_worker_registration_id = service_worker_registration_id;
+  database_data.notification_data = platform_notification_data;
+
+  notification_context_->WriteNotificationData(
+      origin_.GetURL(), database_data,
+      base::AdaptCallbackForRepeating(base::BindOnce(
+          &BlinkNotificationServiceImpl::DisplayPersistentNotificationWithId,
+          weak_ptr_factory_.GetWeakPtr(), service_worker_registration_id,
+          platform_notification_data, notification_resources,
+          std::move(callback))));
+}
+
+void BlinkNotificationServiceImpl::DisplayPersistentNotificationWithId(
+    int64_t service_worker_registration_id,
+    const PlatformNotificationData& platform_notification_data,
+    const NotificationResources& notification_resources,
+    DisplayPersistentNotificationCallback callback,
+    bool success,
+    const std::string& notification_id) {
+  if (!success) {
+    std::move(callback).Run(
+        blink::mojom::PersistentNotificationError::INTERNAL_ERROR);
+    return;
+  }
 
   // Using base::Unretained here is safe because Service() returns a singleton.
   // TODO(https://crbug.com/796991): Get service worker registration from its
@@ -177,6 +205,7 @@ void BlinkNotificationServiceImpl::DisplayPersistentNotification(
           origin_.GetURL() /* service_worker_scope */,
           origin_.GetURL() /* origin */, platform_notification_data,
           notification_resources));
+
   std::move(callback).Run(blink::mojom::PersistentNotificationError::NONE);
 }
 

content/browser/notifications/blink_notification_service_impl.h

@@ -60,6 +60,14 @@ class CONTENT_EXPORT BlinkNotificationServiceImpl
       const content::NotificationResources& notification_resources,
       blink::mojom::NonPersistentNotificationListenerPtrInfo listener_ptr_info);
 
+  void DisplayPersistentNotificationWithId(
+      int64_t service_worker_registration_id,
+      const PlatformNotificationData& platform_notification_data,
+      const NotificationResources& notification_resources,
+      DisplayPersistentNotificationCallback callback,
+      bool success,
+      const std::string& notification_id);
+
   void CloseNonPersistentNotificationOnUIThread(
       const std::string& notification_id);
 

content/browser/notifications/blink_notification_service_impl_unittest.cc

@@ -92,6 +92,11 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
         nullptr /* service_worker_context */);
     notification_context_->Initialize();
 
+    // Wait for notification context to be initialized to avoid TSAN detecting
+    // a memory race in tests - in production the PlatformNotificationContext
+    // will be initialized long before it is read from so this is fine.
+    RunAllTasksUntilIdle();
+
     blink::mojom::NotificationServicePtr notification_service_ptr;
     notification_service_ = std::make_unique<BlinkNotificationServiceImpl>(
         notification_context_.get(), &browser_context_, &resource_context_,
@@ -109,8 +114,10 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
   }
 
   void DidDisplayPersistentNotification(
+      base::OnceClosure quit_closure,
       blink::mojom::PersistentNotificationError error) {
     display_persistent_callback_result_ = error;
+    std::move(quit_closure).Run();
   }
 
   void DidGetDisplayedNotifications(
@@ -121,6 +128,17 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
     std::move(quit_closure).Run();
   }
 
+  void DisplayPersistentNotificationSync() {
+    base::RunLoop run_loop;
+    notification_service_->DisplayPersistentNotification(
+        kFakeServiceWorkerRegistrationId, PlatformNotificationData(),
+        NotificationResources(),
+        base::BindOnce(
+            &BlinkNotificationServiceImplTest::DidDisplayPersistentNotification,
+            base::Unretained(this), run_loop.QuitClosure()));
+    run_loop.Run();
+  }
+
   // Synchronous wrapper of
   // PlatformNotificationService::GetDisplayedNotifications
   std::set<std::string> GetDisplayedNotifications() {
@@ -227,12 +245,8 @@ TEST_F(BlinkNotificationServiceImplTest,
        DisplayPersistentNotificationWithPermission) {
   mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::GRANTED);
 
-  notification_service_->DisplayPersistentNotification(
-      kFakeServiceWorkerRegistrationId, PlatformNotificationData(),
-      NotificationResources(),
-      base::BindOnce(
-          &BlinkNotificationServiceImplTest::DidDisplayPersistentNotification,
-          base::Unretained(this)));
+  DisplayPersistentNotificationSync();
+
   EXPECT_EQ(blink::mojom::PersistentNotificationError::NONE,
             display_persistent_callback_result_);
 
@@ -246,12 +260,7 @@ TEST_F(BlinkNotificationServiceImplTest,
        DisplayPersistentNotificationWithoutPermission) {
   mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::DENIED);
 
-  notification_service_->DisplayPersistentNotification(
-      kFakeServiceWorkerRegistrationId, PlatformNotificationData(),
-      NotificationResources(),
-      base::BindOnce(
-          &BlinkNotificationServiceImplTest::DidDisplayPersistentNotification,
-          base::Unretained(this)));
+  DisplayPersistentNotificationSync();
 
   EXPECT_EQ(blink::mojom::PersistentNotificationError::PERMISSION_DENIED,
             display_persistent_callback_result_);
@@ -262,4 +271,17 @@ TEST_F(BlinkNotificationServiceImplTest,
   EXPECT_EQ(0u, GetDisplayedNotifications().size());
 }
 
+TEST_F(BlinkNotificationServiceImplTest,
+       DisplayMultiplePersistentNotifications) {
+  mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::GRANTED);
+
+  DisplayPersistentNotificationSync();
+
+  DisplayPersistentNotificationSync();
+
+  // Wait for service to receive all the Display calls.
+  RunAllTasksUntilIdle();
+
+  EXPECT_EQ(2u, GetDisplayedNotifications().size());
+}
 }  // namespace content