[GCM] Fetching OAuth2 tokens periodically in account tracker

chrome/browser/services/gcm/gcm_account_tracker.h

Index: chrome/browser/services/gcm/gcm_account_tracker.h
diff --git a/chrome/browser/services/gcm/gcm_account_tracker.h b/chrome/browser/services/gcm/gcm_account_tracker.h
index 0b1c5e81f32cb42623ea79a8667de59f666ee05c..d94fe66697c18761a39d21d57924aa9c9df34368 100644
--- a/chrome/browser/services/gcm/gcm_account_tracker.h
+++ b/chrome/browser/services/gcm/gcm_account_tracker.h
@@ -13,6 +13,11 @@
 #include "components/gcm_driver/gcm_connection_observer.h"
 #include "google_apis/gaia/account_tracker.h"
 #include "google_apis/gaia/oauth2_token_service.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"

[Nicolas Zea, 2014/11/06 21:43:17]

why is this needed?

[fgorski, 2014/11/07 01:42:13]

Done.
In the end I made the test class a friend

+
+namespace base {
+class Time;
+}
 
 namespace gcm {
 
@@ -20,6 +25,11 @@ class GCMDriver;
 
 // Class for reporting back which accounts are signed into. It is only meant to
 // be used when the user is signed into sync.
+//
+// This class makes a check for tokens periodically, to make sure the user is
+// still logged into the profile, so that in the case that the user is not, we

[Nicolas Zea, 2014/11/06 21:43:17]

Is it just to ensure that we're still logged into the profile? Or is it to
ensure that the tokens themselves haven't expired or been revoked remotely?

We shouldn't need to periodically poll for a local event (i.e. sign out). We can
listen for those events directly.

[fgorski, 2014/11/07 01:42:13]

Every checkin with GCM should have a fresh set of tokens. We are watching for
the event, but on top of that want to periodically get a fresh batch of tokens
to report (in case the user never restarts Chrome.

[Nicolas Zea, 2014/11/07 22:19:25]

I think its worth mentioning this part in the comment. We're not really fetching
tokens periodically to see if we're logged in (although that checks that too),
but to actually have a fresh set of access tokens to send to the Checkin server.

+// can immediately report that to the GCM and stop messages addressed to that
+// user from ever reaching Chrome.
 class GCMAccountTracker : public gaia::AccountTracker::Observer,
                           public OAuth2TokenService::Consumer,
                           public GCMConnectionObserver {
@@ -76,6 +86,8 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer,
   }
 
  private:
+  friend class GCMAccountTrackerTest;
+
   // Maps account keys to account states. Keyed by account_ids as used by
   // OAuth2TokenService.
   typedef std::map<std::string, AccountInfo> AccountInfos;
@@ -97,13 +109,22 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer,
   void OnConnected(const net::IPEndPoint& ip_endpoint) override;
   void OnDisconnected() override;
 
+  // Schedules fetching tokens at the time when they are needed to be reported.
+  void ScheduleFetchingTokens();
   // Report the list of accounts with OAuth2 tokens back using the |callback_|
   // function. If there are token requests in progress, do nothing.
-  void CompleteCollectingTokens();
+  void ReportTokens();
   // Verify that all of the tokens are ready to be passed down to the GCM
   // Driver, e.g. none of them has expired or is missing. Returns true if not
   // all tokens are valid and a fetching yet more tokens is required.
   bool SanitizeTokens();
+  // Indicates whether token reporting is required, either because it is due, or
+  // some of the accounts were removed.
+  bool IsTokenReportingRequired() const;
+  // Indicates whether there are tokens that still need fetching.
+  bool IsTokenFetchingRequired() const;
+  // Gets the time until next token fetching.
+  base::TimeDelta GetTimeToNextTokenFetching() const;
   // Deletes a token request. Should be called from OnGetTokenSuccess(..) or
   // OnGetTokenFailure(..).
   void DeleteTokenRequest(const OAuth2TokenService::Request* request);
@@ -133,6 +154,8 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer,
 
   ScopedVector<OAuth2TokenService::Request> pending_token_requests_;
 
+  base::WeakPtrFactory<GCMAccountTracker> weak_ptr_factory_;

[Nicolas Zea, 2014/11/06 21:43:17]

nit: Given that this is specifically for reporting tokens, perhaps we should
name it something else? If someone were to attempt to use this for binding to
other methods, the InvalidateAll might introduce bugs. 

[fgorski, 2014/11/07 01:42:12]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(GCMAccountTracker);
 };