[Notifications] Use the service worker scope when displaying via mojo

- Display() Part 3: Attribute notification correctly by looking up
service worker scope from the service worker registration ID.

- Note the mojo pathway is guarded behind the NotificationsWithMojo
flag as development continues.

R=peter@chromium.org

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

chrome/browser/notifications/platform_notification_service_interactive_uitest.cc

@@ -1034,3 +1034,22 @@ IN_PROC_BROWSER_TEST_F(PlatformNotificationServiceMojoEnabledBrowserTest,
       GetDisplayedNotifications(true /* is_persistent */);
   ASSERT_EQ(1u, notifications.size());
 }
+
+IN_PROC_BROWSER_TEST_F(PlatformNotificationServiceMojoEnabledBrowserTest,
+                       PersistentNotificationServiceWorkerScope) {
+  RequestAndAcceptPermission();
+
+  // Creates a simple notification.
+  std::string script_result;
+  ASSERT_TRUE(RunScript("DisplayPersistentNotification()", &script_result));
+
+  std::vector<message_center::Notification> notifications =
+      GetDisplayedNotifications(true /* is_persistent */);
+  ASSERT_EQ(1u, notifications.size());
+
+  EXPECT_EQ(
+      TestPageUrl(),
+      PersistentNotificationMetadata::From(
+          display_service_tester_->GetMetadataForNotification(notifications[0]))
+          ->service_worker_scope);
+}

content/browser/notifications/blink_notification_service_impl.cc

@@ -10,6 +10,7 @@
 #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/common/service_worker/service_worker_status_code.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_database_data.h"
@@ -35,12 +36,14 @@ BlinkNotificationServiceImpl::BlinkNotificationServiceImpl(
     PlatformNotificationContextImpl* notification_context,
     BrowserContext* browser_context,
     ResourceContext* resource_context,
+    scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,
     int render_process_id,
     const url::Origin& origin,
     mojo::InterfaceRequest<blink::mojom::NotificationService> request)
     : notification_context_(notification_context),
       browser_context_(browser_context),
       resource_context_(resource_context),
+      service_worker_context_(std::move(service_worker_context)),
       render_process_id_(render_process_id),
       origin_(origin),
       binding_(this, std::move(request)),
@@ -188,22 +191,55 @@ void BlinkNotificationServiceImpl::DisplayPersistentNotificationWithId(
     DisplayPersistentNotificationCallback callback,
     bool success,
     const std::string& notification_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   if (!success) {
     std::move(callback).Run(
         blink::mojom::PersistentNotificationError::INTERNAL_ERROR);
     return;
   }
 
+  service_worker_context_->FindReadyRegistrationForId(
+      service_worker_registration_id, origin_.GetURL(),
+      base::BindOnce(&BlinkNotificationServiceImpl::
+                         DisplayPersistentNotificationWithIdForServiceWorker,
+                     weak_ptr_factory_.GetWeakPtr(), notification_id,
+                     platform_notification_data, notification_resources,
+                     std::move(callback)));
+}
+
+void BlinkNotificationServiceImpl::
+    DisplayPersistentNotificationWithIdForServiceWorker(
+        const std::string& notification_id,
+        const PlatformNotificationData& platform_notification_data,
+        const NotificationResources& notification_resources,
+        DisplayPersistentNotificationCallback callback,
+        content::ServiceWorkerStatusCode service_worker_status,
+        scoped_refptr<content::ServiceWorkerRegistration> registration) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (service_worker_status != SERVICE_WORKER_OK) {
+    std::move(callback).Run(
+        blink::mojom::PersistentNotificationError::INTERNAL_ERROR);
+    LOG(ERROR) << "Registration not found for " << origin_.GetURL().spec();
+    // TODO(peter): Add UMA to track how often this occurs.
+    return;
+  }
+
+  if (registration->pattern().GetOrigin() != origin_.GetURL()) {
+    // Bail out, something's wrong.
+    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
-  // ID, and pass the service worker scope (instead of the origin twice) below.
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::BindOnce(
           &PlatformNotificationService::DisplayPersistentNotification,
           base::Unretained(Service()), browser_context_, notification_id,
-          origin_.GetURL() /* service_worker_scope */,
-          origin_.GetURL() /* origin */, platform_notification_data,
+          registration->pattern(), origin_.GetURL(), platform_notification_data,
           notification_resources));
 
   std::move(callback).Run(blink::mojom::PersistentNotificationError::NONE);

content/browser/notifications/blink_notification_service_impl.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/browser_context.h"
 #include "mojo/public/cpp/bindings/binding.h"
@@ -30,6 +31,7 @@ class CONTENT_EXPORT BlinkNotificationServiceImpl
       PlatformNotificationContextImpl* notification_context,
       BrowserContext* browser_context,
       ResourceContext* resource_context,
