diff options
author | peter <peter@chromium.org> | 2015-05-19 12:35:09 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-19 19:35:12 +0000 |
commit | 5b494b3b5f13d6a89afead81e3081a298e10b23b (patch) | |
tree | 36f8d650a01e926abba0cc74f72ab1a4f2593ffc /content | |
parent | 3e4787d6fe8c22a59c02cb84c03c9f6adde10a83 (diff) | |
download | chromium_src-5b494b3b5f13d6a89afead81e3081a298e10b23b.zip chromium_src-5b494b3b5f13d6a89afead81e3081a298e10b23b.tar.gz chromium_src-5b494b3b5f13d6a89afead81e3081a298e10b23b.tar.bz2 |
Beginnings of synchronizing notifications in the notification database.
For both desktop Chrome and Android we need to support a mechanism for
synchronizing open notifications. For desktop the most important case
is where Chrome just started up, where there won't be any notifications
since the message center is owned by Chrome.
For Android, there are more cases, since the platform hosts displaying
the notifications and informs Chrome through Intents of mutations. This
patch leaves synchronization there mostly out of scope, while still
laying the early groundwork towards supporting this.
BUG=442143
Review URL: https://codereview.chromium.org/1127013008
Cr-Commit-Position: refs/heads/master@{#330583}
Diffstat (limited to 'content')
7 files changed, 83 insertions, 3 deletions
diff --git a/content/browser/notifications/platform_notification_context_impl.cc b/content/browser/notifications/platform_notification_context_impl.cc index db6f136..bdeabce 100644 --- a/content/browser/notifications/platform_notification_context_impl.cc +++ b/content/browser/notifications/platform_notification_context_impl.cc @@ -11,7 +11,9 @@ #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/content_browser_client.h" #include "content/public/browser/notification_database_data.h" +#include "content/public/browser/platform_notification_service.h" using base::DoNothing; @@ -24,8 +26,10 @@ const base::FilePath::CharType kPlatformNotificationsDirectory[] = PlatformNotificationContextImpl::PlatformNotificationContextImpl( const base::FilePath& path, + BrowserContext* browser_context, const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context) : path_(path), + browser_context_(browser_context), service_worker_context_(service_worker_context) { DCHECK_CURRENTLY_ON(BrowserThread::UI); } @@ -43,6 +47,30 @@ PlatformNotificationContextImpl::~PlatformNotificationContextImpl() { void PlatformNotificationContextImpl::Initialize() { DCHECK_CURRENTLY_ON(BrowserThread::UI); + PlatformNotificationService* service = + GetContentClient()->browser()->GetPlatformNotificationService(); + if (service) { + std::set<std::string> displayed_notifications; + + bool notification_synchronization_supported = + service->GetDisplayedPersistentNotifications(browser_context_, + &displayed_notifications); + + // Synchronize the notifications stored in the database with the set of + // displaying notifications in |displayed_notifications|. This is necessary + // because flakiness may cause a platform to inform Chrome of a notification + // that has since been closed, or because the platform does not support + // notifications that exceed the lifetime of the browser process. + + // TODO(peter): Synchronizing the actual notifications will be done when the + // persistent notification ids are stable. For M44 we need to support the + // case where there may be no notifications after a Chrome restart. + if (notification_synchronization_supported && + !displayed_notifications.size()) { + prune_database_on_open_ = true; + } + } + BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, @@ -330,10 +358,23 @@ void PlatformNotificationContextImpl::OpenDatabase( UMA_HISTOGRAM_ENUMERATION("Notifications.Database.OpenResult", status, NotificationDatabase::STATUS_COUNT); + // TODO(peter): Do finer-grained synchronization here. + if (prune_database_on_open_) { + prune_database_on_open_ = false; + DestroyDatabase(); + + database_.reset(new NotificationDatabase(GetDatabasePath())); + status = database_->Open(true /* create_if_missing */); + + // TODO(peter): Find the appropriate UMA to cover in regards to + // synchronizing notifications after the implementation is complete. + } + // When the database could not be opened due to corruption, destroy it, blow // away the contents of the directory and try re-opening the database. if (status == NotificationDatabase::STATUS_ERROR_CORRUPTED) { if (DestroyDatabase()) { + database_.reset(new NotificationDatabase(GetDatabasePath())); status = database_->Open(true /* create_if_missing */); UMA_HISTOGRAM_ENUMERATION( diff --git a/content/browser/notifications/platform_notification_context_impl.h b/content/browser/notifications/platform_notification_context_impl.h index c6b370d..a41729b 100644 --- a/content/browser/notifications/platform_notification_context_impl.h +++ b/content/browser/notifications/platform_notification_context_impl.h @@ -6,6 +6,8 @@ #define CONTENT_BROWSER_NOTIFICATIONS_PLATFORM_NOTIFICATION_CONTEXT_IMPL_H_ #include <stdint.h> +#include <set> +#include <string> #include "base/callback.h" #include "base/compiler_specific.h" @@ -25,6 +27,7 @@ class SequencedTaskRunner; namespace content { +class BrowserContext; class NotificationDatabase; struct NotificationDatabaseData; class ServiceWorkerContextWrapper; @@ -42,6 +45,7 @@ class CONTENT_EXPORT PlatformNotificationContextImpl // constructor must only be called on the IO thread. PlatformNotificationContextImpl( const base::FilePath& path, + BrowserContext* browser_context, const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context); // To be called on the UI thread to initialize the instance. @@ -139,12 +143,16 @@ class CONTENT_EXPORT PlatformNotificationContextImpl const scoped_refptr<base::SequencedTaskRunner>& task_runner); base::FilePath path_; + BrowserContext* browser_context_; scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_; scoped_refptr<base::SequencedTaskRunner> task_runner_; scoped_ptr<NotificationDatabase> database_; + // Indicates whether the database should be pruned when it's opened. + bool prune_database_on_open_ = false; + DISALLOW_COPY_AND_ASSIGN(PlatformNotificationContextImpl); }; diff --git a/content/browser/notifications/platform_notification_context_unittest.cc b/content/browser/notifications/platform_notification_context_unittest.cc index 6dca0d0..30ab46a 100644 --- a/content/browser/notifications/platform_notification_context_unittest.cc +++ b/content/browser/notifications/platform_notification_context_unittest.cc @@ -11,6 +11,7 @@ #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_context.h" #include "content/public/test/test_browser_thread_bundle.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" @@ -86,7 +87,9 @@ class PlatformNotificationContextTest : public ::testing::Test { // current message loop proxy will be used as the task runner. PlatformNotificationContextImpl* CreatePlatformNotificationContext() { PlatformNotificationContextImpl* context = - new PlatformNotificationContextImpl(base::FilePath(), nullptr); + new PlatformNotificationContextImpl(base::FilePath(), + &browser_context_, + nullptr); context->Initialize(); OverrideTaskRunnerForTesting(context); @@ -99,6 +102,9 @@ class PlatformNotificationContextTest : public ::testing::Test { context->SetTaskRunnerForTesting(base::MessageLoopProxy::current()); } + // Returns the testing browsing context that can be used for this test. + BrowserContext* browser_context() { return &browser_context_; } + // Returns whether the last invoked callback finished successfully. bool success() const { return success_; } @@ -113,6 +119,7 @@ class PlatformNotificationContextTest : public ::testing::Test { private: TestBrowserThreadBundle thread_bundle_; + TestBrowserContext browser_context_; bool success_; NotificationDatabaseData database_data_; @@ -240,6 +247,7 @@ TEST_F(PlatformNotificationContextTest, ServiceWorkerUnregistered) { scoped_refptr<PlatformNotificationContextImpl> notification_context( new PlatformNotificationContextImpl( base::FilePath(), + browser_context(), embedded_worker_test_helper->context_wrapper())); notification_context->Initialize(); @@ -343,7 +351,9 @@ TEST_F(PlatformNotificationContextTest, DestroyOnDiskDatabase) { // Manually construct the PlatformNotificationContextImpl because this test // requires the database to be created on the filesystem. scoped_refptr<PlatformNotificationContextImpl> context( - new PlatformNotificationContextImpl(database_dir.path(), nullptr)); + new PlatformNotificationContextImpl(database_dir.path(), + browser_context(), + nullptr)); OverrideTaskRunnerForTesting(context.get()); diff --git a/content/browser/storage_partition_impl.cc b/content/browser/storage_partition_impl.cc index 53c928c8..dc4f55d 100644 --- a/content/browser/storage_partition_impl.cc +++ b/content/browser/storage_partition_impl.cc @@ -521,7 +521,8 @@ StoragePartitionImpl* StoragePartitionImpl::Create( new NavigatorConnectServiceWorkerServiceFactory(service_worker_context))); scoped_refptr<PlatformNotificationContextImpl> platform_notification_context = - new PlatformNotificationContextImpl(path, service_worker_context); + new PlatformNotificationContextImpl(path, context, + service_worker_context); platform_notification_context->Initialize(); scoped_refptr<BackgroundSyncContextImpl> background_sync_context = diff --git a/content/public/browser/platform_notification_service.h b/content/public/browser/platform_notification_service.h index afcc319..6c9e509 100644 --- a/content/public/browser/platform_notification_service.h +++ b/content/public/browser/platform_notification_service.h @@ -6,6 +6,7 @@ #define CONTENT_PUBLIC_BROWSER_PLATFORM_NOTIFICATION_SERVICE_H_ #include <stdint.h> +#include <set> #include <string> #include "base/callback_forward.h" @@ -72,6 +73,13 @@ class CONTENT_EXPORT PlatformNotificationService { virtual void ClosePersistentNotification( BrowserContext* browser_context, int64_t persistent_notification_id) = 0; + + // Writes the ids of all currently displaying persistent notifications for the + // given |browser_context| to |displayed_notifications|. Returns whether the + // platform is able to provide such a set. + virtual bool GetDisplayedPersistentNotifications( + BrowserContext* browser_context, + std::set<std::string>* displayed_notifications) = 0; }; } // namespace content diff --git a/content/shell/browser/layout_test/layout_test_notification_manager.cc b/content/shell/browser/layout_test/layout_test_notification_manager.cc index a30c719..057cc17 100644 --- a/content/shell/browser/layout_test/layout_test_notification_manager.cc +++ b/content/shell/browser/layout_test/layout_test_notification_manager.cc @@ -97,6 +97,15 @@ void LayoutTestNotificationManager::ClosePersistentNotification( } } +bool LayoutTestNotificationManager::GetDisplayedPersistentNotifications( + BrowserContext* browser_context, + std::set<std::string>* displayed_notifications) { + DCHECK(displayed_notifications); + + // Notifications will never outlive the lifetime of running layout tests. + return false; +} + void LayoutTestNotificationManager::SimulateClick(const std::string& title) { DCHECK_CURRENTLY_ON(BrowserThread::UI); diff --git a/content/shell/browser/layout_test/layout_test_notification_manager.h b/content/shell/browser/layout_test/layout_test_notification_manager.h index ac51c5c..53c5930 100644 --- a/content/shell/browser/layout_test/layout_test_notification_manager.h +++ b/content/shell/browser/layout_test/layout_test_notification_manager.h @@ -55,6 +55,9 @@ class LayoutTestNotificationManager : public PlatformNotificationService { void ClosePersistentNotification( BrowserContext* browser_context, int64_t persistent_notification_id) override; + bool GetDisplayedPersistentNotifications( + BrowserContext* browser_context, + std::set<std::string>* displayed_notifications) override; private: // Structure to represent the information of a persistent notification. |