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

Unified Diff: chrome/browser/push_messaging/push_messaging_service_impl.cc

Issue 1485743002: Push API: test that default notification is shown on Android when needed. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@PushMessagingInstrumentationTest
Patch Set: Address bauerb's comments. Created 5 years 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: chrome/browser/push_messaging/push_messaging_service_impl.cc
diff --git a/chrome/browser/push_messaging/push_messaging_service_impl.cc b/chrome/browser/push_messaging/push_messaging_service_impl.cc
index c26b3b516a26c702388a1f419c5111059c2671d2..2c50f9212ed98278f0b9fbf8111bb4f75fdbcb13 100644
--- a/chrome/browser/push_messaging/push_messaging_service_impl.cc
+++ b/chrome/browser/push_messaging/push_messaging_service_impl.cc
@@ -21,6 +21,7 @@
#include "chrome/browser/push_messaging/push_messaging_app_identifier.h"
#include "chrome/browser/push_messaging/push_messaging_constants.h"
#include "chrome/browser/push_messaging/push_messaging_service_factory.h"
+#include "chrome/browser/push_messaging/push_messaging_service_observer.h"
#include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
#include "chrome/browser/ui/chrome_pages.h"
#include "chrome/common/chrome_switches.h"
@@ -121,6 +122,7 @@ PushMessagingServiceImpl::PushMessagingServiceImpl(Profile* profile)
#if defined(ENABLE_NOTIFICATIONS)
notification_manager_(profile),
#endif
+ push_messaging_service_observer_(PushMessagingServiceObserver::Create()),
weak_factory_(this) {
DCHECK(profile);
HostContentSettingsMapFactory::GetForProfile(profile_)->AddObserver(this);
@@ -242,13 +244,12 @@ void PushMessagingServiceImpl::DeliverMessageCallback(
const gcm::IncomingMessage& message,
const base::Closure& message_handled_closure,
content::PushDeliveryStatus status) {
- // Remove a single in-flight delivery for |app_id|. This has to be done using
- // an iterator rather than by value, as the latter removes all entries.
- DCHECK(in_flight_message_deliveries_.find(app_id) !=
- in_flight_message_deliveries_.end());
+ DCHECK_GE(in_flight_message_deliveries_.count(app_id), 1u);
- in_flight_message_deliveries_.erase(
- in_flight_message_deliveries_.find(app_id));
+ // TODO(mvanouwerkerk): Use ScopedClosureRunner for this.
+ base::Closure completion_closure =
+ base::Bind(&PushMessagingServiceImpl::DidHandleMessage,
+ weak_factory_.GetWeakPtr(), app_id, message_handled_closure);
// TODO(mvanouwerkerk): Show a warning in the developer console of the
// Service Worker corresponding to app_id (and/or on an internals page).
@@ -261,35 +262,50 @@ void PushMessagingServiceImpl::DeliverMessageCallback(
case content::PUSH_DELIVERY_STATUS_SUCCESS:
case content::PUSH_DELIVERY_STATUS_EVENT_WAITUNTIL_REJECTED:
#if defined(ENABLE_NOTIFICATIONS)
- // Only enforce the user visible requirements after the entire queue of
- // incoming messages for |app_id| has been flushed.
- if (!in_flight_message_deliveries_.count(app_id)) {
+ // Only enforce the user visible requirements if this is currently running
+ // as the delivery callback for the last in-flight message.
+ if (in_flight_message_deliveries_.count(app_id) == 1) {
notification_manager_.EnforceUserVisibleOnlyRequirements(
requesting_origin, service_worker_registration_id,
- message_handled_closure);
+ completion_closure);
} else {
- message_handled_closure.Run();
+ completion_closure.Run();
}
#else
- message_handled_closure.Run();
+ completion_closure.Run();
#endif
break;
case content::PUSH_DELIVERY_STATUS_INVALID_MESSAGE:
case content::PUSH_DELIVERY_STATUS_SERVICE_WORKER_ERROR:
- message_handled_closure.Run();
+ completion_closure.Run();
break;
case content::PUSH_DELIVERY_STATUS_UNKNOWN_APP_ID:
case content::PUSH_DELIVERY_STATUS_PERMISSION_DENIED:
case content::PUSH_DELIVERY_STATUS_NO_SERVICE_WORKER:
- Unsubscribe(
- app_id, message.sender_id,
- base::Bind(&UnregisterCallbackToClosure, message_handled_closure));
+ Unsubscribe(app_id, message.sender_id,
+ base::Bind(&UnregisterCallbackToClosure, completion_closure));
break;
}
RecordDeliveryStatus(status);
}
+void PushMessagingServiceImpl::DidHandleMessage(
+ const std::string& app_id,
+ const base::Closure& message_handled_closure) {
+ auto in_flight_iterator = in_flight_message_deliveries_.find(app_id);
+ DCHECK(in_flight_iterator != in_flight_message_deliveries_.end());
+
+ // Remove a single in-flight delivery for |app_id|. This has to be done using
+ // an iterator rather than by value, as the latter removes all entries.
+ in_flight_message_deliveries_.erase(in_flight_iterator);
+
+ message_handled_closure.Run();
+
+ if (push_messaging_service_observer_)
+ push_messaging_service_observer_->OnMessageHandled();
+}
+
void PushMessagingServiceImpl::SetMessageCallbackForTesting(
const base::Closure& callback) {
message_callback_for_testing_ = callback;

Powered by Google App Engine
This is Rietveld 408576698