Hook up the Push API with GCM's new ability to own encryption keys.

chrome/browser/push_messaging/push_messaging_service_impl.cc

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 48bc0d813ced6d859e455ec399fe6cbe089e9c0f..4f00a1935aeefb8bea6a558d4821cb0eaae69beb 100644
--- a/chrome/browser/push_messaging/push_messaging_service_impl.cc
+++ b/chrome/browser/push_messaging/push_messaging_service_impl.cc
@@ -211,8 +211,7 @@ void PushMessagingServiceImpl::OnMessage(const std::string& app_id,
   std::string data;
   // TODO(peter): Message payloads are disabled pending mandatory encryption.
   // https://crbug.com/449184
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnablePushMessagePayload)) {
+  if (AreMessagePayloadsEnabled()) {
     gcm::MessageData::const_iterator it = message.data.find("data");
     if (it != message.data.end())
       data = it->second;
@@ -315,15 +314,38 @@ GURL PushMessagingServiceImpl::GetPushEndpoint() {
   return GURL(std::string(kPushMessagingEndpoint));
 }
 
-// GetPublicEncryptionKey method -----------------------------------------------
+// GetPublicEncryptionKey methods ----------------------------------------------

[johnme, 2015/07/21 15:37:25]

Nit: Ideally this section would be between "Subscribe and GetPermissionStatus
methods" and "Unsubscribe methods" in the cc file as well.

[Peter Beverloo, 2015/07/21 15:51:08]

Done.

 
 void PushMessagingServiceImpl::GetPublicEncryptionKey(
     const GURL& origin,
     int64_t service_worker_registration_id,
     const PushMessagingService::PublicKeyCallback& callback) {
-  // TODO(peter): Get the public key from the GCM Driver. Right now we have to
-  // return success=true here, otherwise subscriptions would fail.
-  callback.Run(true /* success */, std::vector<uint8_t>());
+  // An empty public key will be returned if payloads are not enabled.
+  if (!AreMessagePayloadsEnabled()) {
+    callback.Run(true /* success */, std::vector<uint8_t>());
+    return;
+  }
+
+  PushMessagingAppIdentifier app_identifier =
+      PushMessagingAppIdentifier::FindByServiceWorker(
+          profile_, origin, service_worker_registration_id);
+
+  DCHECK(!app_identifier.is_null());
+
+  GetGCMDriver()->GetPublicKey(
+      app_identifier.app_id(),
+      base::Bind(&PushMessagingServiceImpl::DidGetPublicKey,
+                 weak_factory_.GetWeakPtr(), callback));
+}
+
+void PushMessagingServiceImpl::DidGetPublicKey(
+    const PushMessagingService::PublicKeyCallback& callback,
+    const std::string& public_key) const {
+  // I/O errors might prevent the GCM Driver from retrieving a key-pair.
+  const bool success = !!public_key.size();
+
+  callback.Run(success, std::vector<uint8_t>(public_key.begin(),

[johnme, 2015/07/21 15:37:25]

On platforms where char is signed, this might cause a conversion. Though I guess
it'll wrap correctly, so that's probably ok.

[Peter Beverloo, 2015/07/21 15:51:08]

Ack - yay for protobufs storing any binary data as strings :).

+                                             public_key.end()));
 }
 
 // Subscribe and GetPermissionStatus methods -----------------------------------
@@ -449,17 +471,28 @@ void PushMessagingServiceImpl::DidSubscribe(
     gcm::GCMClient::Result result) {
   content::PushRegistrationStatus status =
       content::PUSH_REGISTRATION_STATUS_SERVICE_ERROR;
-  std::vector<uint8_t> curve25519dh;
 
   switch (result) {
     case gcm::GCMClient::SUCCESS:
-      status = content::PUSH_REGISTRATION_STATUS_SUCCESS_FROM_PUSH_SERVICE;
-      app_identifier.PersistToPrefs(profile_);
-
-      // TODO(peter): Hook up getting the keys from the GCM Driver.
+      // Do not get a certificate if message payloads have not been enabled.
+      if (!AreMessagePayloadsEnabled()) {
+        DidSubscribeWithPublicKey(
+            app_identifier, callback, subscription_id,
+            std::string() /* public_key */);
+        return;
+      }
 
-      IncreasePushSubscriptionCount(1, false /* is_pending */);
-      break;
+      // Make sure that this subscription has associated encryption keys prior
+      // to returning it to the developer - they'll need this information in
+      // order to send payloads to the user.
+      GetGCMDriver()->GetPublicKey(
+          app_identifier.app_id(),
+          base::Bind(
+              &PushMessagingServiceImpl::DidSubscribeWithPublicKey,
+              weak_factory_.GetWeakPtr(), app_identifier, callback,
+              subscription_id));
+
+      return;
     case gcm::GCMClient::INVALID_PARAMETER:
     case gcm::GCMClient::GCM_DISABLED:
     case gcm::GCMClient::ASYNC_OPERATION_PENDING:
@@ -473,8 +506,28 @@ void PushMessagingServiceImpl::DidSubscribe(
       break;
   }
 
-  SubscribeEnd(callback, subscription_id, curve25519dh, status);
+  SubscribeEndWithError(callback, status);

[johnme, 2015/07/21 15:37:26]

Why are you removing the call to `DecreasePushSubscriptionCount(1, true /*
was_pending */);` in the error case?

[Peter Beverloo, 2015/07/21 15:51:08]

Oops - done.

+}
+
+void PushMessagingServiceImpl::DidSubscribeWithPublicKey(
+    const PushMessagingAppIdentifier& app_identifier,
+    const content::PushMessagingService::RegisterCallback& callback,
+    const std::string& subscription_id,
+    const std::string& public_key) {
+  if (!public_key.size() && AreMessagePayloadsEnabled()) {
+    SubscribeEndWithError(

[johnme, 2015/07/21 15:37:26]

You need to call `DecreasePushSubscriptionCount(1, true /* was_pending */);`
here too. In fact you might find it easier to just call it once from the top of
DidSubscribe.

[Peter Beverloo, 2015/07/21 15:51:08]

Done.

+        callback, content::PUSH_REGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE);
+    return;
+  }
+
+  app_identifier.PersistToPrefs(profile_);
+
+  IncreasePushSubscriptionCount(1, false /* is_pending */);
   DecreasePushSubscriptionCount(1, true /* was_pending */);
+
+  SubscribeEnd(callback, subscription_id,
+               std::vector<uint8_t>(public_key.begin(), public_key.end()),
+               content::PUSH_REGISTRATION_STATUS_SUCCESS_FROM_PUSH_SERVICE);
 }
 
 void PushMessagingServiceImpl::DidRequestPermission(
@@ -672,6 +725,11 @@ bool PushMessagingServiceImpl::IsPermissionSet(const GURL& origin) {
          blink::WebPushPermissionStatusGranted;
 }
 
+bool PushMessagingServiceImpl::AreMessagePayloadsEnabled() const {
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnablePushMessagePayload);
+}
+
 gcm::GCMDriver* PushMessagingServiceImpl::GetGCMDriver() const {
   gcm::GCMProfileService* gcm_profile_service =
       gcm::GCMProfileServiceFactory::GetForProfile(profile_);