Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1533)

Unified Diff: content/browser/push_messaging/push_messaging_manager.cc

Issue 2697793004: Push API: Validate storage before returning cached subscriptions (Closed)
Patch Set: Fix include Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/push_messaging/push_messaging_manager.cc
diff --git a/content/browser/push_messaging/push_messaging_manager.cc b/content/browser/push_messaging/push_messaging_manager.cc
index b2d354464870f29ebd7a291f537b59d765f1926b..212fc59086ae4b1d20d9be0140414d68daf8afff 100644
--- a/content/browser/push_messaging/push_messaging_manager.cc
+++ b/content/browser/push_messaging/push_messaging_manager.cc
@@ -49,37 +49,30 @@ const char kIncognitoPushUnsupportedMessage[] =
"feature-detect this, since incognito mode needs to be undetectable by "
"websites.";
-// These UMA methods are only called from IO thread, but it would be acceptable
-// (even though slightly racy) to call them from UI thread as well, see
+// These UMA methods are called from the IO and/or UI threads. Racey but ok, see
// https://groups.google.com/a/chromium.org/d/msg/chromium-dev/FNzZRJtN2aw/Aw0CWAXJJ1kJ
void RecordRegistrationStatus(PushRegistrationStatus status) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) ||
+ BrowserThread::CurrentlyOn(BrowserThread::UI));
UMA_HISTOGRAM_ENUMERATION("PushMessaging.RegistrationStatus", status,
PUSH_REGISTRATION_STATUS_LAST + 1);
}
-
void RecordUnregistrationStatus(PushUnregistrationStatus status) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
UMA_HISTOGRAM_ENUMERATION("PushMessaging.UnregistrationStatus", status,
PUSH_UNREGISTRATION_STATUS_LAST + 1);
}
-
void RecordGetRegistrationStatus(PushGetRegistrationStatus status) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) ||
+ BrowserThread::CurrentlyOn(BrowserThread::UI));
UMA_HISTOGRAM_ENUMERATION("PushMessaging.GetRegistrationStatus", status,
PUSH_GETREGISTRATION_STATUS_LAST + 1);
}
-// Curries the |success| and |p256dh| parameters over to |callback| and
-// posts a task to invoke |callback| on the IO thread.
-void ForwardEncryptionInfoToIOThreadProxy(
- const PushMessagingService::EncryptionInfoCallback& callback,
- bool success,
- const std::vector<uint8_t>& p256dh,
- const std::vector<uint8_t>& auth) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
- BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
- base::Bind(callback, success, p256dh, auth));
+void UnregisterCallbackToClosure(const base::Closure& closure,
+ PushUnregistrationStatus status) {
+ DCHECK(!closure.is_null());
+ closure.Run();
}
// Returns whether |sender_info| contains a valid application server key, that
@@ -128,6 +121,14 @@ class PushMessagingManager::Core {
// Public Register methods on UI thread --------------------------------------
+ // Callback called on UI thread.
+ void SubscribeDidGetInfoOnUI(const RegisterData& data,
+ const std::string& push_subscription_id,
+ const std::string& sender_id,
+ bool is_valid,
+ const std::vector<uint8_t>& p256dh,
+ const std::vector<uint8_t>& auth);
+
// Called via PostTask from IO thread.
void RegisterOnUI(const RegisterData& data);
@@ -139,6 +140,24 @@ class PushMessagingManager::Core {
const GURL& requesting_origin,
const std::string& sender_id);
+ // Public GetSubscription methods on UI thread -------------------------------
+
+ // Callback called on UI thread.
+ void GetSubscriptionDidGetInfoOnUI(const GetSubscriptionCallback& callback,
+ const GURL& origin,
+ int64_t service_worker_registration_id,
+ const GURL& endpoint,
+ const std::string& sender_info,
+ bool is_valid,
+ const std::vector<uint8_t>& p256dh,
+ const std::vector<uint8_t>& auth);
+
+ // Callback called on UI thread.
+ void GetSubscriptionDidUnsubscribe(
+ const GetSubscriptionCallback& callback,
+ PushGetRegistrationStatus get_status,
+ PushUnregistrationStatus unsubscribe_status);
+
// Public GetPermission methods on UI thread ---------------------------------
// Called via PostTask from IO thread.
@@ -148,13 +167,13 @@ class PushMessagingManager::Core {
// Public helper methods on UI thread ----------------------------------------
- // Called via PostTask from IO thread. The |io_thread_callback| callback
- // will be invoked on the IO thread.
- void GetEncryptionInfoOnUI(
+ // Called via PostTask from IO thread. |callback| will be run on UI thread.
+ void GetSubscriptionInfoOnUI(
const GURL& origin,
int64_t service_worker_registration_id,
const std::string& sender_id,
- const PushMessagingService::EncryptionInfoCallback& io_thread_callback);
+ const std::string& push_subscription_id,
+ const PushMessagingService::SubscriptionInfoCallback& callback);
// Called (directly) from both the UI and IO threads.
bool is_incognito() const { return is_incognito_; }
@@ -162,6 +181,10 @@ class PushMessagingManager::Core {
// Returns a push messaging service. May return null.
PushMessagingService* service();
+ // Returns a weak ptr. Must only be called on the UI thread (and hence can
+ // only be called from the outer class's constructor).
+ base::WeakPtr<Core> GetWeakPtrFromIOParentConstructor();
+
private:
friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
friend class base::DeleteHelper<Core>;
@@ -231,6 +254,7 @@ PushMessagingManager::PushMessagingManager(
// constructor finishes.
ui_core_.reset(
new Core(weak_factory_io_to_io_.GetWeakPtr(), render_process_id));
+ ui_core_weak_ptr_ = ui_core_->GetWeakPtrFromIOParentConstructor();
PushMessagingService* service = ui_core_->service();
service_available_ = !!service;
@@ -293,7 +317,7 @@ void PushMessagingManager::DidCheckForExistingRegistration(
DCHECK_CURRENTLY_ON(BrowserThread::IO);
if (service_worker_status == SERVICE_WORKER_OK) {
DCHECK_EQ(2u, push_registration_id_and_sender_id.size());
- const auto& push_registration_id = push_registration_id_and_sender_id[0];
+ const auto& push_subscription_id = push_registration_id_and_sender_id[0];
const auto& stored_sender_id = push_registration_id_and_sender_id[1];
std::string fixed_sender_id =
FixSenderInfo(data.options.sender_info, stored_sender_id);
@@ -305,15 +329,14 @@ void PushMessagingManager::DidCheckForExistingRegistration(
SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_SENDER_ID_MISMATCH);
return;
}
- auto callback = base::Bind(&PushMessagingManager::DidGetEncryptionKeys,
- weak_factory_io_to_io_.GetWeakPtr(), data,
- push_registration_id);
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
- base::Bind(&Core::GetEncryptionInfoOnUI,
+ base::Bind(&Core::GetSubscriptionInfoOnUI,
base::Unretained(ui_core_.get()), data.requesting_origin,
data.service_worker_registration_id, fixed_sender_id,
- callback));
+ push_subscription_id,
+ base::Bind(&Core::SubscribeDidGetInfoOnUI, ui_core_weak_ptr_,
+ data, push_subscription_id, fixed_sender_id)));
return;
}
// TODO(johnme): The spec allows the register algorithm to reject with an
@@ -335,21 +358,53 @@ void PushMessagingManager::DidCheckForExistingRegistration(
}
}
-void PushMessagingManager::DidGetEncryptionKeys(
+void PushMessagingManager::Core::SubscribeDidGetInfoOnUI(
const RegisterData& data,
- const std::string& push_registration_id,
- bool success,
+ const std::string& push_subscription_id,
+ const std::string& sender_id,
+ bool is_valid,
const std::vector<uint8_t>& p256dh,
const std::vector<uint8_t>& auth) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
- if (!success) {
- SendSubscriptionError(data,
- PUSH_REGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE);
- return;
- }
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ if (is_valid) {
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&PushMessagingManager::SendSubscriptionSuccess, io_parent_,
+ data, PUSH_REGISTRATION_STATUS_SUCCESS_FROM_CACHE,
+ push_subscription_id, p256dh, auth));
+ } else {
+ PushMessagingService* push_service = service();
+ if (!push_service) {
+ NOTREACHED() << "Shouldn't be possible to have a stored push "
+ "subscription in a profile with no push service.";
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&PushMessagingManager::SendSubscriptionError, io_parent_,
+ data, PUSH_REGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE));
+ return;
+ }
- SendSubscriptionSuccess(data, PUSH_REGISTRATION_STATUS_SUCCESS_FROM_CACHE,
- push_registration_id, p256dh, auth);
+ // Uh-oh! Although there was a cached subscription in the Service Worker
+ // database, it did not have matching counterparts in the
+ // PushMessagingAppIdentifier map and/or GCM Store. Unsubscribe and
+ // re-subscribe to fix this inconsistency.
+
+ // Consider this subscription attempt to have failed. The re-subscribe will
+ // be logged to UMA as a separate subscription attempt.
+ RecordRegistrationStatus(PUSH_REGISTRATION_STATUS_STORAGE_CORRUPT);
+
+ auto try_again_on_io = base::Bind(
+ &PushMessagingManager::DidCheckForExistingRegistration, io_parent_,
+ data,
+ std::vector<std::string>() /* push_registration_id_and_sender_id */,
+ SERVICE_WORKER_ERROR_NOT_FOUND);
+ push_service->Unsubscribe(
+ PUSH_UNREGISTRATION_REASON_SUBSCRIBE_STORAGE_CORRUPT,
+ data.requesting_origin, data.service_worker_registration_id, sender_id,
+ base::Bind(&UnregisterCallbackToClosure,
+ base::Bind(IgnoreResult(&BrowserThread::PostTask),
+ BrowserThread::IO, FROM_HERE, try_again_on_io)));
+ }
}
void PushMessagingManager::DidGetSenderIdFromStorage(
@@ -522,7 +577,6 @@ void PushMessagingManager::DidPersistRegistrationOnIO(
void PushMessagingManager::SendSubscriptionError(
const RegisterData& data,
PushRegistrationStatus status) {
- // Only called from IO thread, but would be safe to call from UI thread.
DCHECK_CURRENTLY_ON(BrowserThread::IO);
data.callback.Run(status, base::nullopt /* endpoint */,
base::nullopt /* options */, base::nullopt /* p256dh */,
@@ -536,7 +590,6 @@ void PushMessagingManager::SendSubscriptionSuccess(
const std::string& push_subscription_id,
const std::vector<uint8_t>& p256dh,
const std::vector<uint8_t>& auth) {
- // Only called from IO thread, but would be safe to call from UI thread.
DCHECK_CURRENTLY_ON(BrowserThread::IO);
if (!service_available_) {
// This shouldn't be possible in incognito mode, since we've already checked
@@ -616,7 +669,8 @@ void PushMessagingManager::Core::UnregisterFromService(
}
push_service->Unsubscribe(
- requesting_origin, service_worker_registration_id, sender_id,
+ PUSH_UNREGISTRATION_REASON_JAVASCRIPT_API, requesting_origin,
+ service_worker_registration_id, sender_id,
base::Bind(&Core::DidUnregisterFromService,
weak_factory_ui_to_ui_.GetWeakPtr(), callback,
service_worker_registration_id));
@@ -694,6 +748,9 @@ void PushMessagingManager::DidGetSubscription(
switch (service_worker_status) {
case SERVICE_WORKER_OK: {
DCHECK_EQ(2u, push_subscription_id_and_sender_info.size());
+ const std::string& push_subscription_id =
+ push_subscription_id_and_sender_info[0];
+ const std::string& sender_info = push_subscription_id_and_sender_info[1];
if (!service_available_) {
// Return not found in incognito mode, so websites can't detect it.
@@ -709,22 +766,20 @@ void PushMessagingManager::DidGetSubscription(
service_worker_registration_id);
const GURL origin = registration->pattern().GetOrigin();
- const bool uses_standard_protocol =
- IsApplicationServerKey(push_subscription_id_and_sender_info[1]);
- const GURL endpoint = CreateEndpoint(
- uses_standard_protocol, push_subscription_id_and_sender_info[0]);
-
- auto callback_ui =
- base::Bind(&PushMessagingManager::DidGetSubscriptionKeys,
- weak_factory_io_to_io_.GetWeakPtr(), callback, endpoint,
- push_subscription_id_and_sender_info[1]);
+ const bool uses_standard_protocol = IsApplicationServerKey(sender_info);
+ const GURL endpoint =
+ CreateEndpoint(uses_standard_protocol, push_subscription_id);
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
- base::Bind(&Core::GetEncryptionInfoOnUI,
+ base::Bind(&Core::GetSubscriptionInfoOnUI,
base::Unretained(ui_core_.get()), origin,
- service_worker_registration_id,
- push_subscription_id_and_sender_info[1], callback_ui));
+ service_worker_registration_id, sender_info,
+ push_subscription_id,
+ base::Bind(&Core::GetSubscriptionDidGetInfoOnUI,
+ ui_core_weak_ptr_, callback, origin,
+ service_worker_registration_id, endpoint,
+ sender_info)));
return;
}
@@ -765,37 +820,62 @@ void PushMessagingManager::DidGetSubscription(
RecordGetRegistrationStatus(get_status);
}
-void PushMessagingManager::DidGetSubscriptionKeys(
+void PushMessagingManager::Core::GetSubscriptionDidGetInfoOnUI(
const GetSubscriptionCallback& callback,
+ const GURL& origin,
+ int64_t service_worker_registration_id,
const GURL& endpoint,
const std::string& sender_info,
- bool success,
+ bool is_valid,
const std::vector<uint8_t>& p256dh,
const std::vector<uint8_t>& auth) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
- if (!success) {
- PushGetRegistrationStatus status =
- PUSH_GETREGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE;
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ if (is_valid) {
+ PushSubscriptionOptions options;
+ // Chrome rejects subscription requests with userVisibleOnly false, so it
+ // must have been true. TODO(harkness): If Chrome starts accepting silent
+ // push subscriptions with userVisibleOnly false, the bool will need to be
+ // stored.
+ options.user_visible_only = true;
+ options.sender_info = sender_info;
- callback.Run(status, base::nullopt /* endpoint */,
- base::nullopt /* options */, base::nullopt /* p256dh */,
- base::nullopt /* auth */);
+ PushGetRegistrationStatus status = PUSH_GETREGISTRATION_STATUS_SUCCESS;
+
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(callback, status, endpoint, options, p256dh, auth));
RecordGetRegistrationStatus(status);
- return;
- }
+ } else {
+ // Uh-oh! Although there was a cached subscription in the Service Worker
+ // database, it did not have matching counterparts in the
+ // PushMessagingAppIdentifier map and/or GCM Store. Unsubscribe to fix this
+ // inconsistency.
+ PushGetRegistrationStatus status =
+ PUSH_GETREGISTRATION_STATUS_STORAGE_CORRUPT;
- PushSubscriptionOptions options;
- // Chrome rejects subscription requests with userVisibleOnly false, so it must
- // have been true. TODO(harkness): If Chrome starts accepting silent push
- // subscriptions with userVisibleOnly false, the bool will need to be stored.
- options.user_visible_only = true;
- options.sender_info = sender_info;
+ PushMessagingService* push_service = service();
+ DCHECK(push_service); // NOT_FOUND response can only come from service.
+ push_service->Unsubscribe(
+ PUSH_UNREGISTRATION_REASON_GET_SUBSCRIPTION_STORAGE_CORRUPT, origin,
+ service_worker_registration_id, sender_info,
+ base::Bind(&Core::GetSubscriptionDidUnsubscribe,
+ weak_factory_ui_to_ui_.GetWeakPtr(), callback, status));
- callback.Run(PUSH_GETREGISTRATION_STATUS_SUCCESS, endpoint, options, p256dh,
- auth);
+ RecordGetRegistrationStatus(status);
+ }
+}
- RecordGetRegistrationStatus(PUSH_GETREGISTRATION_STATUS_SUCCESS);
+void PushMessagingManager::Core::GetSubscriptionDidUnsubscribe(
+ const GetSubscriptionCallback& callback,
+ PushGetRegistrationStatus get_status,
+ PushUnregistrationStatus unsubscribe_status) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(callback, get_status, base::nullopt /* endpoint */,
+ base::nullopt /* options */, base::nullopt /* p256dh */,
+ base::nullopt /* auth */));
}
// GetPermission methods on both IO and UI threads, merged in order of use from
@@ -864,24 +944,22 @@ void PushMessagingManager::Core::GetPermissionStatusOnUI(
// PushMessagingManager and Core.
// -----------------------------------------------------------------------------
-void PushMessagingManager::Core::GetEncryptionInfoOnUI(
+void PushMessagingManager::Core::GetSubscriptionInfoOnUI(
const GURL& origin,
int64_t service_worker_registration_id,
const std::string& sender_id,
- const PushMessagingService::EncryptionInfoCallback& io_thread_callback) {
+ const std::string& push_subscription_id,
+ const PushMessagingService::SubscriptionInfoCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
PushMessagingService* push_service = service();
- if (push_service) {
- push_service->GetEncryptionInfo(
- origin, service_worker_registration_id, sender_id,
- base::Bind(&ForwardEncryptionInfoToIOThreadProxy, io_thread_callback));
+ if (!push_service) {
+ callback.Run(false /* is_valid */, std::vector<uint8_t>() /* p256dh */,
+ std::vector<uint8_t>() /* auth */);
return;
}
- BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
- base::Bind(io_thread_callback, false /* success */,
- std::vector<uint8_t>() /* p256dh */,
- std::vector<uint8_t>() /* auth */));
+ push_service->GetSubscriptionInfo(origin, service_worker_registration_id,
+ sender_id, push_subscription_id, callback);
}
GURL PushMessagingManager::CreateEndpoint(
@@ -902,4 +980,10 @@ PushMessagingService* PushMessagingManager::Core::service() {
: nullptr;
}
+base::WeakPtr<PushMessagingManager::Core>
+PushMessagingManager::Core::GetWeakPtrFromIOParentConstructor() {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ return weak_factory_ui_to_ui_.GetWeakPtr();
+}
+
} // namespace content
« no previous file with comments | « content/browser/push_messaging/push_messaging_manager.h ('k') | content/child/push_messaging/push_provider.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698