The platform notification context should observe the Service Worker Context.

When a Service Worker registration gets dropped, all notifications
associated with that notification should be removed as well.

Two cases which will be addressed in follow-up patches are when the
Service Worker database gets wiped, and granting the notification context
the ability to close deleted notifications by itself.

Design document:
  http://goo.gl/TciXVp

BUG=447628

Review URL: https://codereview.chromium.org/1014703007

Cr-Commit-Position: refs/heads/master@{#322692}

content/browser/notifications/notification_message_filter.cc

@@ -6,12 +6,12 @@
 
 #include "base/callback.h"
 #include "content/browser/notifications/page_notification_delegate.h"
+#include "content/browser/notifications/platform_notification_context_impl.h"
 #include "content/common/platform_notification_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/desktop_notification_delegate.h"
-#include "content/public/browser/platform_notification_context.h"
 #include "content/public/browser/platform_notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_client.h"
@@ -20,7 +20,7 @@ namespace content {
 
 NotificationMessageFilter::NotificationMessageFilter(
     int process_id,
-    PlatformNotificationContext* notification_context,
+    PlatformNotificationContextImpl* notification_context,
     ResourceContext* resource_context,
     BrowserContext* browser_context)
     : BrowserMessageFilter(PlatformNotificationMsgStart),

content/browser/notifications/notification_message_filter.h

@@ -17,7 +17,7 @@ class SkBitmap;
 namespace content {
 
 class BrowserContext;
-class PlatformNotificationContext;
+class PlatformNotificationContextImpl;
 struct PlatformNotificationData;
 class PlatformNotificationService;
 class ResourceContext;
@@ -26,7 +26,7 @@ class NotificationMessageFilter : public BrowserMessageFilter {
  public:
   NotificationMessageFilter(
       int process_id,
-      PlatformNotificationContext* notification_context,
+      PlatformNotificationContextImpl* notification_context,
       ResourceContext* resource_context,
       BrowserContext* browser_context);
 
@@ -73,7 +73,7 @@ class NotificationMessageFilter : public BrowserMessageFilter {
       const GURL& origin);
 
   int process_id_;
-  scoped_refptr<PlatformNotificationContext> notification_context_;
+  scoped_refptr<PlatformNotificationContextImpl> notification_context_;
   ResourceContext* resource_context_;
   BrowserContext* browser_context_;
 

content/browser/notifications/platform_notification_context_impl.cc

@@ -4,11 +4,15 @@
 
 #include "content/browser/notifications/platform_notification_context_impl.h"
 
+#include "base/bind_helpers.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "content/browser/notifications/notification_database.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_database_data.h"
 
+using base::DoNothing;
+
 namespace content {
 
 // Name of the directory in the user's profile directory where the notification
@@ -17,11 +21,16 @@ const base::FilePath::CharType kPlatformNotificationsDirectory[] =
     FILE_PATH_LITERAL("Platform Notifications");
 
 PlatformNotificationContextImpl::PlatformNotificationContextImpl(
-    const base::FilePath& path)
-    : path_(path) {
+    const base::FilePath& path,
+    const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context)
+    : path_(path),
+      service_worker_context_(service_worker_context) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
 PlatformNotificationContextImpl::~PlatformNotificationContextImpl() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   // If the database has been initialized, it must be deleted on the task runner
   // thread as closing it may cause file I/O.
   if (database_) {
@@ -30,6 +39,38 @@ PlatformNotificationContextImpl::~PlatformNotificationContextImpl() {
   }
 }
 
+void PlatformNotificationContextImpl::Initialize() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&PlatformNotificationContextImpl::InitializeOnIO, this));
+}
+
+void PlatformNotificationContextImpl::InitializeOnIO() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  // |service_worker_context_| may be NULL in tests.
+  if (service_worker_context_)
+    service_worker_context_->AddObserver(this);
+}
+
+void PlatformNotificationContextImpl::Shutdown() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&PlatformNotificationContextImpl::ShutdownOnIO, this));
+}
+
+void PlatformNotificationContextImpl::ShutdownOnIO() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  // |service_worker_context_| may be NULL in tests.
+  if (service_worker_context_)
+    service_worker_context_->RemoveObserver(this);
+}
+
 void PlatformNotificationContextImpl::ReadNotificationData(
     int64_t notification_id,
     const GURL& origin,
@@ -144,6 +185,33 @@ void PlatformNotificationContextImpl::DoDeleteNotificationData(
                           base::Bind(callback, success));
 }
 