+      scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,
       int render_process_id,
       const url::Origin& origin,
       mojo::InterfaceRequest<blink::mojom::NotificationService> request);
@@ -68,6 +70,14 @@ class CONTENT_EXPORT BlinkNotificationServiceImpl
       bool success,
       const std::string& notification_id);
 
+  void DisplayPersistentNotificationWithIdForServiceWorker(
+      const std::string& notification_id,
+      const PlatformNotificationData& platform_notification_data,
+      const NotificationResources& notification_resources,
+      DisplayPersistentNotificationCallback callback,
+      content::ServiceWorkerStatusCode service_worker_status,
+      scoped_refptr<content::ServiceWorkerRegistration> registration);
+
   void CloseNonPersistentNotificationOnUIThread(
       const std::string& notification_id);
 
@@ -80,6 +90,8 @@ class CONTENT_EXPORT BlinkNotificationServiceImpl
 
   ResourceContext* resource_context_;
 
+  scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
+
   int render_process_id_;
 
   // The origin that this notification service is communicating with.

content/browser/notifications/blink_notification_service_impl_unittest.cc

@@ -7,6 +7,7 @@
 #include <vector>
 
 #include "base/callback_forward.h"
+#include "base/callback_helpers.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/run_loop.h"
@@ -14,7 +15,9 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/notifications/blink_notification_service_impl.h"
 #include "content/browser/notifications/platform_notification_context_impl.h"
+#include "content/browser/service_worker/embedded_worker_test_helper.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
+#include "content/common/service_worker/service_worker_types.h"
 #include "content/public/test/mock_resource_context.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -32,8 +35,9 @@ namespace content {
 namespace {
 
 const int kFakeRenderProcessId = 1;
+
 const char kTestOrigin[] = "https://example.com";
-const int64_t kFakeServiceWorkerRegistrationId = 1234;
+const char kTestServiceWorkerUrl[] = "https://example.com/sw.js";
 
 class MockNonPersistentNotificationListener
     : public blink::mojom::NonPersistentNotificationListener {
@@ -79,6 +83,8 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
   // at time of writing EmbeddedWorkerTestHelper didn't seem to support that.
   BlinkNotificationServiceImplTest()
       : thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP),
+        embedded_worker_helper_(
+            std::make_unique<EmbeddedWorkerTestHelper>(base::FilePath())),
         notification_browser_client_(&mock_platform_service_) {
     SetBrowserClientForTesting(&notification_browser_client_);
   }
@@ -89,7 +95,7 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
   void SetUp() override {
     notification_context_ = new PlatformNotificationContextImpl(
         base::FilePath(), &browser_context_,
-        nullptr /* service_worker_context */);
+        embedded_worker_helper_->context_wrapper());
     notification_context_->Initialize();
 
     // Wait for notification context to be initialized to avoid TSAN detecting
@@ -100,10 +106,88 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
     blink::mojom::NotificationServicePtr notification_service_ptr;
     notification_service_ = std::make_unique<BlinkNotificationServiceImpl>(
         notification_context_.get(), &browser_context_, &resource_context_,
-        kFakeRenderProcessId, url::Origin::Create(GURL(kTestOrigin)),
+        embedded_worker_helper_->context_wrapper(), kFakeRenderProcessId,
+        url::Origin::Create(GURL(kTestOrigin)),
         mojo::MakeRequest(&notification_service_ptr));
   }
 
