From f08addc1cd8b71b8e60c6b33838ff4d02c62a9a1 Mon Sep 17 00:00:00 2001 From: peter Date: Thu, 16 Apr 2015 10:37:43 -0700 Subject: [PATCH] Consistently handle persistent notification ids as int64_t This changes the renderer side of Chromium to only denote persistent notification ids as int64_ts rather than as strings. This CL is part of a three-sided patch: [1] https://codereview.chromium.org/1078783002/ [2] This CL. [3] https://codereview.chromium.org/1072873002/ BUG= Review URL: https://codereview.chromium.org/1071203002 Cr-Commit-Position: refs/heads/master@{#325469} --- .../notification_event_dispatcher_impl.cc | 8 +------ .../service_worker/service_worker_version.cc | 20 ++++++++--------- .../service_worker/service_worker_version.h | 2 +- .../child/notifications/notification_manager.cc | 25 +++++----------------- content/child/notifications/notification_manager.h | 2 +- content/common/platform_notification_messages.h | 2 +- .../service_worker/service_worker_messages.h | 2 +- .../service_worker_script_context.cc | 4 ++-- .../service_worker/service_worker_script_context.h | 2 +- 9 files changed, 22 insertions(+), 45 deletions(-) diff --git a/content/browser/notifications/notification_event_dispatcher_impl.cc b/content/browser/notifications/notification_event_dispatcher_impl.cc index 34fe581f7993..b1119616b36c 100644 --- a/content/browser/notifications/notification_event_dispatcher_impl.cc +++ b/content/browser/notifications/notification_event_dispatcher_impl.cc @@ -5,7 +5,6 @@ #include "content/browser/notifications/notification_event_dispatcher_impl.h" #include "base/callback.h" -#include "base/strings/string_number_conversions.h" #include "content/browser/notifications/platform_notification_context_impl.h" #include "content/browser/service_worker/service_worker_context_wrapper.h" #include "content/browser/service_worker/service_worker_registration.h" @@ -77,15 +76,10 @@ void DispatchNotificationClickEventOnRegistration( dispatch_complete_callback, service_worker_registration); - // TODO(peter): Pass the persistent notification id as an int64_t rather - // than as a string. This depends on the Blink API being updated. - std::string persistent_notification_id_string = - base::Int64ToString(notification_database_data.notification_id); - service_worker_registration->active_version()-> DispatchNotificationClickEvent( dispatch_event_callback, - persistent_notification_id_string, + notification_database_data.notification_id, notification_database_data.notification_data); return; } diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc index 9103b8328798..b83e16de35bd 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc @@ -643,26 +643,24 @@ void ServiceWorkerVersion::DispatchSyncEvent(const StatusCallback& callback) { void ServiceWorkerVersion::DispatchNotificationClickEvent( const StatusCallback& callback, - const std::string& notification_id, + int64_t persistent_notification_id, const PlatformNotificationData& notification_data) { DCHECK_EQ(ACTIVATED, status()) << status(); if (running_status() != RUNNING) { // Schedule calling this method after starting the worker. - StartWorker(base::Bind(&RunTaskAfterStartWorker, - weak_factory_.GetWeakPtr(), callback, - base::Bind(&self::DispatchNotificationClickEvent, - weak_factory_.GetWeakPtr(), - callback, notification_id, - notification_data))); + StartWorker(base::Bind( + &RunTaskAfterStartWorker, weak_factory_.GetWeakPtr(), callback, + base::Bind(&self::DispatchNotificationClickEvent, + weak_factory_.GetWeakPtr(), callback, + persistent_notification_id, notification_data))); return; } int request_id = AddRequest(callback, ¬ification_click_callbacks_, REQUEST_NOTIFICATION_CLICK); - ServiceWorkerStatusCode status = embedded_worker_->SendMessage( - ServiceWorkerMsg_NotificationClickEvent(request_id, - notification_id, - notification_data)); + ServiceWorkerStatusCode status = + embedded_worker_->SendMessage(ServiceWorkerMsg_NotificationClickEvent( + request_id, persistent_notification_id, notification_data)); if (status != SERVICE_WORKER_OK) { notification_click_callbacks_.Remove(request_id); RunSoon(base::Bind(callback, status)); diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h index a617d554b715..111867b3ffec 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h @@ -215,7 +215,7 @@ class CONTENT_EXPORT ServiceWorkerVersion // This must be called when the status() is ACTIVATED. void DispatchNotificationClickEvent( const StatusCallback& callback, - const std::string& notification_id, + int64_t persistent_notification_id, const PlatformNotificationData& notification_data); // Sends push event to the associated embedded worker and asynchronously calls diff --git a/content/child/notifications/notification_manager.cc b/content/child/notifications/notification_manager.cc index b442bafb2eaa..e39588e44abf 100644 --- a/content/child/notifications/notification_manager.cc +++ b/content/child/notifications/notification_manager.cc @@ -8,7 +8,6 @@ #include "base/lazy_instance.h" #include "base/metrics/histogram_macros.h" -#include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "base/thread_task_runner_handle.h" #include "base/threading/thread_local.h" @@ -93,7 +92,7 @@ void NotificationManager::showPersistent( blink::WebServiceWorkerRegistration* service_worker_registration, blink::WebNotificationShowCallbacks* callbacks) { DCHECK(service_worker_registration); - int64 service_worker_registration_id = + int64_t service_worker_registration_id = static_cast( service_worker_registration)->registration_id(); @@ -147,7 +146,7 @@ void NotificationManager::getNotifications( service_worker_registration); GURL origin = GURL(service_worker_registration_impl->scope()).GetOrigin(); - int64 service_worker_registration_id = + int64_t service_worker_registration_id = service_worker_registration_impl->registration_id(); // TODO(peter): GenerateNotificationId is more of a request id. Consider @@ -186,20 +185,7 @@ void NotificationManager::close(blink::WebNotificationDelegate* delegate) { void NotificationManager::closePersistent( const blink::WebSerializedOrigin& origin, - const blink::WebString& persistent_notification_id_string) { - // TODO(peter): Blink should store the persistent_notification_id as an - // int64_t instead of a string. The id, created by Chromium, is a decimal - // number that fits in an int64_t, so convert it until the API updates. - base::string16 string_value = persistent_notification_id_string; - - int64_t persistent_notification_id = 0; - if (!base::StringToInt64(string_value, - &persistent_notification_id)) { - NOTREACHED() << "Unable to close persistent notification; invalid id: " - << string_value; - return; - } - + int64_t persistent_notification_id) { thread_safe_sender_->Send(new PlatformNotificationHostMsg_ClosePersistent( GURL(origin.string()), persistent_notification_id)); @@ -301,8 +287,7 @@ void NotificationManager::OnDidGetNotifications( for (size_t i = 0; i < notification_infos.size(); ++i) { blink::WebPersistentNotificationInfo web_notification_info; - web_notification_info.persistentNotificationId = - blink::WebString::fromUTF8(notification_infos[i].first); + web_notification_info.persistentId = notification_infos[i].first; web_notification_info.data = ToWebNotificationData(notification_infos[i].second); @@ -334,7 +319,7 @@ void NotificationManager::DisplayPageNotification( void NotificationManager::DisplayPersistentNotification( const blink::WebSerializedOrigin& origin, const blink::WebNotificationData& notification_data, - int64 service_worker_registration_id, + int64_t service_worker_registration_id, scoped_ptr callbacks, const SkBitmap& icon) { // TODO(peter): GenerateNotificationId is more of a request id. Consider diff --git a/content/child/notifications/notification_manager.h b/content/child/notifications/notification_manager.h index 7624198583ca..21769b5adfff 100644 --- a/content/child/notifications/notification_manager.h +++ b/content/child/notifications/notification_manager.h @@ -56,7 +56,7 @@ class NotificationManager : public blink::WebNotificationManager, virtual void close(blink::WebNotificationDelegate* delegate); virtual void closePersistent( const blink::WebSerializedOrigin& origin, - const blink::WebString& persistent_notification_id); + int64_t persistent_notification_id); virtual void notifyDelegateDestroyed( blink::WebNotificationDelegate* delegate); virtual blink::WebNotificationPermission checkPermission( diff --git a/content/common/platform_notification_messages.h b/content/common/platform_notification_messages.h index 7ad1e6bfe61a..f21713fa282b 100644 --- a/content/common/platform_notification_messages.h +++ b/content/common/platform_notification_messages.h @@ -22,7 +22,7 @@ // Defines the pair of [persistent notification id] => [notification data] used // when getting the notifications for a given Service Worker registration. using PersistentNotificationInfo = - std::pair; + std::pair; #endif // CONTENT_COMMON_PLATFORM_NOTIFICATION_MESSAGES_H_ diff --git a/content/common/service_worker/service_worker_messages.h b/content/common/service_worker/service_worker_messages.h index 00eb170efc29..2c40c232758c 100644 --- a/content/common/service_worker/service_worker_messages.h +++ b/content/common/service_worker/service_worker_messages.h @@ -384,7 +384,7 @@ IPC_MESSAGE_CONTROL1(ServiceWorkerMsg_SyncEvent, int /* request_id */) IPC_MESSAGE_CONTROL3(ServiceWorkerMsg_NotificationClickEvent, int /* request_id */, - std::string /* notification_id */, + int64_t /* persistent_notification_id */, content::PlatformNotificationData /* notification_data */) IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_PushEvent, int /* request_id */, diff --git a/content/renderer/service_worker/service_worker_script_context.cc b/content/renderer/service_worker/service_worker_script_context.cc index 372052057edb..de32db8ddc38 100644 --- a/content/renderer/service_worker/service_worker_script_context.cc +++ b/content/renderer/service_worker/service_worker_script_context.cc @@ -365,14 +365,14 @@ void ServiceWorkerScriptContext::OnSyncEvent(int request_id) { void ServiceWorkerScriptContext::OnNotificationClickEvent( int request_id, - const std::string& notification_id, + int64_t persistent_notification_id, const PlatformNotificationData& notification_data) { TRACE_EVENT0("ServiceWorker", "ServiceWorkerScriptContext::OnNotificationClickEvent"); notification_click_start_timings_[request_id] = base::TimeTicks::Now(); proxy_->dispatchNotificationClickEvent( request_id, - blink::WebString::fromUTF8(notification_id), + persistent_notification_id, ToWebNotificationData(notification_data)); } diff --git a/content/renderer/service_worker/service_worker_script_context.h b/content/renderer/service_worker/service_worker_script_context.h index 2def78fe69c1..f0c82585d388 100644 --- a/content/renderer/service_worker/service_worker_script_context.h +++ b/content/renderer/service_worker/service_worker_script_context.h @@ -115,7 +115,7 @@ class ServiceWorkerScriptContext { void OnSyncEvent(int request_id); void OnNotificationClickEvent( int request_id, - const std::string& notification_id, + int64_t persistent_notification_id, const PlatformNotificationData& notification_data); void OnPushEvent(int request_id, const std::string& data); void OnGeofencingEvent(int request_id, -- 2.11.4.GIT