[GCM] Adding registration ID request and tests to GCM Account Mapper

components/gcm_driver/gcm_account_mapper.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/gcm_driver/gcm_account_mapper.h"
 
 #include "base/bind.h"
 #include "base/guid.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
@@ skipping 29 matching lines @@
     : gcm_driver_(gcm_driver),
       clock_(new base::DefaultClock),
       initialized_(false),
       weak_ptr_factory_(this) {
 }
 
 GCMAccountMapper::~GCMAccountMapper() {
 }
 
 void GCMAccountMapper::Initialize(
-    const std::vector<AccountMapping>& account_mappings,
-    const std::string& registration_id) {
+    const std::vector<AccountMapping>& account_mappings) {
   DCHECK(!initialized_);
   initialized_ = true;
-  registration_id_ = registration_id;
-
   accounts_ = account_mappings;
-
   gcm_driver_->AddAppHandler(kGCMAccountMapperAppId, this);
-
-  // TODO(fgorski): if no registration ID, get registration ID.
+  Register();
 }
 
 void GCMAccountMapper::SetAccountTokens(
     const std::vector<GCMClient::AccountTokenInfo> account_tokens) {
-  DCHECK(initialized_);
+  // If account mapper is not ready to handle tasks yet, save the lastest

[Nicolas Zea, 2014/09/18 17:35:14]

nit: lastest -> latest

[fgorski, 2014/09/18 18:08:53]

Done.

+  // account tokens and return.
+  if (!IsReady()) {
+    pending_account_tokens_ = account_tokens;
+    return;
+  }
 
   // Start from removing the old tokens, from all of the known accounts.
   for (AccountMappings::iterator iter = accounts_.begin();
        iter != accounts_.end();
        ++iter) {
     iter->access_token.clear();
   }
 
   // Update the internal collection of mappings with the new tokens.
   for (std::vector<GCMClient::AccountTokenInfo>::const_iterator token_iter =
@@ skipping 131 matching lines @@
     account_mapping_it->last_message_id.clear();
 
     gcm_driver_->UpdateAccountMapping(*account_mapping_it);
   }
 }
 
 bool GCMAccountMapper::CanHandle(const std::string& app_id) const {
   return app_id.compare(kGCMAccountMapperAppId) == 0;
 }
 
+bool GCMAccountMapper::IsReady() {
+  return initialized_ && !registration_id_.empty();
+}
+
 void GCMAccountMapper::SendAddMappingMessage(AccountMapping& account_mapping) {
   CreateAndSendMessage(account_mapping);
 }
 
 void GCMAccountMapper::SendRemoveMappingMessage(
     AccountMapping& account_mapping) {
   // We want to persist an account that is being removed as quickly as possible
   // as well as clean up the last message information.
   if (account_mapping.status != AccountMapping::REMOVING) {
     account_mapping.status = AccountMapping::REMOVING;
@@ skipping 23 matching lines @@
   }
 
   gcm_driver_->Send(kGCMAccountMapperAppId,
                     kGCMAccountMapperSenderId,
                     outgoing_message,
                     base::Bind(&GCMAccountMapper::OnSendFinished,
                                weak_ptr_factory_.GetWeakPtr(),
                                account_mapping.account_id));
 }
 
-
 void GCMAccountMapper::OnSendFinished(const std::string& account_id,
                                       const std::string& message_id,
                                       GCMClient::Result result) {
   // TODO(fgorski): Add another attempt, in case the QUEUE is not full.
   if (result != GCMClient::SUCCESS)
     return;
 
   AccountMapping* account_mapping = FindMappingByAccountId(account_id);
   DCHECK(account_mapping);
 
   // If we are dealing with account with status NEW, it is the first time
   // mapping, and we should mark it as ADDING.
   if (account_mapping->status == AccountMapping::NEW) {
     account_mapping->status = AccountMapping::ADDING;
     account_mapping->status_change_timestamp = clock_->Now();
   }
 
   account_mapping->last_message_id = message_id;
 
   gcm_driver_->UpdateAccountMapping(*account_mapping);
 }
 
+void GCMAccountMapper::Register() {

[Nicolas Zea, 2014/09/18 17:35:14]

nit: dcheck !registration_id_? Or do we want to have support for calling
register more than once?

[Nicolas Zea, 2014/09/18 17:36:10]

Come to think of it, do we have any logic to prevent multiple registration
requests happening at once?

[fgorski, 2014/09/18 18:08:53]

Yes, the async operation pending will be issued immediately.

[fgorski, 2014/09/18 18:08:53]

Done.

+  std::vector<std::string> sender_ids;
+  sender_ids.push_back(kGCMAccountMapperSenderId);
+  gcm_driver_->Register(kGCMAccountMapperAppId,
+                        sender_ids,
+                        base::Bind(&GCMAccountMapper::OnRegisterFinished,
+                                   weak_ptr_factory_.GetWeakPtr()));
+}
+
+void GCMAccountMapper::OnRegisterFinished(const std::string& registration_id,
+                                          GCMClient::Result result) {
+  if (result == GCMClient::SUCCESS)
+    registration_id_ = registration_id;
+
+  if (IsReady()) {
+    if (!pending_account_tokens_.empty()) {
+      SetAccountTokens(pending_account_tokens_);
+      pending_account_tokens_.clear();
+    }
+  } else {
+    if (result != GCMClient::ASYNC_OPERATION_PENDING)
+      Register();

[Nicolas Zea, 2014/09/18 17:35:14]

how many times will we retry?

[fgorski, 2014/09/18 18:08:53]

Register implements back-off, so it does not matter that much... If we don't
have a network we can keep trying. Here are alternatives:
* I can implement gcm connection observer to get an indication of when network
is present, but that would not be on the connection that registration would run
on.
* I could listen to a generic network event and only retry Register then.
* I can bail and never care about registration, or perhaps reach out for it when
I got a batch of tokens but no registration yet. (this is lazy, but should work
-> I think I like that the most.)
* We could simply ignore it and wait for a restart, but some people don't
restart that often.

In either case we are only talking about the case where there is no registration
id in the cache, so this is a very narrow case. (First time setup of the
mapper.)

[Nicolas Zea, 2014/09/19 17:41:58]

Does the bail approach mean retry when new tokens arrive (new refresh tokens or
access tokens?), and if that doesn't happen retry on restart? Unless we're
talking access tokens, we don't generally get refresh tokens very often, so I
suspect this will primarily rely on restart.

I'm okay if we let it keep retrying, but would prefer putting at least 30 second
delay between a failed registration attempt and a retry (backoff is reset after
all)

+  }
+}
+
 bool GCMAccountMapper::CanTriggerUpdate(
     const base::Time& last_update_time) const {
   return last_update_time +
              base::TimeDelta::FromHours(kGCMUpdateIntervalHours -
                                         kGCMUpdateEarlyStartHours) <
          clock_->Now();
 }
 
 bool GCMAccountMapper::IsLastStatusChangeOlderThanTTL(
     const AccountMapping& account_mapping) const {
@@ skipping 25 matching lines @@
   }
 
   return accounts_.end();
 }
 
 void GCMAccountMapper::SetClockForTesting(scoped_ptr<base::Clock> clock) {
   clock_ = clock.Pass();
 }
 
 }  // namespace gcm