+  void TearDown() override {
+    embedded_worker_helper_.reset();
+
+    // Give pending shutdown operations a chance to finish.
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void RegisterServiceWorker(
+      scoped_refptr<ServiceWorkerRegistration>* service_worker_registration) {
+    int64_t service_worker_registration_id =
+        blink::mojom::kInvalidServiceWorkerRegistrationId;
+
+    blink::mojom::ServiceWorkerRegistrationOptions options;
+    options.scope = GURL(kTestOrigin);
+
+    {
+      base::RunLoop run_loop;
+      embedded_worker_helper_->context()->RegisterServiceWorker(
+          GURL(kTestServiceWorkerUrl), options,
+          base::AdaptCallbackForRepeating(base::BindOnce(
+              &BlinkNotificationServiceImplTest::DidRegisterServiceWorker,
+              base::Unretained(this), &service_worker_registration_id,
+              run_loop.QuitClosure())));
+      run_loop.Run();
+    }
+
+    if (service_worker_registration_id ==
+        blink::mojom::kInvalidServiceWorkerRegistrationId) {
+      ADD_FAILURE() << "Could not obtain a valid Service Worker registration";
+    }
+
+    {
+      base::RunLoop run_loop;
+      embedded_worker_helper_->context()->storage()->FindRegistrationForId(
+          service_worker_registration_id, GURL(kTestOrigin),
+          base::BindOnce(&BlinkNotificationServiceImplTest::
+                             DidFindServiceWorkerRegistration,
+                         base::Unretained(this), service_worker_registration,
+                         run_loop.QuitClosure()));
+
+      run_loop.Run();
+    }
+
+    // Wait for the worker to be activated.
+    base::RunLoop().RunUntilIdle();
+
+    if (!*service_worker_registration) {
+      ADD_FAILURE() << "Could not find the new Service Worker registration.";
+    }
+  }
+
+  void DidRegisterServiceWorker(int64_t* out_service_worker_registration_id,
+                                base::OnceClosure quit_closure,
+                                ServiceWorkerStatusCode status,
+                                const std::string& status_message,
+                                int64_t service_worker_registration_id) {
+    DCHECK(out_service_worker_registration_id);
+    EXPECT_EQ(SERVICE_WORKER_OK, status) << ServiceWorkerStatusToString(status);
+
+    *out_service_worker_registration_id = service_worker_registration_id;
+
+    std::move(quit_closure).Run();
+  }
+
+  void DidFindServiceWorkerRegistration(
+      scoped_refptr<ServiceWorkerRegistration>* out_service_worker_registration,
+      base::OnceClosure quit_closure,
+      ServiceWorkerStatusCode status,
+      scoped_refptr<ServiceWorkerRegistration> service_worker_registration) {
+    DCHECK(out_service_worker_registration);
+    EXPECT_EQ(SERVICE_WORKER_OK, status) << ServiceWorkerStatusToString(status);
+
+    *out_service_worker_registration = service_worker_registration;
+
+    std::move(quit_closure).Run();
+  }
+
   void DidGetPermissionStatus(
       blink::mojom::PermissionStatus permission_status) {
     permission_callback_result_ = permission_status;
@@ -128,10 +212,11 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
     std::move(quit_closure).Run();
   }
 
-  void DisplayPersistentNotificationSync() {
+  void DisplayPersistentNotificationSync(
+      int64_t service_worker_registration_id) {
     base::RunLoop run_loop;
     notification_service_->DisplayPersistentNotification(
-        kFakeServiceWorkerRegistrationId, PlatformNotificationData(),
+        service_worker_registration_id, PlatformNotificationData(),
         NotificationResources(),
         base::BindOnce(
             &BlinkNotificationServiceImplTest::DidDisplayPersistentNotification,
@@ -155,6 +240,8 @@ class BlinkNotificationServiceImplTest : public ::testing::Test {
  protected:
   TestBrowserThreadBundle thread_bundle_;  // Must be first member.
 
+  std::unique_ptr<EmbeddedWorkerTestHelper> embedded_worker_helper_;
+
   std::unique_ptr<BlinkNotificationServiceImpl> notification_service_;
 
   TestBrowserContext browser_context_;
@@ -245,7 +332,10 @@ TEST_F(BlinkNotificationServiceImplTest,
        DisplayPersistentNotificationWithPermission) {
   mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::GRANTED);
 
-  DisplayPersistentNotificationSync();
+  scoped_refptr<ServiceWorkerRegistration> registration;
+  RegisterServiceWorker(&registration);
+
+  DisplayPersistentNotificationSync(registration->id());
 
   EXPECT_EQ(blink::mojom::PersistentNotificationError::NONE,
             display_persistent_callback_result_);
@@ -260,7 +350,10 @@ TEST_F(BlinkNotificationServiceImplTest,
        DisplayPersistentNotificationWithoutPermission) {
   mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::DENIED);
 
-  DisplayPersistentNotificationSync();
+  scoped_refptr<ServiceWorkerRegistration> registration;
+  RegisterServiceWorker(&registration);
+
+  DisplayPersistentNotificationSync(registration->id());
 
   EXPECT_EQ(blink::mojom::PersistentNotificationError::PERMISSION_DENIED,
             display_persistent_callback_result_);
@@ -275,9 +368,12 @@ TEST_F(BlinkNotificationServiceImplTest,
        DisplayMultiplePersistentNotifications) {
   mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::GRANTED);
 
-  DisplayPersistentNotificationSync();
+  scoped_refptr<ServiceWorkerRegistration> registration;
+  RegisterServiceWorker(&registration);
+
+  DisplayPersistentNotificationSync(registration->id());
 
-  DisplayPersistentNotificationSync();
+  DisplayPersistentNotificationSync(registration->id());
 
   // Wait for service to receive all the Display calls.
   RunAllTasksUntilIdle();

content/browser/notifications/platform_notification_context_impl.cc

@@ -140,8 +140,8 @@ void PlatformNotificationContextImpl::CreateServiceOnIO(
     mojo::InterfaceRequest<blink::mojom::NotificationService> request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   services_.push_back(std::make_unique<BlinkNotificationServiceImpl>(
-      this, browser_context_, resource_context, render_process_id, origin,
-      std::move(request)));
+      this, browser_context_, resource_context, service_worker_context_,
+      render_process_id, origin, std::move(request)));
 }
 
 void PlatformNotificationContextImpl::RemoveService(