+void PlatformNotificationContextImpl::OnRegistrationDeleted(
+    int64_t registration_id,
+    const GURL& pattern) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  LazyInitialize(
+      base::Bind(&PlatformNotificationContextImpl::
+                     DoDeleteNotificationsForServiceWorkerRegistration,
+                 this, pattern.GetOrigin(), registration_id),
+      base::Bind(&DoNothing));
+}
+
+void PlatformNotificationContextImpl::
+    DoDeleteNotificationsForServiceWorkerRegistration(
+        const GURL& origin,
+        int64_t service_worker_registration_id) {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
+  std::set<int64_t> deleted_notifications_set;
+  database_->DeleteAllNotificationDataForServiceWorkerRegistration(
+        origin, service_worker_registration_id, &deleted_notifications_set);
+
+  // TODO(peter): Record UMA on status for deleting from the database.
+  // TODO(peter): Do the DeleteAndStartOver dance for STATUS_ERROR_CORRUPTED.
+
+  // TODO(peter): Close the notifications in |deleted_notifications_set|.
+}
+
 void PlatformNotificationContextImpl::LazyInitialize(
     const base::Closure& success_closure,
     const base::Closure& failure_closure) {

content/browser/notifications/platform_notification_context_impl.h

@@ -10,8 +10,11 @@
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
+#include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
+#include "content/browser/service_worker/service_worker_context_observer.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/platform_notification_context.h"
 
 class GURL;
@@ -24,16 +27,30 @@ namespace content {
 
 class NotificationDatabase;
 struct NotificationDatabaseData;
+class ServiceWorkerContextWrapper;
 
 // Implementation of the Web Notification storage context. The public methods
-// defined in this interface must only be called on the IO thread.
+// defined in this interface must only be called on the IO thread unless
+// otherwise specified.
 class CONTENT_EXPORT PlatformNotificationContextImpl
-    : public NON_EXPORTED_BASE(PlatformNotificationContext) {
+    : NON_EXPORTED_BASE(public PlatformNotificationContext),
+      NON_EXPORTED_BASE(public ServiceWorkerContextObserver),
+      public base::RefCountedThreadSafe<PlatformNotificationContextImpl,
+                                        BrowserThread::DeleteOnUIThread> {
  public:
   // Constructs a new platform notification context. If |path| is non-empty, the
   // database will be initialized in the "Platform Notifications" subdirectory
-  // of |path|. Otherwise, the database will be initialized in memory.
-  explicit PlatformNotificationContextImpl(const base::FilePath& path);
+  // of |path|. Otherwise, the database will be initialized in memory. The
+  // constructor must only be called on the IO thread.
+  PlatformNotificationContextImpl(
+      const base::FilePath& path,
+      const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context);
+
+  // To be called on the UI thread to initialize the instance.
+  void Initialize();
+
+  // To be called on the UI thread when the context is being shut down.
+  void Shutdown();
 
   // PlatformNotificationContext implementation.
   void ReadNotificationData(int64_t notification_id,
@@ -46,11 +63,22 @@ class CONTENT_EXPORT PlatformNotificationContextImpl
                               const GURL& origin,
                               const DeleteResultCallback& callback) override;
 
+  // ServiceWorkerContextObserver implementation.
+  void OnRegistrationDeleted(int64_t registration_id,
+                             const GURL& pattern) override;
+
  private:
+  friend class base::DeleteHelper<PlatformNotificationContextImpl>;
+  friend class base::RefCountedThreadSafe<PlatformNotificationContextImpl,
+                                        BrowserThread::DeleteOnUIThread>;
+  friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class PlatformNotificationContextTest;
 
   ~PlatformNotificationContextImpl() override;
 
+  void InitializeOnIO();
+  void ShutdownOnIO();
+
   // Initializes the database if neccesary. Must be called on the IO thread.
   // |success_closure| will be invoked on a the |task_runner_| thread when
   // everything is available, or |failure_closure_| will be invoked on the
@@ -85,6 +113,12 @@ class CONTENT_EXPORT PlatformNotificationContextImpl
                                 const GURL& origin,
                                 const DeleteResultCallback& callback);
 
+  // Deletes all notifications associated with |service_worker_registration_id|
+  // belonging to |origin|. Must be called on the |task_runner_| thread.
+  void DoDeleteNotificationsForServiceWorkerRegistration(
+      const GURL& origin,
+      int64_t service_worker_registration_id);
+
   // Returns the path in which the database should be initialized. May be empty.
   base::FilePath GetDatabasePath() const;
 
@@ -94,6 +128,8 @@ class CONTENT_EXPORT PlatformNotificationContextImpl
 
   base::FilePath path_;
 
+  scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
+
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
   scoped_ptr<NotificationDatabase> database_;
 

content/browser/notifications/platform_notification_context_unittest.cc

@@ -5,6 +5,9 @@
 #include "base/bind.h"
 #include "base/run_loop.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/browser/notification_database_data.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -12,6 +15,9 @@
 
 namespace content {
 
+// Fake render process id to use in tests requiring one.
+const int kFakeRenderProcessId = 99;
+
 class PlatformNotificationContextTest : public ::testing::Test {
  public:
   PlatformNotificationContextTest()
@@ -36,18 +42,45 @@ class PlatformNotificationContextTest : public ::testing::Test {
     success_ = success;
   }
 
+  // Callback to provide when registering a Service Worker with a Service
+  // Worker Context. Will write the registration id to |store_registration_id|.
+  void DidRegisterServiceWorker(int64_t* store_registration_id,
+                                ServiceWorkerStatusCode status,
+                                const std::string& status_message,
+                                int64_t service_worker_registration_id) {
+    DCHECK(store_registration_id);
+    EXPECT_EQ(SERVICE_WORKER_OK, status);
+
+    *store_registration_id = service_worker_registration_id;
+  }
+
+  // Callback to provide when unregistering a Service Worker. Will write the
+  // resulting status code to |store_status|.
+  void DidUnregisterServiceWorker(ServiceWorkerStatusCode* store_status,
+                                  ServiceWorkerStatusCode status) {
+    DCHECK(store_status);
+    *store_status = status;
+  }
+
  protected:
   // Creates a new PlatformNotificationContextImpl instance. When using this
   // method, the underlying database will always be created in memory. The
   // current message loop proxy will be used as the task runner.
   PlatformNotificationContextImpl* CreatePlatformNotificationContext() {
     PlatformNotificationContextImpl* context =
-        new PlatformNotificationContextImpl(base::FilePath());
+        new PlatformNotificationContextImpl(base::FilePath(), nullptr);
+    context->Initialize();
 
-    context->SetTaskRunnerForTesting(base::MessageLoopProxy::current());
+    OverrideTaskRunnerForTesting(context);
     return context;
   }
 
+  // Overrides the task runner in |context| with the current message loop
+  // proxy, to reduce the number of threads involved in the tests.
+  void OverrideTaskRunnerForTesting(PlatformNotificationContextImpl* context) {
+    context->SetTaskRunnerForTesting(base::MessageLoopProxy::current());
+  }
+
   // Returns whether the last invoked callback finished successfully.
   bool success() const { return success_; }
 
@@ -180,4 +213,72 @@ TEST_F(PlatformNotificationContextTest, DeleteNotification) {
   EXPECT_FALSE(success());
 }
 
+TEST_F(PlatformNotificationContextTest, ServiceWorkerUnregistered) {
+  scoped_ptr<EmbeddedWorkerTestHelper> embedded_worker_test_helper(
+      new EmbeddedWorkerTestHelper(base::FilePath(), kFakeRenderProcessId));
+
+  // Manually create the PlatformNotificationContextImpl so that the Service
+  // Worker context wrapper can be passed in.
+  scoped_refptr<PlatformNotificationContextImpl> notification_context(
+      new PlatformNotificationContextImpl(
+          base::FilePath(),
+          embedded_worker_test_helper->context_wrapper()));
+  notification_context->Initialize();
+
+  OverrideTaskRunnerForTesting(notification_context.get());
+
+  GURL origin("https://example.com");
+  GURL script_url("https://example.com/worker.js");
+
+  int64_t service_worker_registration_id = kInvalidServiceWorkerRegistrationId;
+
+  // Register a Service Worker to get a valid registration id.
+  embedded_worker_test_helper->context()->RegisterServiceWorker(
+      origin,
+      script_url,
+      nullptr /* provider_host */,
+      base::Bind(&PlatformNotificationContextTest::DidRegisterServiceWorker,
+                 base::Unretained(this), &service_worker_registration_id));
+
+  base::RunLoop().RunUntilIdle();
+  ASSERT_NE(service_worker_registration_id,
+            kInvalidServiceWorkerRegistrationId);
+
+  NotificationDatabaseData notification_database_data;
+
+  // Create a notification for that Service Worker registration.
+  notification_context->WriteNotificationData(
+      origin,
+      notification_database_data,
+      base::Bind(&PlatformNotificationContextTest::DidWriteNotificationData,
+                 base::Unretained(this)));
+
+  base::RunLoop().RunUntilIdle();
+
+  ASSERT_TRUE(success());
+  EXPECT_GT(notification_id(), 0);
+
+  ServiceWorkerStatusCode unregister_status;
+
+  // Now drop the Service Worker registration which owns that notification.
+  embedded_worker_test_helper->context()->UnregisterServiceWorker(
+      origin,
+      base::Bind(&PlatformNotificationContextTest::DidUnregisterServiceWorker,
+                 base::Unretained(this), &unregister_status));
+
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(SERVICE_WORKER_OK, unregister_status);
+
+  // And verify that the associated notification has indeed been dropped.
+  notification_context->ReadNotificationData(
+      notification_id(),
+      origin,
+      base::Bind(&PlatformNotificationContextTest::DidReadNotificationData,
+                 base::Unretained(this)));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(success());
+}
+
 }  // namespace content

content/browser/storage_partition_impl.cc

@@ -422,6 +422,9 @@ StoragePartitionImpl::~StoragePartitionImpl() {
 
   if (GetGeofencingManager())
     GetGeofencingManager()->Shutdown();
+
+  if (GetPlatformNotificationContext())
+    GetPlatformNotificationContext()->Shutdown();
 }
 
 StoragePartitionImpl* StoragePartitionImpl::Create(
@@ -510,7 +513,8 @@ StoragePartitionImpl* StoragePartitionImpl::Create(
       new NavigatorConnectServiceWorkerServiceFactory(service_worker_context)));
 
   scoped_refptr<PlatformNotificationContextImpl> platform_notification_context =
-      new PlatformNotificationContextImpl(path);
+      new PlatformNotificationContextImpl(path, service_worker_context);
+  platform_notification_context->Initialize();
 
   StoragePartitionImpl* storage_partition = new StoragePartitionImpl(
       context, partition_path, quota_manager.get(), appcache_service.get(),

content/public/browser/platform_notification_context.h

@@ -8,7 +8,6 @@
 #include <stdint.h>
 
 #include "base/callback.h"
-#include "base/memory/ref_counted.h"
 
 class GURL;
 
@@ -19,8 +18,7 @@ struct NotificationDatabaseData;
 // Represents the storage context for persistent Web Notifications, specific to
 // the storage partition owning the instance. All methods defined in this
 // interface may only be used on the IO thread.
-class PlatformNotificationContext
-    : public base::RefCountedThreadSafe<PlatformNotificationContext> {
+class PlatformNotificationContext {
  public:
   using ReadResultCallback =
       base::Callback<void(bool /* success */,
@@ -55,8 +53,6 @@ class PlatformNotificationContext
                                       const DeleteResultCallback& callback) = 0;
 
  protected:
-  friend class base::RefCountedThreadSafe<PlatformNotificationContext>;
-
   virtual ~PlatformNotificationContext() {}
 };