[GCM] Check-in with signed in accounts associates device to user

components/gcm_driver/gcm_client_impl.cc

Index: components/gcm_driver/gcm_client_impl.cc
diff --git a/components/gcm_driver/gcm_client_impl.cc b/components/gcm_driver/gcm_client_impl.cc
index e36d36c79735e46432bac2d1fb6764b0de917f43..b7abe72d29497734c31245e37f975ff2fa0da993 100644
--- a/components/gcm_driver/gcm_client_impl.cc
+++ b/components/gcm_driver/gcm_client_impl.cc
@@ -247,6 +247,24 @@ scoped_ptr<ConnectionFactory> GCMInternalsBuilder::BuildConnectionFactory(
                                 recorder));
 }
 
+GCMClientImpl::CheckinInfo::CheckinInfo()
+    : android_id(0),
+      secret(0),
+      last_checkin_accounts_count(0),
+      accounts_set(false) {
+}
+
+GCMClientImpl::CheckinInfo::~CheckinInfo() {
+}
+
+void GCMClientImpl::CheckinInfo::Reset() {
+  android_id = 0;
+  secret = 0;
+  last_checkin_accounts_count = 0;
+  accounts_set = false;
+  account_tokens.clear();
+}
+
 GCMClientImpl::GCMClientImpl(scoped_ptr<GCMInternalsBuilder> internals_builder)
     : internals_builder_(internals_builder.Pass()),
       state_(UNINITIALIZED),
