[GCM] Handling connection events in GCMAccountTracker

chrome/browser/services/gcm/gcm_account_tracker.cc

Index: chrome/browser/services/gcm/gcm_account_tracker.cc
diff --git a/chrome/browser/services/gcm/gcm_account_tracker.cc b/chrome/browser/services/gcm/gcm_account_tracker.cc
index ca2cda5c401b99616ef8c54a3269489ffb6510de..8779b47a2d995a582aa41ea1ef9e4babf52b4a99 100644
--- a/chrome/browser/services/gcm/gcm_account_tracker.cc
+++ b/chrome/browser/services/gcm/gcm_account_tracker.cc
@@ -8,7 +8,9 @@
 #include <vector>
 
 #include "base/time/time.h"
+#include "components/gcm_driver/gcm_driver.h"
 #include "google_apis/gaia/google_service_auth_error.h"
+#include "net/base/ip_endpoint.h"
 
 namespace gcm {
 
@@ -17,6 +19,7 @@ const char kGCMGroupServerScope[] = "https://www.googleapis.com/auth/gcm";
 const char kGCMCheckinServerScope[] =
     "https://www.googleapis.com/auth/android_checkin";
 const char kGCMAccountTrackerName[] = "gcm_account_tracker";
+const int64 kMinimumTokenValidityMs = 500;
 }  // namespace
 
 GCMAccountTracker::AccountInfo::AccountInfo(const std::string& email,
@@ -29,12 +32,11 @@ GCMAccountTracker::AccountInfo::~AccountInfo() {
 
 GCMAccountTracker::GCMAccountTracker(
     scoped_ptr<gaia::AccountTracker> account_tracker,
-    const UpdateAccountsCallback& callback)
+    GCMDriver* driver)
     : OAuth2TokenService::Consumer(kGCMAccountTrackerName),
       account_tracker_(account_tracker.release()),
-      callback_(callback),
+      driver_(driver),
       shutdown_called_(false) {
-  DCHECK(!callback_.is_null());
 }
 
 GCMAccountTracker::~GCMAccountTracker() {
@@ -42,14 +44,16 @@ GCMAccountTracker::~GCMAccountTracker() {
 }
 
 void GCMAccountTracker::Shutdown() {
-  Stop();
   shutdown_called_ = true;
+  driver_->RemoveConnectionObserver(this);
+  account_tracker_->RemoveObserver(this);
   account_tracker_->Shutdown();
 }
 
 void GCMAccountTracker::Start() {
   DCHECK(!shutdown_called_);
   account_tracker_->AddObserver(this);
+  driver_->AddConnectionObserver(this);
 
   std::vector<gaia::AccountIds> accounts = account_tracker_->GetAccounts();
   if (accounts.empty()) {
@@ -69,12 +73,6 @@ void GCMAccountTracker::Start() {
   GetAllNeededTokens();
 }
 
-void GCMAccountTracker::Stop() {
-  DCHECK(!shutdown_called_);
-  account_tracker_->RemoveObserver(this);
-  pending_token_requests_.clear();
-}
-
 void GCMAccountTracker::OnAccountAdded(const gaia::AccountIds& ids) {
   DVLOG(1) << "Account added: " << ids.email;
   // We listen for the account signing in, which happens after account is added.
@@ -112,6 +110,7 @@ void GCMAccountTracker::OnGetTokenSuccess(
     if (iter->second.state == GETTING_TOKEN) {
       iter->second.state = TOKEN_PRESENT;
       iter->second.access_token = access_token;
+      iter->second.expiration_time = expiration_time;
     }
   }
 
@@ -141,8 +140,22 @@ void GCMAccountTracker::OnGetTokenFailure(
   CompleteCollectingTokens();
 }
 
+void GCMAccountTracker::OnConnected(const net::IPEndPoint& ip_endpoint) {
+  if (SanitizeTokens())
+    GetAllNeededTokens();
+}
+
+void GCMAccountTracker::OnDisconnected() {
+  // We are disconnected, so no point in trying to work with tokens.
+}
+
 void GCMAccountTracker::CompleteCollectingTokens() {
-  DCHECK(!callback_.is_null());
+  // Make sure all tokens are valid.
+  if (SanitizeTokens()) {
+    GetAllNeededTokens();
+    return;
+  }
+
   // Wait for gaia::AccountTracker to be done with fetching the user info, as
   // well as all of the pending token requests from GCMAccountTracker to be done
   // before you report the results.
@@ -183,9 +196,9 @@ void GCMAccountTracker::CompleteCollectingTokens() {
         break;
 
       case TOKEN_NEEDED:
-        // We failed to fetch an access token for the account, but it has not
-        // been signed out (perhaps there is a network issue). We don't report
-        // it, but next time there is a sign-in change we will update its state.
+        // This should not happen, as we are calling GetAllNeededTokens in that
+        // case and return.
+        NOTREACHED();
         ++iter;

[Nicolas Zea, 2014/10/08 00:36:35]

nit: Why not have ++iter in the for loop itself?

[fgorski, 2014/10/08 18:04:27]

Invalidates the iterator in 178. Since it does not affect complexity, I'll
extract the removing to a separate loop.

         break;
     }
@@ -193,13 +206,34 @@ void GCMAccountTracker::CompleteCollectingTokens() {
 
   // Make sure that there is something to report, otherwise bail out.
   if (!account_tokens.empty() || account_removed) {
-    DVLOG(1) << "Calling callback: " << account_tokens.size();
-    callback_.Run(account_tokens);
+    DVLOG(1) << "Reporting the tokens to driver: " << account_tokens.size();
+    driver_->SetAccountTokens(account_tokens);
   } else {
     DVLOG(1) << "No tokens and nothing removed. Skipping callback.";
   }
 }
 
+bool GCMAccountTracker::SanitizeTokens() {
+  bool tokens_needed = false;
+  for (AccountInfos::iterator iter = account_infos_.begin();
+       iter != account_infos_.end();
+       ++iter) {
+    if (iter->second.state == TOKEN_PRESENT &&
+        iter->second.expiration_time <
+            base::Time::Now() +
+                base::TimeDelta::FromMilliseconds(kMinimumTokenValidityMs)) {
+      iter->second.access_token.clear();
+      iter->second.state = TOKEN_NEEDED;
+      iter->second.expiration_time = base::Time();
+    }
+
+    if (iter->second.state == TOKEN_NEEDED)
+      tokens_needed = true;
+  }
+
+  return tokens_needed;
+}
+
 void GCMAccountTracker::DeleteTokenRequest(
     const OAuth2TokenService::Request* request) {
   ScopedVector<OAuth2TokenService::Request>::iterator iter = std::find(
@@ -209,6 +243,11 @@ void GCMAccountTracker::DeleteTokenRequest(
 }
 
 void GCMAccountTracker::GetAllNeededTokens() {
+  // Only start fetching tokens if driver is running, they have a limited
+  // validity time and GCM connection is a good indication of network running.
+  if (!driver_->IsConnected())
+    return;
+
   for (AccountInfos::iterator iter = account_infos_.begin();
        iter != account_infos_.end();
        ++iter) {