Signup for GCM upstream notifications

chrome/browser/chromeos/policy/heartbeat_scheduler.cc

Index: chrome/browser/chromeos/policy/heartbeat_scheduler.cc
diff --git a/chrome/browser/chromeos/policy/heartbeat_scheduler.cc b/chrome/browser/chromeos/policy/heartbeat_scheduler.cc
index a9109649dd59ef2996603b2b0f252dc59fa7615e..29f0d3cd1c7327c60c85a1bc36cea4b413c6778f 100644
--- a/chrome/browser/chromeos/policy/heartbeat_scheduler.cc
+++ b/chrome/browser/chromeos/policy/heartbeat_scheduler.cc
@@ -19,6 +19,7 @@
 namespace {
 const int kMinHeartbeatIntervalMs = 30 * 1000;  // 30 seconds
 const int kMaxHeartbeatIntervalMs = 24 * 60 * 60 * 1000;  // 24 hours
+const int kMinimumSignupIntervalMs = 3 * 60 * 1000;       // 3 minutes

[Andrew T Wilson (Slow), 2015/11/18 11:43:32]

Would be good to add a comment for what this is.

[binjin, 2015/11/19 05:50:27]

Done.

 
 // Our sender ID we send up with all of our GCM messages.
 const char* kHeartbeatGCMAppID = "com.google.chromeos.monitoring";
@@ -27,11 +28,20 @@ const char* kHeartbeatGCMAppID = "com.google.chromeos.monitoring";
 const char* kHeartbeatGCMDestinationID = "1013309121859";
 const char* kHeartbeatGCMSenderSuffix = "@google.com";
 
-const char* kMonitoringMessageTypeKey = "type";
+// Destination of upstream notification sign up message.
+const char* kUpstreamNotificationSignUpDestinationID =
+    "https://gcm.googleapis.com/gcm/gcm.event_tracker";
+
+// A bit mask, listening events of upstream notification.
+const char* kUpstreamNotificationSignUpListeningEvents =
+    "3";  // START | DISCONNECTED
+
+const char* kGcmMessageTypeKey = "type";
 const char* kHeartbeatTimestampKey = "timestamp";
 const char* kHeartbeatDomainNameKey = "domain_name";
 const char* kHeartbeatDeviceIDKey = "device_id";
 const char* kHeartbeatTypeValue = "hb";
+const char* kUpstreamNotificationNotifyKey = "notify";
 
 // If we get an error registering with GCM, try again in two minutes.
 const int64 kRegistrationRetryDelayMs = 2 * 60 * 1000;
@@ -242,6 +252,7 @@ void HeartbeatScheduler::ShutdownGCM() {
   if (registered_app_handler_) {
     registered_app_handler_ = false;
     gcm_driver_->RemoveAppHandler(kHeartbeatGCMAppID);
+    gcm_driver_->RemoveConnectionObserver(this);
   }
 }
 
@@ -274,6 +285,7 @@ void HeartbeatScheduler::ScheduleNextHeartbeat() {
       // a GCM connection.
       registered_app_handler_ = true;
       gcm_driver_->AddAppHandler(kHeartbeatGCMAppID, this);
+      gcm_driver_->AddConnectionObserver(this);
       registration_helper_.reset(new HeartbeatRegistrationHelper(
           gcm_driver_, task_runner_));
       registration_helper_->Register(
@@ -308,6 +320,7 @@ void HeartbeatScheduler::OnRegistrationComplete(
         registration_id,
         base::Bind(&HeartbeatScheduler::OnGcmIdUpdateRequestSent,
                    weak_factory_.GetWeakPtr()));
+    SignUpUpstreamNotification();
   }
 
   // Now that GCM registration is complete, start sending heartbeats.
@@ -328,7 +341,7 @@ void HeartbeatScheduler::SendHeartbeat() {
   // https://developer.chrome.com/apps/cloudMessaging#send_messages
   message.id = base::Int64ToString(
       base::Time::NowFromSystemTime().ToInternalValue());
-  message.data[kMonitoringMessageTypeKey] = kHeartbeatTypeValue;
+  message.data[kGcmMessageTypeKey] = kHeartbeatTypeValue;
   message.data[kHeartbeatTimestampKey] = base::Int64ToString(
       base::Time::NowFromSystemTime().ToJavaTime());
   message.data[kHeartbeatDomainNameKey] = enrollment_domain_;
@@ -340,6 +353,33 @@ void HeartbeatScheduler::SendHeartbeat() {
                                weak_factory_.GetWeakPtr()));
 }
 
+void HeartbeatScheduler::SignUpUpstreamNotification() {

[jinzhang1, 2015/11/16 19:10:49]

Hey Bin, I don't see you use AddHeartbeatInterval() defined in
https://codereview.chromium.org/1124783002/diff/80001/components/gcm_driver/g...
to set the gcm client heartbeat interval according to the device policy for
heartbeat frequency. Without it, GCM will only send heartbeat like every half an
hour.

[binjin, 2015/11/16 19:13:15]

This will be in a separated CL.

+  DCHECK(gcm_driver_);
+
+  if (registration_id_.empty())
+    return;
+
+  base::TimeDelta since_last_sign_up =
+      base::TimeTicks::Now() - last_upstream_notification_signup_time_;
+
+  if (!last_upstream_notification_signup_time_.is_null() &&

[Andrew T Wilson (Slow), 2015/11/18 11:43:32]

What's this doing? Seems like you are making sure we don't ping the upstream
server too often? What's the case where we need to worry about this?

[binjin, 2015/11/19 05:50:27]

The concern is over the GCM driver OnConnected event, I'm not exactly sure if
there can be a scenario that it ends up flooding the server.

+      since_last_sign_up <
+          base::TimeDelta::FromMilliseconds(kMinimumSignupIntervalMs)) {
+    return;
+  }
+
+  gcm::OutgoingMessage message;
+  message.id =
+      base::Int64ToString(base::Time::NowFromSystemTime().ToInternalValue());
+  message.data[kGcmMessageTypeKey] = kUpstreamNotificationSignUpListeningEvents;
+  message.data[kUpstreamNotificationNotifyKey] =
+      GetDestinationID() + kHeartbeatGCMSenderSuffix;
+  gcm_driver_->Send(kHeartbeatGCMAppID,
+                    kUpstreamNotificationSignUpDestinationID, message,

[Andrew T Wilson (Slow), 2015/11/18 11:43:32]

Do we need to be able to specify a different destination ID (for example, when
testing with the staging server vs prod)?

[binjin, 2015/11/19 05:50:27]

The current destination ID (https://gcm.googleapis.com/gcm/gcm.event_tracker) is
public and it's just a relay service. If it's just for testing, specifying the
actual destination ID being relayed (kHeartbeatGCMDestinationID) should be
enough.

+                    base::Bind(&HeartbeatScheduler::OnUpstreamNotificationSent,
+                               weak_factory_.GetWeakPtr()));
+}
+
 void HeartbeatScheduler::OnHeartbeatSent(const std::string& message_id,
                                          gcm::GCMClient::Result result) {
   DVLOG(1) << "Monitoring heartbeat sent - result = " << result;
@@ -351,6 +391,15 @@ void HeartbeatScheduler::OnHeartbeatSent(const std::string& message_id,
   ScheduleNextHeartbeat();
 }
 
+void HeartbeatScheduler::OnUpstreamNotificationSent(
+    const std::string& message_id,
+    gcm::GCMClient::Result result) {
+  DVLOG(1) << "Upstream notification signup message sent - result = " << result;
+  DLOG_IF(ERROR, result != gcm::GCMClient::SUCCESS)
+      << "Error sending upstream notification signup message: " << result;
+  last_upstream_notification_signup_time_ = base::TimeTicks::Now();
+}
+
 HeartbeatScheduler::~HeartbeatScheduler() {
   ShutdownGCM();
 }
@@ -385,6 +434,10 @@ void HeartbeatScheduler::OnSendAcknowledged(const std::string& app_id,
   DVLOG(1) << "Heartbeat sent with message_id: " << message_id;
 }
 
+void HeartbeatScheduler::OnConnected(const net::IPEndPoint&) {
+  SignUpUpstreamNotification();
+}
+
 void HeartbeatScheduler::OnGcmIdUpdateRequestSent(bool success) {
   // TODO(binjin): Handle the failure, probably by exponential backoff.
   LOG_IF(WARNING, !success) << "Failed to send GCM id to DM server";