@@ -314,6 +332,14 @@ void GCMClientImpl::OnLoadCompleted(scoped_ptr<GCMStore::LoadResult> result) {
   registrations_ = result->registrations;
   device_checkin_info_.android_id = result->device_android_id;
   device_checkin_info_.secret = result->device_security_token;
+  device_checkin_info_.last_checkin_accounts_count = result->accounts_count;
+  // A case where there were previously no accounts reported with checkin is
+  // considered to be the same as when the list of accounts is empty. It enables
+  // scheduling a periodc checkin for devices with no signed in users

[Nicolas Zea, 2014/07/10 20:57:32]

nit: periodic

[fgorski, 2014/07/11 22:51:20]

Done.

+  // immediately after restart, while keeping |accounts_set == false| delays the
+  // checkin until the list of accounts is set explicitly.
+  if (result->accounts_count == 0)
+    device_checkin_info_.accounts_set = true;
   last_checkin_time_ = result->last_checkin_time;
   gservices_settings_.UpdateFromLoadResult(*result);
   InitializeMCSClient(result.Pass());
@@ -363,6 +389,9 @@ void GCMClientImpl::OnFirstTimeDeviceCheckinCompleted(
 
   device_checkin_info_.android_id = checkin_info.android_id;
   device_checkin_info_.secret = checkin_info.secret;
+  // If accounts were not set by now, we can consider them set (to empty list)
+  // to make sure periodic checkins get scheduled after initial checkin.
+  device_checkin_info_.accounts_set = true;
   gcm_store_->SetDeviceCredentials(
       checkin_info.android_id, checkin_info.secret,
       base::Bind(&GCMClientImpl::SetDeviceCredentialsCallback,
@@ -390,6 +419,34 @@ void GCMClientImpl::ResetState() {
   // TODO(fgorski): reset all of the necessart objects and start over.
 }
 
+void GCMClientImpl::SetAccountsForCheckin(
+    const std::map<std::string, std::string>& account_tokens,
+    bool account_removed) {
+  bool accounts_set_before = device_checkin_info_.accounts_set;
+  device_checkin_info_.account_tokens = account_tokens;
+  device_checkin_info_.accounts_set = true;
+
+  DVLOG(1) << "Set account called with: " << account_tokens.size()
+           << " accounts.";
+
+  if (state_ != READY && state_ != INITIAL_DEVICE_CHECKIN)
+    return;
+
+  // Checkin will be forced when any of the accounts was removed during the
+  // current Chrome session or if there has been an account removed between the
+  // restarts of Chrome. If there is a checkin in progress, it will be canceled.
+  if (account_removed ||

[Nicolas Zea, 2014/07/10 20:57:32]

Do we need the account_removed boolean if we can compare the
last_checkin_accounts_count with account_tokens.size()?

[fgorski, 2014/07/11 22:51:20]

If the same account is first removed and then added after a few minutes, it will
take 2 days to get the link between account and device back, but it will
eventually fix itself. We are making the process more sensitive to account
removal here.

+      device_checkin_info_.last_checkin_accounts_count >

[Nicolas Zea, 2014/07/10 20:57:32]

I also wonder if there's  risk of issues if an account was removed and then one
was added back. Last checkin accounts count might match the current token size.
Is that possible?

[fgorski, 2014/07/11 22:51:21]

This approach can run into issues where the count is the same because you added
one and removed another, hence we are using the accounts_removed flag.

If some accounts can be removed when the device is not looking, we generally
fall back to the number comparison.

There is another way to do the whole thing, which requires storing a complete
list of accounts and making a more thorough comparison when setting a new list
of accounts. I can implement it that way. We won't require the account_removed
flag then.

+          account_tokens.size()) {
+    DVLOG(1) << "Detecting that account has been removed, forcing checkin.";
+    checkin_request_.reset();
+    StartCheckin();
+  } else if (!accounts_set_before) {
+    SchedulePeriodicCheckin();
+    DVLOG(1) << "Scheduled periodic checkin.";
+  }
+}
+
 void GCMClientImpl::StartCheckin() {
   // Make sure no checkin is in progress.
   if (checkin_request_.get())
@@ -399,6 +456,7 @@ void GCMClientImpl::StartCheckin() {
   ToCheckinProtoVersion(chrome_build_info_, &chrome_build_proto);
   CheckinRequest::RequestInfo request_info(device_checkin_info_.android_id,
                                            device_checkin_info_.secret,
+                                           device_checkin_info_.account_tokens,
                                            gservices_settings_.digest(),
                                            chrome_build_proto);
   checkin_request_.reset(
@@ -409,6 +467,10 @@ void GCMClientImpl::StartCheckin() {
                                     weak_ptr_factory_.GetWeakPtr()),
                          url_request_context_getter_,
                          &recorder_));
+  // Taking a snapshot of the accounts count here, as there might be an asynch
+  // update of the account tokens while checkin is in progress.
+  device_checkin_info_.last_checkin_accounts_count =
+      device_checkin_info_.account_tokens.size();
   checkin_request_->Start();
 }
 
@@ -448,9 +510,10 @@ void GCMClientImpl::OnCheckinCompleted(
     }
 
     last_checkin_time_ = clock_->Now();
-    gcm_store_->SetLastCheckinTime(
+    gcm_store_->SetLastCheckinInfo(
         last_checkin_time_,
-        base::Bind(&GCMClientImpl::SetLastCheckinTimeCallback,
+        device_checkin_info_.last_checkin_accounts_count,
+        base::Bind(&GCMClientImpl::SetLastCheckinInfoCallback,
                    weak_ptr_factory_.GetWeakPtr()));
     SchedulePeriodicCheckin();
   }
@@ -462,7 +525,7 @@ void GCMClientImpl::SetGServicesSettingsCallback(bool success) {
 
 void GCMClientImpl::SchedulePeriodicCheckin() {
   // Make sure no checkin is in progress.
-  if (checkin_request_.get())
+  if (checkin_request_.get() || !device_checkin_info_.accounts_set)
     return;
 
   // There should be only one periodic checkin pending at a time. Removing
@@ -485,7 +548,7 @@ base::TimeDelta GCMClientImpl::GetTimeToNextCheckin() const {
          clock_->Now();
 }
 
-void GCMClientImpl::SetLastCheckinTimeCallback(bool success) {
+void GCMClientImpl::SetLastCheckinInfoCallback(bool success) {
   // TODO(fgorski): This is one of the signals that store needs a rebuild.
   DCHECK(success);
 }