diff options
Diffstat (limited to 'chromium/content/browser/notifications')
15 files changed, 825 insertions, 394 deletions
diff --git a/chromium/content/browser/notifications/DEPS b/chromium/content/browser/notifications/DEPS index a0ec7348e6f..adad170be21 100644 --- a/chromium/content/browser/notifications/DEPS +++ b/chromium/content/browser/notifications/DEPS @@ -1,3 +1,4 @@ include_rules = [ "+third_party/leveldatabase", -]
\ No newline at end of file + "+third_party/WebKit/public/platform/modules/notifications/notification_service.mojom.h", +] diff --git a/chromium/content/browser/notifications/blink_notification_service_impl.cc b/chromium/content/browser/notifications/blink_notification_service_impl.cc index a84707cdf4e..7bbfd53c36c 100644 --- a/chromium/content/browser/notifications/blink_notification_service_impl.cc +++ b/chromium/content/browser/notifications/blink_notification_service_impl.cc @@ -4,11 +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" @@ -39,7 +43,8 @@ BlinkNotificationServiceImpl::BlinkNotificationServiceImpl( resource_context_(resource_context), render_process_id_(render_process_id), origin_(origin), - binding_(this, std::move(request)) { + binding_(this, std::move(request)), + weak_ptr_factory_(this) { DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK(notification_context_); DCHECK(browser_context_); @@ -62,9 +67,7 @@ void BlinkNotificationServiceImpl::GetPermissionStatus( return; } - blink::mojom::PermissionStatus permission_status = - Service()->CheckPermissionOnIOThread(resource_context_, origin_.GetURL(), - render_process_id_); + blink::mojom::PermissionStatus permission_status = CheckPermissionStatus(); std::move(callback).Run(permission_status); } @@ -75,30 +78,135 @@ void BlinkNotificationServiceImpl::OnConnectionError() { } void BlinkNotificationServiceImpl::DisplayNonPersistentNotification( - const PlatformNotificationData& platform_notification_data) { + const std::string& token, + const PlatformNotificationData& platform_notification_data, + const NotificationResources& notification_resources, + blink::mojom::NonPersistentNotificationListenerPtr event_listener_ptr) { DCHECK_CURRENTLY_ON(BrowserThread::IO); if (!Service()) return; + if (CheckPermissionStatus() != blink::mojom::PermissionStatus::GRANTED) + return; + + std::string notification_id = + notification_context_->notification_id_generator() + ->GenerateForNonPersistentNotification(origin_, token); + + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::BindOnce(&BlinkNotificationServiceImpl:: + DisplayNonPersistentNotificationOnUIThread, + weak_ptr_factory_.GetWeakPtr(), notification_id, + origin_.GetURL(), platform_notification_data, + notification_resources, + event_listener_ptr.PassInterface())); +} + +void BlinkNotificationServiceImpl::DisplayNonPersistentNotificationOnUIThread( + const std::string& notification_id, + const GURL& origin, + const content::PlatformNotificationData& notification_data, + const content::NotificationResources& notification_resources, + blink::mojom::NonPersistentNotificationListenerPtrInfo listener_ptr_info) { + NotificationEventDispatcherImpl* event_dispatcher = + NotificationEventDispatcherImpl::GetInstance(); + event_dispatcher->RegisterNonPersistentNotificationListener( + notification_id, std::move(listener_ptr_info)); + + Service()->DisplayNotification(browser_context_, notification_id, origin, + notification_data, notification_resources); +} - // TODO(https://crbug.com/595685): Generate a GUID in the - // NotificationIdGenerator instead. - static int request_id = 0; - request_id++; +void BlinkNotificationServiceImpl::CloseNonPersistentNotification( + const std::string& token) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + if (!Service()) + return; std::string notification_id = notification_context_->notification_id_generator() - ->GenerateForNonPersistentNotification( - origin_.GetURL(), platform_notification_data.tag, request_id, - render_process_id_); + ->GenerateForNonPersistentNotification(origin_, token); + + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::BindOnce(&BlinkNotificationServiceImpl:: + CloseNonPersistentNotificationOnUIThread, + weak_ptr_factory_.GetWeakPtr(), notification_id)); +} + +void BlinkNotificationServiceImpl::CloseNonPersistentNotificationOnUIThread( + const std::string& notification_id) { + Service()->CloseNotification(browser_context_, notification_id); + + NotificationEventDispatcherImpl::GetInstance() + ->DispatchNonPersistentCloseEvent(notification_id); +} + +blink::mojom::PermissionStatus +BlinkNotificationServiceImpl::CheckPermissionStatus() { + return Service()->CheckPermissionOnIOThread( + resource_context_, origin_.GetURL(), render_process_id_); +} + +void BlinkNotificationServiceImpl::DisplayPersistentNotification( + int64_t service_worker_registration_id, + const PlatformNotificationData& platform_notification_data, + const NotificationResources& notification_resources, + DisplayPersistentNotificationCallback callback) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + if (!Service()) { + std::move(callback).Run( + blink::mojom::PersistentNotificationError::INTERNAL_ERROR); + return; + } + if (CheckPermissionStatus() != blink::mojom::PermissionStatus::GRANTED) { + std::move(callback).Run( + blink::mojom::PersistentNotificationError::PERMISSION_DENIED); + return; + } + + // 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; + } - // TODO(crbug.com/595685): Pass the actual notification resources here. - // Using base::Unretained is safe because Service() returns a singleton. + // 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::DisplayNotification, - base::Unretained(Service()), browser_context_, - notification_id, origin_.GetURL(), - platform_notification_data, NotificationResources())); + base::BindOnce( + &PlatformNotificationService::DisplayPersistentNotification, + base::Unretained(Service()), browser_context_, notification_id, + origin_.GetURL() /* service_worker_scope */, + origin_.GetURL() /* origin */, platform_notification_data, + notification_resources)); + + std::move(callback).Run(blink::mojom::PersistentNotificationError::NONE); } } // namespace content diff --git a/chromium/content/browser/notifications/blink_notification_service_impl.h b/chromium/content/browser/notifications/blink_notification_service_impl.h index 599012e7edb..5453d9d6c58 100644 --- a/chromium/content/browser/notifications/blink_notification_service_impl.h +++ b/chromium/content/browser/notifications/blink_notification_service_impl.h @@ -6,6 +6,8 @@ #define CONTENT_BROWSER_NOTIFICATIONS_BLINK_NOTIFICATION_SERVICE_IMPL_H_ #include "base/macros.h" +#include "base/memory/weak_ptr.h" +#include "content/common/content_export.h" #include "content/public/browser/browser_context.h" #include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/interface_request.h" @@ -21,7 +23,8 @@ class ResourceContext; // Implementation of the NotificationService used for Web Notifications. Is // responsible for displaying, updating and reading of both non-persistent // and persistent notifications. Lives on the IO thread. -class BlinkNotificationServiceImpl : public blink::mojom::NotificationService { +class CONTENT_EXPORT BlinkNotificationServiceImpl + : public blink::mojom::NotificationService { public: BlinkNotificationServiceImpl( PlatformNotificationContextImpl* notification_context, @@ -35,12 +38,41 @@ class BlinkNotificationServiceImpl : public blink::mojom::NotificationService { // blink::mojom::NotificationService implementation. void GetPermissionStatus(GetPermissionStatusCallback callback) override; void DisplayNonPersistentNotification( - const PlatformNotificationData& platform_notification_data) override; + const std::string& token, + const PlatformNotificationData& platform_notification_data, + const NotificationResources& notification_resources, + blink::mojom::NonPersistentNotificationListenerPtr listener_ptr) override; + void CloseNonPersistentNotification(const std::string& token) override; + void DisplayPersistentNotification( + int64_t service_worker_registration_id, + const PlatformNotificationData& platform_notification_data, + const NotificationResources& notification_resources, + DisplayPersistentNotificationCallback) override; private: // Called when an error is detected on binding_. void OnConnectionError(); + void DisplayNonPersistentNotificationOnUIThread( + const std::string& notification_id, + const GURL& origin, + const content::PlatformNotificationData& notification_data, + 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); + + blink::mojom::PermissionStatus CheckPermissionStatus(); + // The notification context that owns this service instance. PlatformNotificationContextImpl* notification_context_; @@ -55,6 +87,8 @@ class BlinkNotificationServiceImpl : public blink::mojom::NotificationService { mojo::Binding<blink::mojom::NotificationService> binding_; + base::WeakPtrFactory<BlinkNotificationServiceImpl> weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(BlinkNotificationServiceImpl); }; diff --git a/chromium/content/browser/notifications/blink_notification_service_impl_unittest.cc b/chromium/content/browser/notifications/blink_notification_service_impl_unittest.cc new file mode 100644 index 00000000000..66064f1b673 --- /dev/null +++ b/chromium/content/browser/notifications/blink_notification_service_impl_unittest.cc @@ -0,0 +1,287 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include <stdint.h> +#include <memory> +#include <vector> + +#include "base/callback_forward.h" +#include "base/macros.h" +#include "base/memory/scoped_refptr.h" +#include "base/run_loop.h" +#include "base/test/test_simple_task_runner.h" +#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/service_worker_context_wrapper.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" +#include "content/public/test/test_utils.h" +#include "content/test/mock_platform_notification_service.h" +#include "content/test/test_content_browser_client.h" +#include "mojo/public/cpp/bindings/binding.h" +#include "mojo/public/cpp/bindings/interface_request.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/WebKit/public/platform/modules/notifications/notification_service.mojom.h" +#include "third_party/WebKit/public/platform/modules/permissions/permission_status.mojom.h" + +namespace content { + +namespace { + +const int kFakeRenderProcessId = 1; +const char kTestOrigin[] = "https://example.com"; +const int64_t kFakeServiceWorkerRegistrationId = 1234; + +class MockNonPersistentNotificationListener + : public blink::mojom::NonPersistentNotificationListener { + public: + MockNonPersistentNotificationListener() : binding_(this) {} + ~MockNonPersistentNotificationListener() override = default; + + blink::mojom::NonPersistentNotificationListenerPtr GetPtr() { + blink::mojom::NonPersistentNotificationListenerPtr ptr; + binding_.Bind(mojo::MakeRequest(&ptr)); + return ptr; + } + + // NonPersistentNotificationListener interface. + void OnShow() override {} + void OnClick() override {} + void OnClose() override {} + + private: + mojo::Binding<blink::mojom::NonPersistentNotificationListener> binding_; +}; + +} // anonymous namespace + +// This is for overriding the Platform Notification Service with a mock one. +class NotificationBrowserClient : public TestContentBrowserClient { + public: + NotificationBrowserClient( + MockPlatformNotificationService* mock_platform_service) + : platform_notification_service_(mock_platform_service) {} + + PlatformNotificationService* GetPlatformNotificationService() override { + return platform_notification_service_; + } + + private: + MockPlatformNotificationService* platform_notification_service_; +}; + +class BlinkNotificationServiceImplTest : public ::testing::Test { + public: + // Using REAL_IO_THREAD would give better coverage for thread safety, but + // at time of writing EmbeddedWorkerTestHelper didn't seem to support that. + BlinkNotificationServiceImplTest() + : thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP), + notification_browser_client_(&mock_platform_service_) { + SetBrowserClientForTesting(¬ification_browser_client_); + } + + ~BlinkNotificationServiceImplTest() override = default; + + // ::testing::Test overrides. + void SetUp() override { + notification_context_ = new PlatformNotificationContextImpl( + base::FilePath(), &browser_context_, + 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_, + kFakeRenderProcessId, url::Origin::Create(GURL(kTestOrigin)), + mojo::MakeRequest(¬ification_service_ptr)); + } + + void DidGetPermissionStatus( + blink::mojom::PermissionStatus permission_status) { + permission_callback_result_ = permission_status; + } + + blink::mojom::PermissionStatus GetPermissionCallbackResult() { + return permission_callback_result_; + } + + void DidDisplayPersistentNotification( + base::OnceClosure quit_closure, + blink::mojom::PersistentNotificationError error) { + display_persistent_callback_result_ = error; + std::move(quit_closure).Run(); + } + + void DidGetDisplayedNotifications( + base::OnceClosure quit_closure, + std::unique_ptr<std::set<std::string>> notification_ids, + bool supports_synchronization) { + get_displayed_callback_result_ = *notification_ids; + 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() { + base::RunLoop run_loop; + mock_platform_service_.GetDisplayedNotifications( + &browser_context_, + base::Bind( + &BlinkNotificationServiceImplTest::DidGetDisplayedNotifications, + base::Unretained(this), run_loop.QuitClosure())); + run_loop.Run(); + return get_displayed_callback_result_; + } + + protected: + TestBrowserThreadBundle thread_bundle_; // Must be first member. + + std::unique_ptr<BlinkNotificationServiceImpl> notification_service_; + + TestBrowserContext browser_context_; + + MockPlatformNotificationService mock_platform_service_; + + MockNonPersistentNotificationListener non_persistent_notification_listener_; + + blink::mojom::PersistentNotificationError display_persistent_callback_result_; + + private: + NotificationBrowserClient notification_browser_client_; + + scoped_refptr<PlatformNotificationContextImpl> notification_context_; + + blink::mojom::PermissionStatus permission_callback_result_ = + blink::mojom::PermissionStatus::ASK; + + std::set<std::string> get_displayed_callback_result_; + + MockResourceContext resource_context_; + + DISALLOW_COPY_AND_ASSIGN(BlinkNotificationServiceImplTest); +}; + +TEST_F(BlinkNotificationServiceImplTest, GetPermissionStatus) { + mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::GRANTED); + + notification_service_->GetPermissionStatus( + base::BindOnce(&BlinkNotificationServiceImplTest::DidGetPermissionStatus, + base::Unretained(this))); + + EXPECT_EQ(blink::mojom::PermissionStatus::GRANTED, + GetPermissionCallbackResult()); + + mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::DENIED); + + notification_service_->GetPermissionStatus( + base::BindOnce(&BlinkNotificationServiceImplTest::DidGetPermissionStatus, + base::Unretained(this))); + + EXPECT_EQ(blink::mojom::PermissionStatus::DENIED, + GetPermissionCallbackResult()); + + mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::ASK); + notification_service_->GetPermissionStatus( + base::BindOnce(&BlinkNotificationServiceImplTest::DidGetPermissionStatus, + base::Unretained(this))); + + EXPECT_EQ(blink::mojom::PermissionStatus::ASK, GetPermissionCallbackResult()); +} + +TEST_F(BlinkNotificationServiceImplTest, + DisplayNonPersistentNotificationWithPermission) { + mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::GRANTED); + + notification_service_->DisplayNonPersistentNotification( + "token", PlatformNotificationData(), NotificationResources(), + non_persistent_notification_listener_.GetPtr()); + + // TODO(https://crbug.com/787459): Pass a callback to + // DisplayNonPersistentNotification instead of waiting for all tasks to run + // here; a callback parameter will be needed anyway to enable + // non-persistent notification event acknowledgements - see bug. + RunAllTasksUntilIdle(); + + EXPECT_EQ(1u, GetDisplayedNotifications().size()); +} + +TEST_F(BlinkNotificationServiceImplTest, + DisplayNonPersistentNotificationWithoutPermission) { + mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::DENIED); + + notification_service_->DisplayNonPersistentNotification( + "token", PlatformNotificationData(), NotificationResources(), + non_persistent_notification_listener_.GetPtr()); + + // TODO(https://crbug.com/787459): Pass a callback to + // DisplayNonPersistentNotification instead of waiting for all tasks to run + // here; a callback parameter will be needed anyway to enable + // non-persistent notification event acknowledgements - see bug. + RunAllTasksUntilIdle(); + + EXPECT_EQ(0u, GetDisplayedNotifications().size()); +} + +TEST_F(BlinkNotificationServiceImplTest, + DisplayPersistentNotificationWithPermission) { + mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::GRANTED); + + DisplayPersistentNotificationSync(); + + EXPECT_EQ(blink::mojom::PersistentNotificationError::NONE, + display_persistent_callback_result_); + + // Wait for service to receive the Display call. + RunAllTasksUntilIdle(); + + EXPECT_EQ(1u, GetDisplayedNotifications().size()); +} + +TEST_F(BlinkNotificationServiceImplTest, + DisplayPersistentNotificationWithoutPermission) { + mock_platform_service_.SetPermission(blink::mojom::PermissionStatus::DENIED); + + DisplayPersistentNotificationSync(); + + EXPECT_EQ(blink::mojom::PersistentNotificationError::PERMISSION_DENIED, + display_persistent_callback_result_); + + // Give Service a chance to receive any unexpected Display calls. + RunAllTasksUntilIdle(); + + 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 diff --git a/chromium/content/browser/notifications/notification_database.cc b/chromium/content/browser/notifications/notification_database.cc index e51a8e4894e..8f212af7f83 100644 --- a/chromium/content/browser/notifications/notification_database.cc +++ b/chromium/content/browser/notifications/notification_database.cc @@ -14,7 +14,7 @@ #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_database_data.h" #include "storage/common/database/database_identifier.h" -#include "third_party/WebKit/common/service_worker/service_worker_registration.mojom.h" +#include "third_party/WebKit/public/mojom/service_worker/service_worker_registration.mojom.h" #include "third_party/leveldatabase/env_chromium.h" #include "third_party/leveldatabase/leveldb_chrome.h" #include "third_party/leveldatabase/src/include/leveldb/db.h" diff --git a/chromium/content/browser/notifications/notification_event_dispatcher_impl.cc b/chromium/content/browser/notifications/notification_event_dispatcher_impl.cc index db1c4cb94e3..3b593eb6c0b 100644 --- a/chromium/content/browser/notifications/notification_event_dispatcher_impl.cc +++ b/chromium/content/browser/notifications/notification_event_dispatcher_impl.cc @@ -10,7 +10,6 @@ #include "build/build_config.h" #include "content/browser/notifications/notification_message_filter.h" #include "content/browser/notifications/platform_notification_context_impl.h" -#include "content/browser/renderer_host/render_process_host_impl.h" #include "content/browser/service_worker/service_worker_context_wrapper.h" #include "content/browser/service_worker/service_worker_registration.h" #include "content/browser/service_worker/service_worker_storage.h" @@ -178,9 +177,9 @@ void FindServiceWorkerRegistration( service_worker_context->FindReadyRegistrationForId( notification_database_data.service_worker_registration_id, origin, - base::Bind(&DispatchNotificationEventOnRegistration, - notification_database_data, notification_context, - notification_action_callback, dispatch_error_callback)); + base::BindOnce(&DispatchNotificationEventOnRegistration, + notification_database_data, notification_context, + notification_action_callback, dispatch_error_callback)); } // Reads the data associated with the |notification_id| belonging to |origin| @@ -413,88 +412,61 @@ void NotificationEventDispatcherImpl::DispatchNotificationCloseEvent( repeating_callback /* notification_error_callback */); } -void NotificationEventDispatcherImpl::RegisterNonPersistentNotification( +void NotificationEventDispatcherImpl::RegisterNonPersistentNotificationListener( const std::string& notification_id, - int renderer_id, - int request_id) { - if (request_ids_.count(notification_id) && - request_ids_[notification_id] != request_id) { - // Notify close for a previously displayed notification with the same - // request id, this can happen when replacing a non-persistent notification - // with the same tag since from the JS point of view there will be two - // notification objects and the old one needs to receive the close event. - // TODO(miguelg) this is probably not the right layer to do this. + blink::mojom::NonPersistentNotificationListenerPtrInfo listener_ptr_info) { + if (non_persistent_notification_listeners_.count(notification_id)) { + // Dispatch the close event for any previously displayed notification with + // the same notification id. This happens whenever a non-persistent + // notification is replaced (by creating another with the same tag), since + // from the JavaScript point of view there will be two notification objects, + // and the old one needs to receive a close event before the new one + // receives a show event. DispatchNonPersistentCloseEvent(notification_id); } - renderer_ids_[notification_id] = renderer_id; - request_ids_[notification_id] = request_id; + + blink::mojom::NonPersistentNotificationListenerPtr listener_ptr( + std::move(listener_ptr_info)); + + // Observe connection errors, which occur when the JavaScript object or the + // renderer hosting them goes away. (For example through navigation.) The + // listener gets freed together with |this|, thus the Unretained is safe. + listener_ptr.set_connection_error_handler(base::BindOnce( + &NotificationEventDispatcherImpl:: + HandleConnectionErrorForNonPersistentNotificationListener, + base::Unretained(this), notification_id)); + + non_persistent_notification_listeners_.emplace(notification_id, + std::move(listener_ptr)); } void NotificationEventDispatcherImpl::DispatchNonPersistentShowEvent( const std::string& notification_id) { - if (!renderer_ids_.count(notification_id)) - return; - DCHECK(request_ids_.count(notification_id)); - - RenderProcessHost* sender = - RenderProcessHost::FromID(renderer_ids_[notification_id]); - if (!sender) + if (!non_persistent_notification_listeners_.count(notification_id)) return; - - sender->Send( - new PlatformNotificationMsg_DidShow(request_ids_[notification_id])); + non_persistent_notification_listeners_[notification_id]->OnShow(); } void NotificationEventDispatcherImpl::DispatchNonPersistentClickEvent( const std::string& notification_id) { - if (!renderer_ids_.count(notification_id)) + if (!non_persistent_notification_listeners_.count(notification_id)) return; - DCHECK(request_ids_.count(notification_id)); - - RenderProcessHost* sender = - RenderProcessHost::FromID(renderer_ids_[notification_id]); - - // This can happen when a notification is clicked by the user but the - // renderer does not exist any more, for example because the tab has been - // closed. - if (!sender) - return; - sender->Send( - new PlatformNotificationMsg_DidClick(request_ids_[notification_id])); + non_persistent_notification_listeners_[notification_id]->OnClick(); } void NotificationEventDispatcherImpl::DispatchNonPersistentCloseEvent( const std::string& notification_id) { - if (!renderer_ids_.count(notification_id)) - return; - DCHECK(request_ids_.count(notification_id)); - - RenderProcessHost* sender = - RenderProcessHost::FromID(renderer_ids_[notification_id]); - - // This can happen when a notification is closed by the user but the - // renderer does not exist any more, for example because the tab has been - // closed. - if (!sender) + if (!non_persistent_notification_listeners_.count(notification_id)) return; - - sender->Send( - new PlatformNotificationMsg_DidClose(request_ids_[notification_id])); - - // No interaction will follow anymore once the notification has been closed. - request_ids_.erase(notification_id); - renderer_ids_.erase(notification_id); + non_persistent_notification_listeners_[notification_id]->OnClose(); + non_persistent_notification_listeners_.erase(notification_id); } -void NotificationEventDispatcherImpl::RendererGone(int renderer_id) { - for (auto iter = renderer_ids_.begin(); iter != renderer_ids_.end();) { - if (iter->second == renderer_id) { - request_ids_.erase(iter->first); - iter = renderer_ids_.erase(iter); - } else { - iter++; - } - } +void NotificationEventDispatcherImpl:: + HandleConnectionErrorForNonPersistentNotificationListener( + const std::string& notification_id) { + DCHECK(non_persistent_notification_listeners_.count(notification_id)); + non_persistent_notification_listeners_.erase(notification_id); } } // namespace content diff --git a/chromium/content/browser/notifications/notification_event_dispatcher_impl.h b/chromium/content/browser/notifications/notification_event_dispatcher_impl.h index 7812a505ecf..80c3583b944 100644 --- a/chromium/content/browser/notifications/notification_event_dispatcher_impl.h +++ b/chromium/content/browser/notifications/notification_event_dispatcher_impl.h @@ -9,12 +9,15 @@ #include "base/macros.h" #include "base/memory/singleton.h" +#include "content/common/content_export.h" #include "content/public/browser/notification_database_data.h" #include "content/public/browser/notification_event_dispatcher.h" +#include "third_party/WebKit/public/platform/modules/notifications/notification_service.mojom.h" namespace content { -class NotificationEventDispatcherImpl : public NotificationEventDispatcher { +class CONTENT_EXPORT NotificationEventDispatcherImpl + : public NotificationEventDispatcher { public: // Returns the instance of the NotificationEventDispatcherImpl. Must be called // on the UI thread. @@ -41,26 +44,28 @@ class NotificationEventDispatcherImpl : public NotificationEventDispatcher { void DispatchNonPersistentCloseEvent( const std::string& notification_id) override; - // Called when a renderer that had shown a non persistent notification - // dissappears. - void RendererGone(int renderer_id); - - // Register the fact that a non persistent notification has been displayed. - void RegisterNonPersistentNotification(const std::string& notification_id, - int renderer_id, - int request_id); + // Registers |listener| to receive the show, click and close events of the + // non-persistent notification identified by |notification_id|. + void RegisterNonPersistentNotificationListener( + const std::string& notification_id, + blink::mojom::NonPersistentNotificationListenerPtrInfo listener_ptr_info); private: + friend class NotificationEventDispatcherImplTest; + friend struct base::DefaultSingletonTraits<NotificationEventDispatcherImpl>; + NotificationEventDispatcherImpl(); ~NotificationEventDispatcherImpl() override; - // Notification Id -> renderer Id. - std::map<std::string, int> renderer_ids_; - - // Notification Id -> request Id. - std::map<std::string, int> request_ids_; + // Removes all references to the listener registered to receive events + // from the non-persistent notification identified by |notification_id|. + // Should be called when the connection to this listener goes away. + void HandleConnectionErrorForNonPersistentNotificationListener( + const std::string& notification_id); - friend struct base::DefaultSingletonTraits<NotificationEventDispatcherImpl>; + // Notification Id -> listener. + std::map<std::string, blink::mojom::NonPersistentNotificationListenerPtr> + non_persistent_notification_listeners_; DISALLOW_COPY_AND_ASSIGN(NotificationEventDispatcherImpl); }; diff --git a/chromium/content/browser/notifications/notification_event_dispatcher_impl_unittest.cc b/chromium/content/browser/notifications/notification_event_dispatcher_impl_unittest.cc new file mode 100644 index 00000000000..2bf90ca13ec --- /dev/null +++ b/chromium/content/browser/notifications/notification_event_dispatcher_impl_unittest.cc @@ -0,0 +1,206 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/browser/notifications/notification_event_dispatcher_impl.h" + +#include <stdint.h> +#include <memory> +#include <vector> + +#include "base/macros.h" +#include "base/test/test_simple_task_runner.h" +#include "base/threading/thread_task_runner_handle.h" +#include "mojo/public/cpp/bindings/binding.h" +#include "mojo/public/cpp/bindings/interface_request.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/WebKit/public/platform/modules/notifications/notification_service.mojom.h" + +namespace content { + +namespace { + +const char kPrimaryUniqueId[] = "this_should_be_a_unique_id"; +const char kSomeOtherUniqueId[] = "and_this_one_is_different_and_also_unique"; + +class TestNotificationListener + : public blink::mojom::NonPersistentNotificationListener { + public: + TestNotificationListener() : binding_(this) {} + ~TestNotificationListener() override = default; + + // Closes the bindings associated with this listener. + void Close() { binding_.Close(); } + + // Returns an InterfacePtr to this listener. + blink::mojom::NonPersistentNotificationListenerPtr GetPtr() { + blink::mojom::NonPersistentNotificationListenerPtr ptr; + binding_.Bind(mojo::MakeRequest(&ptr)); + return ptr; + } + + // Returns the number of OnShow events received by this listener. + int on_show_count() const { return on_show_count_; } + + // Returns the number of OnClick events received by this listener. + int on_click_count() const { return on_click_count_; } + + // Returns the number of OnClose events received by this listener. + int on_close_count() const { return on_close_count_; } + + // blink::mojom::NonPersistentNotificationListener implementation. + void OnShow() override { on_show_count_++; } + void OnClick() override { on_click_count_++; } + void OnClose() override { on_close_count_++; } + + private: + int on_show_count_ = 0; + int on_click_count_ = 0; + int on_close_count_ = 0; + mojo::Binding<blink::mojom::NonPersistentNotificationListener> binding_; + + DISALLOW_COPY_AND_ASSIGN(TestNotificationListener); +}; + +} // anonymous namespace + +class NotificationEventDispatcherImplTest : public ::testing::Test { + public: + NotificationEventDispatcherImplTest() + : task_runner_(new base::TestSimpleTaskRunner), + handle_(task_runner_), + dispatcher_(new NotificationEventDispatcherImpl()) {} + + ~NotificationEventDispatcherImplTest() override { delete dispatcher_; } + + // Waits until the task runner managing the Mojo connection has finished. + void WaitForMojoTasksToComplete() { task_runner_->RunUntilIdle(); } + + protected: + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + base::ThreadTaskRunnerHandle handle_; + + // Using a raw pointer because NotificationEventDispatcherImpl is a singleton + // with private constructor and destructor, so unique_ptr is not an option. + NotificationEventDispatcherImpl* dispatcher_; + + private: + DISALLOW_COPY_AND_ASSIGN(NotificationEventDispatcherImplTest); +}; + +TEST_F(NotificationEventDispatcherImplTest, + DispatchNonPersistentShowEvent_NotifiesCorrectRegisteredListener) { + auto listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kPrimaryUniqueId, listener->GetPtr().PassInterface()); + auto other_listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kSomeOtherUniqueId, other_listener->GetPtr().PassInterface()); + + dispatcher_->DispatchNonPersistentShowEvent(kPrimaryUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_show_count(), 1); + EXPECT_EQ(other_listener->on_show_count(), 0); + + dispatcher_->DispatchNonPersistentShowEvent(kSomeOtherUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_show_count(), 1); + EXPECT_EQ(other_listener->on_show_count(), 1); +} + +TEST_F(NotificationEventDispatcherImplTest, + RegisterReplacementNonPersistentListener_FirstListenerGetsOnClose) { + auto original_listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kPrimaryUniqueId, original_listener->GetPtr().PassInterface()); + + dispatcher_->DispatchNonPersistentShowEvent(kPrimaryUniqueId); + + ASSERT_EQ(original_listener->on_close_count(), 0); + + auto replacement_listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kPrimaryUniqueId, replacement_listener->GetPtr().PassInterface()); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(original_listener->on_close_count(), 1); + EXPECT_EQ(replacement_listener->on_close_count(), 0); +} + +TEST_F(NotificationEventDispatcherImplTest, + DispatchNonPersistentClickEvent_NotifiesCorrectRegisteredListener) { + auto listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kPrimaryUniqueId, listener->GetPtr().PassInterface()); + auto other_listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kSomeOtherUniqueId, other_listener->GetPtr().PassInterface()); + + dispatcher_->DispatchNonPersistentClickEvent(kPrimaryUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_click_count(), 1); + EXPECT_EQ(other_listener->on_click_count(), 0); + + dispatcher_->DispatchNonPersistentClickEvent(kSomeOtherUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_click_count(), 1); + EXPECT_EQ(other_listener->on_click_count(), 1); +} + +TEST_F(NotificationEventDispatcherImplTest, + DispatchNonPersistentCloseEvent_NotifiesCorrectRegisteredListener) { + auto listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kPrimaryUniqueId, listener->GetPtr().PassInterface()); + auto other_listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kSomeOtherUniqueId, other_listener->GetPtr().PassInterface()); + + dispatcher_->DispatchNonPersistentCloseEvent(kPrimaryUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_close_count(), 1); + EXPECT_EQ(other_listener->on_close_count(), 0); + + dispatcher_->DispatchNonPersistentCloseEvent(kSomeOtherUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_close_count(), 1); + EXPECT_EQ(other_listener->on_close_count(), 1); +} + +TEST_F(NotificationEventDispatcherImplTest, + DispatchMultipleNonPersistentEvents_StopsNotifyingAfterClose) { + auto listener = std::make_unique<TestNotificationListener>(); + dispatcher_->RegisterNonPersistentNotificationListener( + kPrimaryUniqueId, listener->GetPtr().PassInterface()); + + dispatcher_->DispatchNonPersistentShowEvent(kPrimaryUniqueId); + dispatcher_->DispatchNonPersistentClickEvent(kPrimaryUniqueId); + dispatcher_->DispatchNonPersistentCloseEvent(kPrimaryUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_show_count(), 1); + EXPECT_EQ(listener->on_click_count(), 1); + EXPECT_EQ(listener->on_close_count(), 1); + + // Should not be counted as the notification was already closed. + dispatcher_->DispatchNonPersistentClickEvent(kPrimaryUniqueId); + + WaitForMojoTasksToComplete(); + + EXPECT_EQ(listener->on_click_count(), 1); +} +} // namespace content diff --git a/chromium/content/browser/notifications/notification_id_generator.cc b/chromium/content/browser/notifications/notification_id_generator.cc index f84f0ae68a0..99fa0034c4d 100644 --- a/chromium/content/browser/notifications/notification_id_generator.cc +++ b/chromium/content/browser/notifications/notification_id_generator.cc @@ -8,30 +8,31 @@ #include "base/logging.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "url/gurl.h" namespace content { namespace { -const char kNotificationTagSeparator[] = "#"; -const char kPersistentNotificationPrefix[] = "p"; -const char kNonPersistentNotificationPrefix[] = "n"; +const char kNotificationTagSeparator = '#'; +const char kPersistentNotificationPrefix = 'p'; +const char kNonPersistentNotificationPrefix = 'n'; } // namespace // static bool NotificationIdGenerator::IsPersistentNotification( const base::StringPiece& notification_id) { - return notification_id.starts_with( - std::string(kPersistentNotificationPrefix)); + return notification_id.length() > 0 && + notification_id.front() == kPersistentNotificationPrefix; } // static bool NotificationIdGenerator::IsNonPersistentNotification( const base::StringPiece& notification_id) { - return notification_id.starts_with( - std::string(kNonPersistentNotificationPrefix)); + return notification_id.length() > 0 && + notification_id.front() == kNonPersistentNotificationPrefix; } // Notification Id is of the following format: @@ -59,32 +60,15 @@ std::string NotificationIdGenerator::GenerateForPersistentNotification( } // Notification Id is of the following format: -// n#<origin>#[1<developer_tag>|0<render_process_id>#<request_id>] +// p#<origin>#<token> std::string NotificationIdGenerator::GenerateForNonPersistentNotification( - const GURL& origin, - const std::string& tag, - int request_id, - int render_process_id) const { - DCHECK(origin.is_valid()); - DCHECK_EQ(origin, origin.GetOrigin()); - - std::stringstream stream; - - stream << kNonPersistentNotificationPrefix << kNotificationTagSeparator; - stream << origin; - stream << kNotificationTagSeparator; - - stream << base::IntToString(!tag.empty()); - if (tag.empty()) { - stream << base::IntToString(render_process_id); - stream << kNotificationTagSeparator; - - stream << base::IntToString(request_id); - } else { - stream << tag; - } - - return stream.str(); + const url::Origin& origin, + const std::string& token) const { + DCHECK(!origin.unique()); + DCHECK(!token.empty()); + return base::StringPrintf( + "%c%c%s%c%s", kNonPersistentNotificationPrefix, kNotificationTagSeparator, + origin.Serialize().c_str(), kNotificationTagSeparator, token.c_str()); } } // namespace content diff --git a/chromium/content/browser/notifications/notification_id_generator.h b/chromium/content/browser/notifications/notification_id_generator.h index ecba5f58c09..8ccf902e46d 100644 --- a/chromium/content/browser/notifications/notification_id_generator.h +++ b/chromium/content/browser/notifications/notification_id_generator.h @@ -11,6 +11,7 @@ #include "base/macros.h" #include "base/strings/string_piece.h" #include "content/common/content_export.h" +#include "url/origin.h" class GURL; @@ -63,12 +64,16 @@ class CONTENT_EXPORT NotificationIdGenerator { int64_t persistent_notification_id) const; // Generates an id for a non-persistent notification given the notification's - // origin, tag and request id. The request id must've been created by the - // |render_process_id|. - std::string GenerateForNonPersistentNotification(const GURL& origin, - const std::string& tag, - int request_id, - int render_process_id) const; + // |origin| and |token|. + // + // |token| is what determines which notifications from the same origin receive + // the same notification ID and therefore which notifications will replace + // each other. (So different notifications with the same non-empty tag should + // have the same token, but notifications without tags should have unique + // tokens.) + std::string GenerateForNonPersistentNotification( + const url::Origin& origin, + const std::string& token) const; private: DISALLOW_COPY_AND_ASSIGN(NotificationIdGenerator); diff --git a/chromium/content/browser/notifications/notification_id_generator_unittest.cc b/chromium/content/browser/notifications/notification_id_generator_unittest.cc index 99918508027..5cd5460d5ff 100644 --- a/chromium/content/browser/notifications/notification_id_generator_unittest.cc +++ b/chromium/content/browser/notifications/notification_id_generator_unittest.cc @@ -9,170 +9,143 @@ #include "content/browser/notifications/notification_id_generator.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" +#include "url/origin.h" namespace content { namespace { -const int kRenderProcessId = 42; const int64_t kPersistentNotificationId = 430; -const int kNonPersistentNotificationId = 5400; const char kExampleTag[] = "example"; class NotificationIdGeneratorTest : public ::testing::Test { public: - NotificationIdGeneratorTest() : origin_("https://example.com") {} + NotificationIdGeneratorTest() + : origin_(url::Origin::Create(GURL("https://example.com"))) {} protected: - GURL origin_; + url::Origin origin_; NotificationIdGenerator generator_; }; // ----------------------------------------------------------------------------- -// Persistent and non-persistent notifications +// Persistent notifications // Two calls to the generator with exactly the same information should result // in exactly the same notification ids being generated. -TEST_F(NotificationIdGeneratorTest, DeterministicGeneration) { - // Persistent notifications. +TEST_F(NotificationIdGeneratorTest, GenerateForPersistent_IsDetermenistic) { EXPECT_EQ(generator_.GenerateForPersistentNotification( - origin_, kExampleTag, kPersistentNotificationId), + origin_.GetURL(), kExampleTag, kPersistentNotificationId), generator_.GenerateForPersistentNotification( - origin_, kExampleTag, kPersistentNotificationId)); + origin_.GetURL(), kExampleTag, kPersistentNotificationId)); EXPECT_EQ(generator_.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId), + origin_.GetURL(), "" /* tag */, kPersistentNotificationId), generator_.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId)); - - // Non-persistent notifications. - EXPECT_EQ( - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, - kRenderProcessId)); - - EXPECT_EQ(generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId, - kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId, - kRenderProcessId)); + origin_.GetURL(), "" /* tag */, kPersistentNotificationId)); } // The origin of the notification will impact the generated notification id. -TEST_F(NotificationIdGeneratorTest, DifferentOrigins) { - GURL different_origin("https://example2.com"); +TEST_F(NotificationIdGeneratorTest, GenerateForPersistent_DifferentOrigins) { + url::Origin different_origin( + url::Origin::Create(GURL("https://example2.com"))); - // Persistent notifications. - EXPECT_NE(generator_.GenerateForPersistentNotification( - origin_, kExampleTag, kPersistentNotificationId), - generator_.GenerateForPersistentNotification( - different_origin, kExampleTag, kPersistentNotificationId)); - - // Non-persistent notifications. EXPECT_NE( - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - different_origin, kExampleTag, kNonPersistentNotificationId, - kRenderProcessId)); + generator_.GenerateForPersistentNotification( + origin_.GetURL(), kExampleTag, kPersistentNotificationId), + generator_.GenerateForPersistentNotification( + different_origin.GetURL(), kExampleTag, kPersistentNotificationId)); } // The tag, when non-empty, will impact the generated notification id. -TEST_F(NotificationIdGeneratorTest, DifferentTags) { +TEST_F(NotificationIdGeneratorTest, GenerateForPersistent_DifferentTags) { const std::string& different_tag = std::string(kExampleTag) + "2"; - // Persistent notifications. EXPECT_NE(generator_.GenerateForPersistentNotification( - origin_, kExampleTag, kPersistentNotificationId), + origin_.GetURL(), kExampleTag, kPersistentNotificationId), generator_.GenerateForPersistentNotification( - origin_, different_tag, kPersistentNotificationId)); - - // Non-persistent notifications. - EXPECT_NE( - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, different_tag, kNonPersistentNotificationId, - kRenderProcessId)); + origin_.GetURL(), different_tag, kPersistentNotificationId)); } // The persistent or non-persistent notification id will impact the generated // notification id when the tag is empty. -TEST_F(NotificationIdGeneratorTest, DifferentIds) { - // Persistent notifications. +TEST_F(NotificationIdGeneratorTest, GenerateForPersistent_DifferentIds) { EXPECT_NE(generator_.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId), + origin_.GetURL(), "" /* tag */, kPersistentNotificationId), generator_.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId + 1)); - - // Non-persistent notifications. - EXPECT_NE(generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId, - kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId + 1, - kRenderProcessId)); - - // Non-persistent when a tag is being used. - EXPECT_EQ( - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, - kRenderProcessId + 1)); + origin_.GetURL(), "" /* tag */, kPersistentNotificationId + 1)); } // Using a numeric tag that could resemble a persistent notification id should // not be equal to a notification without a tag, but with that id. -TEST_F(NotificationIdGeneratorTest, NumericTagAmbiguity) { - // Persistent notifications. - EXPECT_NE(generator_.GenerateForPersistentNotification( - origin_, base::Int64ToString(kPersistentNotificationId), - kPersistentNotificationId), - generator_.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId)); - - // Non-persistent notifications. - EXPECT_NE(generator_.GenerateForNonPersistentNotification( - origin_, base::IntToString(kNonPersistentNotificationId), - kNonPersistentNotificationId, kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId, - kRenderProcessId)); +TEST_F(NotificationIdGeneratorTest, GenerateForPersistent_NumericTagAmbiguity) { + EXPECT_NE( + generator_.GenerateForPersistentNotification( + origin_.GetURL(), base::Int64ToString(kPersistentNotificationId), + kPersistentNotificationId), + generator_.GenerateForPersistentNotification( + origin_.GetURL(), "" /* tag */, kPersistentNotificationId)); } // Using port numbers and a tag which, when concatenated, could end up being // equal to each other if origins stop ending with slashes. -TEST_F(NotificationIdGeneratorTest, OriginPortAmbiguity) { +TEST_F(NotificationIdGeneratorTest, GenerateForPersistent_OriginPortAmbiguity) { GURL origin_805("https://example.com:805"); GURL origin_8051("https://example.com:8051"); - // Persistent notifications. EXPECT_NE(generator_.GenerateForPersistentNotification( origin_805, "17", kPersistentNotificationId), generator_.GenerateForPersistentNotification( origin_8051, "7", kPersistentNotificationId)); +} + +// ----------------------------------------------------------------------------- +// Non-persistent notifications + +TEST_F(NotificationIdGeneratorTest, GenerateForNonPersistent_IsDeterministic) { + EXPECT_EQ( + generator_.GenerateForNonPersistentNotification(origin_, "example-token"), + generator_.GenerateForNonPersistentNotification(origin_, + "example-token")); +} + +TEST_F(NotificationIdGeneratorTest, GenerateForNonPersistent_DifferentOrigins) { + url::Origin different_origin( + url::Origin::Create(GURL("https://example2.com"))); + + EXPECT_NE( + generator_.GenerateForNonPersistentNotification(origin_, "example-token"), + generator_.GenerateForNonPersistentNotification(different_origin, + "example-token")); +} - // Non-persistent notifications. +TEST_F(NotificationIdGeneratorTest, GenerateForNonPersistent_DifferentTokens) { EXPECT_NE( - generator_.GenerateForNonPersistentNotification( - origin_805, "17", kNonPersistentNotificationId, kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_8051, "7", kNonPersistentNotificationId, kRenderProcessId)); + generator_.GenerateForNonPersistentNotification(origin_, "example-token"), + generator_.GenerateForNonPersistentNotification(origin_, "other-token")); } +// Use port numbers and a token which, when concatenated, could end up being +// equal to each other if origins stop ending with slashes. +TEST_F(NotificationIdGeneratorTest, + GenerateForNonPersistent_OriginPortAmbiguity) { + auto origin_805(url::Origin::Create(GURL("https://example.com:805"))); + auto origin_8051(url::Origin::Create(GURL("https://example.com:8051"))); + + EXPECT_NE(generator_.GenerateForNonPersistentNotification(origin_805, "17"), + generator_.GenerateForNonPersistentNotification(origin_8051, "7")); +} + +// ----------------------------------------------------------------------------- +// Both persistent and non-persistent notifications. + // Verifies that the static Is(Non)PersistentNotification helpers can correctly // identify persistent and non-persistent notification IDs. TEST_F(NotificationIdGeneratorTest, DetectIdFormat) { std::string persistent_id = generator_.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId); + origin_.GetURL(), "" /* tag */, kPersistentNotificationId); std::string non_persistent_id = - generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId, - kRenderProcessId); + generator_.GenerateForNonPersistentNotification(origin_, "token"); EXPECT_TRUE(NotificationIdGenerator::IsPersistentNotification(persistent_id)); EXPECT_FALSE( @@ -184,61 +157,5 @@ TEST_F(NotificationIdGeneratorTest, DetectIdFormat) { NotificationIdGenerator::IsPersistentNotification(non_persistent_id)); } -// ----------------------------------------------------------------------------- -// Persistent notifications -// -// Tests covering the logic specific to persistent notifications. This kind of -// notification does not care about the renderer process that created them. - -TEST_F(NotificationIdGeneratorTest, PersistentDifferentRenderProcessIds) { - NotificationIdGenerator second_generator; - - EXPECT_EQ(generator_.GenerateForPersistentNotification( - origin_, kExampleTag, kPersistentNotificationId), - second_generator.GenerateForPersistentNotification( - origin_, kExampleTag, kPersistentNotificationId)); - - EXPECT_EQ(generator_.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId), - second_generator.GenerateForPersistentNotification( - origin_, "" /* tag */, kPersistentNotificationId)); -} - -// ----------------------------------------------------------------------------- -// Non-persistent notifications -// -// Tests covering the logic specific to non-persistent notifications. This kind -// of notification cares about the renderer process they were created by when -// the notification does not have a tag, since multiple renderers would restart -// the count for non-persistent notification ids. - -TEST_F(NotificationIdGeneratorTest, NonPersistentDifferentRenderProcessIds) { - EXPECT_EQ( - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, kExampleTag, kNonPersistentNotificationId, - kRenderProcessId + 1)); - - EXPECT_NE(generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId, - kRenderProcessId), - generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, kNonPersistentNotificationId, - kRenderProcessId + 1)); -} - -// Concatenation of the render process id and the non-persistent notification -// id should not result in the generation of a duplicated notification id. -TEST_F(NotificationIdGeneratorTest, NonPersistentRenderProcessIdAmbiguity) { - EXPECT_NE( - generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, 1337 /* non_persistent_notification_id */, - 5 /* render_process_id */), - generator_.GenerateForNonPersistentNotification( - origin_, "" /* tag */, 337 /* non_persistent_notification_id */, - 51 /* render_process_id */)); -} - } // namespace } // namespace content diff --git a/chromium/content/browser/notifications/notification_message_filter.cc b/chromium/content/browser/notifications/notification_message_filter.cc index 6d6115ceebb..83db315fd57 100644 --- a/chromium/content/browser/notifications/notification_message_filter.cc +++ b/chromium/content/browser/notifications/notification_message_filter.cc @@ -89,7 +89,6 @@ NotificationMessageFilter::NotificationMessageFilter( BrowserContext* browser_context) : BrowserMessageFilter(PlatformNotificationMsgStart), process_id_(process_id), - non_persistent__notification_shown_(false), notification_context_(notification_context), resource_context_(resource_context), service_worker_context_(service_worker_context), @@ -99,26 +98,16 @@ NotificationMessageFilter::NotificationMessageFilter( NotificationMessageFilter::~NotificationMessageFilter() = default; void NotificationMessageFilter::OnDestruct() const { - if (non_persistent__notification_shown_) { - NotificationEventDispatcherImpl* event_dispatcher = - NotificationEventDispatcherImpl::GetInstance(); - DCHECK(event_dispatcher); - event_dispatcher->RendererGone(process_id_); - } BrowserThread::DeleteOnIOThread::Destruct(this); } bool NotificationMessageFilter::OnMessageReceived(const IPC::Message& message) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(NotificationMessageFilter, message) - IPC_MESSAGE_HANDLER(PlatformNotificationHostMsg_Show, - OnShowPlatformNotification) IPC_MESSAGE_HANDLER(PlatformNotificationHostMsg_ShowPersistent, OnShowPersistentNotification) IPC_MESSAGE_HANDLER(PlatformNotificationHostMsg_GetNotifications, OnGetNotifications) - IPC_MESSAGE_HANDLER(PlatformNotificationHostMsg_Close, - OnClosePlatformNotification) IPC_MESSAGE_HANDLER(PlatformNotificationHostMsg_ClosePersistent, OnClosePersistentNotification) IPC_MESSAGE_UNHANDLED(handled = false) @@ -127,49 +116,6 @@ bool NotificationMessageFilter::OnMessageReceived(const IPC::Message& message) { return handled; } -void NotificationMessageFilter::OverrideThreadForMessage( - const IPC::Message& message, - BrowserThread::ID* thread) { - if (message.type() == PlatformNotificationHostMsg_Show::ID || - message.type() == PlatformNotificationHostMsg_Close::ID) - *thread = BrowserThread::UI; -} - -void NotificationMessageFilter::OnShowPlatformNotification( - int request_id, - const GURL& origin, - const PlatformNotificationData& notification_data, - const NotificationResources& notification_resources) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - if (!RenderProcessHost::FromID(process_id_)) - return; - - if (!ValidateNotificationResources(notification_resources)) { - bad_message::ReceivedBadMessage(this, bad_message::NMF_INVALID_ARGUMENT); - return; - } - - PlatformNotificationService* service = - GetContentClient()->browser()->GetPlatformNotificationService(); - DCHECK(service); - - if (!VerifyNotificationPermissionGranted(service, origin)) - return; - - std::string notification_id = - GetNotificationIdGenerator()->GenerateForNonPersistentNotification( - origin, notification_data.tag, request_id, process_id_); - NotificationEventDispatcherImpl* event_dispatcher = - NotificationEventDispatcherImpl::GetInstance(); - non_persistent__notification_shown_ = true; - event_dispatcher->RegisterNonPersistentNotification(notification_id, - process_id_, request_id); - - service->DisplayNotification(browser_context_, notification_id, origin, - SanitizeNotificationData(notification_data), - notification_resources); -} - void NotificationMessageFilter::OnShowPersistentNotification( int request_id, int64_t service_worker_registration_id, @@ -225,9 +171,10 @@ void NotificationMessageFilter::DidWritePersistentNotificationData( // Get the service worker scope. service_worker_context_->FindReadyRegistrationForId( service_worker_registration_id, origin, - base::Bind(&NotificationMessageFilter::DidFindServiceWorkerRegistration, - weak_factory_io_.GetWeakPtr(), request_id, origin, - notification_data, notification_resources, notification_id)); + base::BindOnce( + &NotificationMessageFilter::DidFindServiceWorkerRegistration, + weak_factory_io_.GetWeakPtr(), request_id, origin, notification_data, + notification_resources, notification_id)); } void NotificationMessageFilter::DidFindServiceWorkerRegistration( @@ -305,28 +252,6 @@ void NotificationMessageFilter::DidGetNotifications( request_id, persistent_notifications)); } -void NotificationMessageFilter::OnClosePlatformNotification( - const GURL& origin, - const std::string& tag, - int request_id) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - if (!RenderProcessHost::FromID(process_id_)) - return; - - std::string notification_id = - GetNotificationIdGenerator()->GenerateForNonPersistentNotification( - origin, tag, request_id, process_id_); - - PlatformNotificationService* service = - GetContentClient()->browser()->GetPlatformNotificationService(); - DCHECK(service); - - service->CloseNotification(browser_context_, notification_id); - - NotificationEventDispatcherImpl::GetInstance() - ->DispatchNonPersistentCloseEvent(notification_id); -} - void NotificationMessageFilter::OnClosePersistentNotification( const GURL& origin, const std::string& tag, diff --git a/chromium/content/browser/notifications/notification_message_filter.h b/chromium/content/browser/notifications/notification_message_filter.h index 7e704221709..3f28c07e22a 100644 --- a/chromium/content/browser/notifications/notification_message_filter.h +++ b/chromium/content/browser/notifications/notification_message_filter.h @@ -42,8 +42,6 @@ class NotificationMessageFilter : public BrowserMessageFilter { // BrowserMessageFilter implementation. Called on the UI thread. void OnDestruct() const override; bool OnMessageReceived(const IPC::Message& message) override; - void OverrideThreadForMessage(const IPC::Message& message, - content::BrowserThread::ID* thread) override; protected: ~NotificationMessageFilter() override; @@ -52,11 +50,6 @@ class NotificationMessageFilter : public BrowserMessageFilter { friend class base::DeleteHelper<NotificationMessageFilter>; friend class BrowserThread; - void OnShowPlatformNotification( - int request_id, - const GURL& origin, - const PlatformNotificationData& notification_data, - const NotificationResources& notification_resources); void OnShowPersistentNotification( int request_id, int64_t service_worker_registration_id, @@ -67,9 +60,6 @@ class NotificationMessageFilter : public BrowserMessageFilter { int64_t service_worker_registration_id, const GURL& origin, const std::string& filter_tag); - void OnClosePlatformNotification(const GURL& origin, - const std::string& tag, - int request_id); void OnClosePersistentNotification(const GURL& origin, const std::string& tag, const std::string& notification_id); @@ -128,7 +118,6 @@ class NotificationMessageFilter : public BrowserMessageFilter { NotificationIdGenerator* GetNotificationIdGenerator() const; int process_id_; - bool non_persistent__notification_shown_; scoped_refptr<PlatformNotificationContextImpl> notification_context_; ResourceContext* resource_context_; scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_; diff --git a/chromium/content/browser/notifications/platform_notification_context_impl.cc b/chromium/content/browser/notifications/platform_notification_context_impl.cc index 60d95148533..b37ec6983f2 100644 --- a/chromium/content/browser/notifications/platform_notification_context_impl.cc +++ b/chromium/content/browser/notifications/platform_notification_context_impl.cc @@ -18,8 +18,6 @@ #include "content/public/browser/notification_database_data.h" #include "content/public/browser/platform_notification_service.h" -using base::DoNothing; - namespace content { namespace { @@ -60,7 +58,7 @@ void PlatformNotificationContextImpl::Initialize() { BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, base::BindOnce(&PlatformNotificationContextImpl::InitializeOnIO, this, - base::Passed(&displayed_notifications), false)); + std::move(displayed_notifications), false)); return; } @@ -77,7 +75,7 @@ void PlatformNotificationContextImpl::DidGetNotificationsOnUI( BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, base::BindOnce(&PlatformNotificationContextImpl::InitializeOnIO, this, - base::Passed(&displayed_notifications), + std::move(displayed_notifications), supports_synchronization)); } @@ -132,7 +130,7 @@ void PlatformNotificationContextImpl::CreateService( base::BindOnce(&PlatformNotificationContextImpl::CreateServiceOnIO, this, render_process_id, origin, browser_context_->GetResourceContext(), - base::Passed(&request))); + std::move(request))); } void PlatformNotificationContextImpl::CreateServiceOnIO( @@ -212,7 +210,7 @@ void PlatformNotificationContextImpl:: &PlatformNotificationContextImpl:: SynchronizeDisplayedNotificationsForServiceWorkerRegistrationOnIO, this, origin, service_worker_registration_id, callback, - base::Passed(¬ification_ids), supports_synchronization)); + std::move(notification_ids), supports_synchronization)); } void PlatformNotificationContextImpl:: @@ -440,7 +438,7 @@ void PlatformNotificationContextImpl::OnRegistrationDeleted( base::Bind(&PlatformNotificationContextImpl:: DoDeleteNotificationsForServiceWorkerRegistration, this, pattern.GetOrigin(), registration_id), - base::Bind(&DoNothing)); + base::DoNothing()); } void PlatformNotificationContextImpl:: @@ -472,7 +470,7 @@ void PlatformNotificationContextImpl::OnStorageWiped() { base::Bind( base::IgnoreResult(&PlatformNotificationContextImpl::DestroyDatabase), this), - base::Bind(&DoNothing)); + base::DoNothing()); } void PlatformNotificationContextImpl::LazyInitialize( diff --git a/chromium/content/browser/notifications/platform_notification_context_unittest.cc b/chromium/content/browser/notifications/platform_notification_context_unittest.cc index 0e7d4071683..04b485fe389 100644 --- a/chromium/content/browser/notifications/platform_notification_context_unittest.cc +++ b/chromium/content/browser/notifications/platform_notification_context_unittest.cc @@ -21,7 +21,7 @@ #include "content/test/mock_platform_notification_service.h" #include "content/test/test_content_browser_client.h" #include "testing/gtest/include/gtest/gtest.h" -#include "third_party/WebKit/common/service_worker/service_worker_registration.mojom.h" +#include "third_party/WebKit/public/mojom/service_worker/service_worker_registration.mojom.h" #include "url/gurl.h" namespace content { |