Introduce the PushSubscription.curve25519dh attribute (2/3)

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 992739312ee132d4294673b23a762ae90ec00040..007c2e4ef48a0e9939ab7d0a636479b5eb1a4405 100644
--- a/chrome/browser/push_messaging/push_messaging_service_impl.cc
+++ b/chrome/browser/push_messaging/push_messaging_service_impl.cc
@@ -332,8 +332,8 @@ void PushMessagingServiceImpl::SubscribeFromDocument(
 
   if (push_subscription_count_ + pending_push_subscription_count_ >=
       kMaxRegistrations) {
-    SubscribeEnd(callback, std::string(),
-                 content::PUSH_REGISTRATION_STATUS_LIMIT_REACHED);
+    SubscribeEndWithError(callback,
+                          content::PUSH_REGISTRATION_STATUS_LIMIT_REACHED);
     return;
   }
 
@@ -360,9 +360,8 @@ void PushMessagingServiceImpl::SubscribeFromDocument(
         content::CONSOLE_MESSAGE_LEVEL_ERROR,
         kSilentPushUnsupportedMessage);
 
-    SubscribeEnd(callback,
-                 std::string(),
-                 content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
+    SubscribeEndWithError(callback,
+                          content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
     return;
   }
 
@@ -390,8 +389,8 @@ void PushMessagingServiceImpl::SubscribeFromWorker(
 
   if (profile_->GetPrefs()->GetInteger(
           prefs::kPushMessagingRegistrationCount) >= kMaxRegistrations) {
-    SubscribeEnd(register_callback, std::string(),
-                 content::PUSH_REGISTRATION_STATUS_LIMIT_REACHED);
+    SubscribeEndWithError(register_callback,
+                          content::PUSH_REGISTRATION_STATUS_LIMIT_REACHED);
     return;
   }
 
@@ -404,8 +403,8 @@ void PushMessagingServiceImpl::SubscribeFromWorker(
                                                     embedding_origin,
                                                     user_visible);
   if (permission_status != blink::WebPushPermissionStatusGranted) {
-    SubscribeEnd(register_callback, std::string(),
-                 content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
+    SubscribeEndWithError(register_callback,
+                          content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
     return;
   }
 
@@ -417,6 +416,15 @@ void PushMessagingServiceImpl::SubscribeFromWorker(
                                       app_identifier, register_callback));
 }
 
+void PushMessagingServiceImpl::GetPublicEncryptionKey(

[johnme, 2015/07/09 16:02:53]

Nit: Please move this to a new section of the .cc file called
"GetPublicEncryptionKey method", just above "// Unsubscribe methods" (rather
than in the middle of the subscribe methods - GetPermissionStatus is only here
by historical accident).

[Peter Beverloo, 2015/07/09 18:54:17]

Done.

+    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>());
+}
+
 blink::WebPushPermissionStatus PushMessagingServiceImpl::GetPermissionStatus(
     const GURL& requesting_origin,
     const GURL& embedding_origin,
@@ -436,8 +444,16 @@ bool PushMessagingServiceImpl::SupportNonVisibleMessages() {
 void PushMessagingServiceImpl::SubscribeEnd(
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& subscription_id,
+    const std::vector<uint8_t>& curve25519dh,
+    content::PushRegistrationStatus status) {
+  callback.Run(subscription_id, curve25519dh, status);
+}
+
+void PushMessagingServiceImpl::SubscribeEndWithError(
+    const content::PushMessagingService::RegisterCallback& callback,
     content::PushRegistrationStatus status) {
-  callback.Run(subscription_id, status);
+  SubscribeEnd(callback, std::string() /* subscription_id */,
+               std::vector<uint8_t>() /* curve25519dh */, status);
 }
 
 void PushMessagingServiceImpl::DidSubscribe(
@@ -465,7 +481,11 @@ void PushMessagingServiceImpl::DidSubscribe(
       status = content::PUSH_REGISTRATION_STATUS_NETWORK_ERROR;
       break;
   }
-  SubscribeEnd(callback, subscription_id, status);
+
+  // TODO(peter): Hook up getting the keys from the GCM Driver.
+  std::vector<uint8_t> curve25519dh;
+
+  SubscribeEnd(callback, subscription_id, curve25519dh, status);

[johnme, 2015/07/09 16:02:53]

Nit: it feels slightly inconsistent that you still use SubscribeEnd for the
error cases here, now that there is SubscribeEndWithError. But I don't feel
strongly.

[Peter Beverloo, 2015/07/09 18:54:16]

The alternative would be to return from the gcm::GCMClient::SUCCESS case and
duplicate the DecreasePushSubscriptionCount call. I'd rather keep that logic
isolated.

I did move the curve25519dh definition and the TODO to clarify this.

   DecreasePushSubscriptionCount(1, true /* was_pending */);
 }
 
@@ -475,9 +495,8 @@ void PushMessagingServiceImpl::DidRequestPermission(
     const content::PushMessagingService::RegisterCallback& register_callback,
     ContentSetting content_setting) {
   if (content_setting != CONTENT_SETTING_ALLOW) {
-    SubscribeEnd(register_callback,
-                 std::string(),
-                 content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
+    SubscribeEndWithError(register_callback,
+                          content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
     return;
   }