diff options
5 files changed, 71 insertions, 7 deletions
diff --git a/content/browser/push_messaging/push_messaging_message_filter.cc b/content/browser/push_messaging/push_messaging_message_filter.cc index 9c664ca6..0ac0ce5 100644 --- a/content/browser/push_messaging/push_messaging_message_filter.cc +++ b/content/browser/push_messaging/push_messaging_message_filter.cc @@ -11,7 +11,9 @@ #include "base/metrics/histogram.h" #include "base/strings/string_number_conversions.h" #include "content/browser/renderer_host/render_process_host_impl.h" +#include "content/browser/service_worker/service_worker_context_core.h" #include "content/browser/service_worker/service_worker_context_wrapper.h" +#include "content/browser/service_worker/service_worker_storage.h" #include "content/common/push_messaging_messages.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" @@ -23,11 +25,16 @@ namespace content { namespace { void RecordRegistrationStatus(PushRegistrationStatus status) { + // Called from both UI and IO threads. Slightly racy, but acceptable, see + // https://groups.google.com/a/chromium.org/d/msg/chromium-dev/FNzZRJtN2aw/Aw0CWAXJJ1kJ UMA_HISTOGRAM_ENUMERATION("PushMessaging.RegistrationStatus", status, PUSH_REGISTRATION_STATUS_LAST + 1); } +const char kPushRegistrationIdServiceWorkerKey[] = + "push_registration_id"; + } // namespace PushMessagingMessageFilter::RegisterData::RegisterData() @@ -47,6 +54,7 @@ PushMessagingMessageFilter::PushMessagingMessageFilter( render_process_id_(render_process_id), service_worker_context_(service_worker_context), service_(NULL), + weak_factory_io_to_io_(this), weak_factory_ui_to_ui_(this) { } @@ -240,14 +248,46 @@ void PushMessagingMessageFilter::DidRegister( PushRegistrationStatus status) { DCHECK_CURRENTLY_ON(BrowserThread::UI); if (status == PUSH_REGISTRATION_STATUS_SUCCESS) { - SendRegisterSuccess(data, push_registration_id); + GURL push_endpoint(service()->GetPushEndpoint()); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&PushMessagingMessageFilter::PersistRegistrationOnIO, + this, data, push_endpoint, push_registration_id)); } else { SendRegisterError(data, status); } } +void PushMessagingMessageFilter::PersistRegistrationOnIO( + const RegisterData& data, + const GURL& push_endpoint, + const std::string& push_registration_id) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + service_worker_context_->context()->storage()->StoreUserData( + data.service_worker_registration_id, + data.requesting_origin, + kPushRegistrationIdServiceWorkerKey, + push_registration_id, + base::Bind(&PushMessagingMessageFilter::DidPersistRegistrationOnIO, + weak_factory_io_to_io_.GetWeakPtr(), + data, push_endpoint, push_registration_id)); +} + +void PushMessagingMessageFilter::DidPersistRegistrationOnIO( + const RegisterData& data, + const GURL& push_endpoint, + const std::string& push_registration_id, + ServiceWorkerStatusCode service_worker_status) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + if (service_worker_status == SERVICE_WORKER_OK) + SendRegisterSuccess(data, push_endpoint, push_registration_id); + else + SendRegisterError(data, PUSH_REGISTRATION_STATUS_STORAGE_ERROR); +} + void PushMessagingMessageFilter::SendRegisterError( const RegisterData& data, PushRegistrationStatus status) { + // May be called from both IO and UI threads. if (data.FromDocument()) { Send(new PushMessagingMsg_RegisterFromDocumentError( data.render_frame_id, data.request_id, status)); @@ -259,8 +299,10 @@ void PushMessagingMessageFilter::SendRegisterError( } void PushMessagingMessageFilter::SendRegisterSuccess( - const RegisterData& data, const std::string& push_registration_id) { - GURL push_endpoint(service()->GetPushEndpoint()); + const RegisterData& data, + const GURL& push_endpoint, + const std::string& push_registration_id) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); if (data.FromDocument()) { Send(new PushMessagingMsg_RegisterFromDocumentSuccess( data.render_frame_id, diff --git a/content/browser/push_messaging/push_messaging_message_filter.h b/content/browser/push_messaging/push_messaging_message_filter.h index 85134e3..6414cc1 100644 --- a/content/browser/push_messaging/push_messaging_message_filter.h +++ b/content/browser/push_messaging/push_messaging_message_filter.h @@ -9,6 +9,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" +#include "content/common/service_worker/service_worker_status_code.h" #include "content/public/browser/browser_message_filter.h" #include "content/public/common/push_messaging_status.h" #include "url/gurl.h" @@ -77,9 +78,20 @@ class PushMessagingMessageFilter : public BrowserMessageFilter { const std::string& push_registration_id, PushRegistrationStatus status); + void PersistRegistrationOnIO(const RegisterData& data, + const GURL& push_endpoint, + const std::string& push_registration_id); + + void DidPersistRegistrationOnIO( + const RegisterData& data, + const GURL& push_endpoint, + const std::string& push_registration_id, + ServiceWorkerStatusCode service_worker_status); + void SendRegisterError(const RegisterData& data, PushRegistrationStatus status); void SendRegisterSuccess(const RegisterData& data, + const GURL& push_endpoint, const std::string& push_registration_id); // Returns a push messaging service. The embedder owns the service, and is @@ -93,9 +105,12 @@ class PushMessagingMessageFilter : public BrowserMessageFilter { // Owned by the content embedder's browsing context. PushMessagingService* service_; - // Should only be used for asynchronous calls to the PushMessagingService on - // the UI thread, which may have external dependencies that supersede the - // lifetime of this messaging filter. + // Should only be used for asynchronous calls on the IO thread with external + // dependencies that might outlive this class e.g. ServiceWorkerStorage. + base::WeakPtrFactory<PushMessagingMessageFilter> weak_factory_io_to_io_; + + // TODO(johnme): Remove this, it seems unsafe since this class could be + // destroyed on the IO thread while the callback runs on the UI thread. base::WeakPtrFactory<PushMessagingMessageFilter> weak_factory_ui_to_ui_; DISALLOW_COPY_AND_ASSIGN(PushMessagingMessageFilter); diff --git a/content/public/common/push_messaging_status.cc b/content/public/common/push_messaging_status.cc index d3b3d1b..53a0d92 100644 --- a/content/public/common/push_messaging_status.cc +++ b/content/public/common/push_messaging_status.cc @@ -30,6 +30,9 @@ const char* PushRegistrationStatusToString(PushRegistrationStatus status) { case PUSH_REGISTRATION_STATUS_NO_SENDER_ID: return "Registration failed - no sender id provided"; + + case PUSH_REGISTRATION_STATUS_STORAGE_ERROR: + return "Registration failed - storage error"; } NOTREACHED(); return ""; diff --git a/content/public/common/push_messaging_status.h b/content/public/common/push_messaging_status.h index 6ac9d96..ef5bf4f 100644 --- a/content/public/common/push_messaging_status.h +++ b/content/public/common/push_messaging_status.h @@ -31,13 +31,16 @@ enum PushRegistrationStatus { // Registration failed because no sender id was provided by the page. PUSH_REGISTRATION_STATUS_NO_SENDER_ID = 6, + // Registration succeeded, but we failed to persist it. + PUSH_REGISTRATION_STATUS_STORAGE_ERROR = 7, + // NOTE: Do not renumber these as that would confuse interpretation of // previously logged data. When making changes, also update the enum list // in tools/metrics/histograms/histograms.xml to keep it in sync, and // update PUSH_REGISTRATION_STATUS_LAST below. // Used for IPC message range checks. - PUSH_REGISTRATION_STATUS_LAST = PUSH_REGISTRATION_STATUS_NO_SENDER_ID + PUSH_REGISTRATION_STATUS_LAST = PUSH_REGISTRATION_STATUS_STORAGE_ERROR }; // Push message delivery success / error codes for internal use. diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 3f82040..84859fe 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -53746,6 +53746,7 @@ To add a new entry, add it with any value and run test to compute valid value. <int value="4" label="Permission denied"/> <int value="5" label="Push service error"/> <int value="6" label="No sender id provided"/> + <int value="7" label="Storage error"/> </enum> <enum name="QuicAddressMismatch" type="int"> |